Opened 3 months ago

Last modified 3 months ago

#22895 merge_ready defect

Unused variables in donna's SSE2 header

Reported by: cypherpunks Owned by:
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version: Tor: 0.2.7.2-alpha
Severity: Normal Keywords: 027-backport, 028-backport, 029-backport, 030-backport, 031-backport, compile, warning, 32bit, ed25519, review-group-21
Cc: Actual Points:
Parent ID: Points:
Reviewer: asn Sponsor:

Description

FWICT this only occurs on 32-bit x86 systems because of the portability header.

Child Tickets

Attachments (1)

0001-Remove-unused-variables-in-donna-s-SSE2-code.patch (2.5 KB) - added by cypherpunks 3 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 3 months ago by cypherpunks

I probably should provide a little more information. The unused variables are in the Curve25519 SSE2 header.

In file included from src/ext/ed25519/donna/ed25519-donna.h:55:0,
                 from src/ext/ed25519/donna/ed25519_tor.c:38:
src/ext/ed25519/donna/curve25519-donna-sse2.h:51:27: error: ‘packed121666121665’ defined but not used [-Werror=unused-const-variable=]
 static const packedelem64 packed121666121665 = {{121666, 121665}};
                           ^~~~~~~~~~~~~~~~~~
src/ext/ed25519/donna/curve25519-donna-sse2.h:48:27: error: ‘packed9638’ defined but not used [-Werror=unused-const-variable=]
 static const packedelem64 packed9638 = {{19*4,19*2}};
                           ^~~~~~~~~~
src/ext/ed25519/donna/curve25519-donna-sse2.h:45:27: error: ‘packednineteenone’ defined but not used [-Werror=unused-const-variable=]
 static const packedelem64 packednineteenone = {{19, 1}};
                           ^~~~~~~~~~~~~~~~~
src/ext/ed25519/donna/curve25519-donna-sse2.h:39:27: error: ‘packedmask2625’ defined but not used [-Werror=unused-const-variable=]
 static const packedelem32 packedmask2625 = {{0x3ffffff,0,0x1ffffff,0}};
                           ^~~~~~~~~~~~~~
src/ext/ed25519/donna/curve25519-donna-sse2.h:32:27: error: ‘top32bitmask’ defined but not used [-Werror=unused-const-variable=]
 static const packedelem32 top32bitmask = {{0x00000000, 0xffffffff, 0x00000000, 0xffffffff}};
                           ^~~~~~~~~~~~

comment:2 Changed 3 months ago by cypherpunks

Status: newneeds_review
Version: Tor: 0.2.7.2-alpha

comment:3 Changed 3 months ago by nickm

Keywords: 027-backport 028-backport 029-backport 030-backport 031-backport compile warning 32bit ed25519 added
Milestone: Tor: 0.3.2.x-final

comment:4 Changed 3 months ago by nickm

Keywords: review-group-21 added

comment:5 Changed 3 months ago by asn

Reviewer: asn

comment:6 Changed 3 months ago by asn

Branch looks good to me. The variables removed are indeed unused.

However, I wanted to compile the patch to check that it indeed fixes the gcc warnings. Can anyone help me compile tor in 32-bit mode? IIUC I should be doing ./configure --host=i686-linux-gnu "CFLAGS=-m32" && make, but when I did that the configure script was complaining that it can't find my libevent. Is that the correct way to do it, and I just need to get a 32-bit libevent or something? If someone helps me do this, I promise I'm gonna transcribe the process to doc/HACKING/ for future generations to be able to do it as well.

comment:7 Changed 3 months ago by nickm

Right; you're going to need 32-bit versions of the libraries you're using so that you can link against them. If you're on a linux distribution, most of them already have their libraries available in both 64-bit and 32-bit.

comment:8 Changed 3 months ago by asn

Status: needs_reviewmerge_ready

Thanks for the help Nick. I managed to compile Tor in 32-bit mode:

$ file src/or/tor
src/or/tor: ELF 32-bit LSB shared object, Intel 80386, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux.so.2, for GNU/Linux 2.6.32, BuildID[sha1]=72f975f78f2ef98852368ebbe3a6205ddf0ea944, not stripped

I still have not managed to trigger this error unfortunately, even tho I compiled tor with $ CFLAGS="-m32" ./configure --enable-gcc-warnings --disable-asciidoc --enable-expensive-hardening and I checked that src/ext/ed25519/donna/libed25519_donna.a was built...

Not sure what to do here. Anyone has tips on how to repro this?

Otherwise, I just say we merge the patch since it just removes unused variables anyway (I checked all of them).

comment:9 Changed 3 months ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.1.x-final

In that case, I think it's fine to take this in 0.3.2, and consider for a possible backport. Thanks!

comment:10 Changed 3 months ago by nickm

The branch to backport is bug22895_027.

Note: See TracTickets for help on using tickets.