Opened 4 days ago

Last modified 35 hours ago

#27728 needs_review defect

Signature mismatch for crypto_strongest_rand() in ed-donna code breaks -flto compilation

Reported by: Hello71 Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Normal Keywords: regression
Cc: dgoulet, haxxpop Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

./src/lib/crypt_ops/crypto_rand.h:24:1: error: variable ‘crypto_strongest_rand’ redeclared as function
 MOCK_DECL(void,crypto_strongest_rand,(uint8_t *out, size_t out_len));
 ^
./src/lib/crypt_ops/crypto_rand.h:24:1: error: variable ‘crypto_strongest_rand’ redeclared as function
src/lib/crypt_ops/crypto_rand.c:338:1: note: previously declared here
 MOCK_IMPL(void,
 ^

Child Tickets

Change History (11)

comment:1 Changed 3 days ago by nickm

Milestone: Tor: 0.3.5.x-final

comment:2 Changed 3 days ago by nickm

Component: - Select a componentCore Tor/Tor

version, compiler, etc? This builds fine for me.

Also, I don't understand why the compiler thinks that line 24 of crypto_rand.h is happening *after* line 338 of crypto_rand.c.

comment:3 Changed 3 days ago by Hello71

tor 035166e (master as of about 6 hours ago), gcc 8.2.1 with -O3 -flto and other flags. I assume the order is across files due to the LTO.

It looks like src/ext/ed25519/ref10/src_ext_ed25519_ref10_libed25519_ref10_a-keypair.o and src/ext/ed25519/donna/src_ext_ed25519_donna_libed25519_donna_a-ed25519_tor.o are built without TOR_UNIT_TESTS, so they declare crypto_strongest_rand as a function instead of a variable. I checked this by putting #ifndef TOR_UNIT_TESTS #error bad include #endif in src/lib/crypt_ops/crypto_rand.h and then remaking src/test/test, relying on automake's dependency tracking.

Last edited 3 days ago by Hello71 (previous) (diff)

comment:4 Changed 3 days ago by Hello71

gcc 8.2.1+20180831-1

comment:5 Changed 2 days ago by Hello71

specifically, steps to reproduce:

  1. install gcc, maybe you need 8.2.1+20180831-1 (I use Arch, fyi), probably 8.2.0 or even earlier will break too
  1. configure tor git master with CFLAGS='-O3 -flto' LDFLAGS=-flto (probably -O2 works too, but I only tried -O3)
  1. make src/test/test

alternatively:

  1. insert:
    #ifndef TOR_UNIT_TESTS
    #error bad include
    #endif
    

in src/lib/crypt_ops/crypto_rand.h.

  1. make src/test/test.

because only the test executable is being built, all object files must define crypto_strongest_rand as the same type. this requires that src/lib/crypt_ops/crypto_rand.h must always be included with TOR_UNIT_TESTS (or without, but we are forcing only tests to be built here)

comment:6 Changed 37 hours ago by nickm

Owner: set to nickm
Status: newaccepted

comment:7 Changed 37 hours ago by nickm

Summary: test: Build an HSv3 descriptor with authorized client is apparently brokenSignature mismatch for crypto_strongest_rand() in ed-donna code breaks -flto compilation

comment:8 Changed 35 hours ago by nickm

Status: acceptedneeds_review

Okay -- I've had to install GCC 8.2 locally, and after a bit I've successfully reproduced this. So I don't forget, the configuration line I used was:

./configure CC=/opt/gcc-8.2/bin/gcc CFLAGS='-O3 -flto' LDFLAGS='-flto' \
    AR=/opt/gcc-8.2/bin/gcc-ar \
    RANLIB=/opt/gcc-8.2/bin/gcc-ranlib \
    NM=/opt/gcc-8.2/gcc-nm

I've added a simple fix as branch bug27728, with PR at https://github.com/torproject/tor/pull/354 .

I'm seeing a few other GCC 8.2 warnings on the source code -- I'll try to check whether they're LTO only.

comment:9 Changed 35 hours ago by nickm

Keywords: regression added

comment:10 Changed 35 hours ago by nickm

The other warnings seem to be LTO-only. I'll look to see if any of them are troubling.

comment:11 Changed 35 hours ago by nickm

Opened #27772 for the other warnings

Note: See TracTickets for help on using tickets.