Opened 6 years ago

Closed 6 years ago

#8587 closed defect (fixed)

Test for curve25519-donna-c64 usability is borken.

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client build
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In #8753, weasel discovered that under some circumstances, we're building curve25519-donna-c64 on 32-bit operating systems. It looks like our order-of-operations is busted in the testing code for whether 128-bit int ops work, and we hadn't noticed because our return-status boolean was inverted.

Child Tickets

Attachments (1)

e9e430403cb70e18ae3c22e47d24c189be8e492c-config.log.gz (17.7 KB) - added by weasel 6 years ago.
config.log

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 years ago by nickm

Status: newneeds_review

There's a possible fix in branch "bug8587"; not yet tested on i386. Needs review and testing.

comment:2 Changed 6 years ago by weasel

Status: needs_reviewnew

e9e430403cb70e18ae3c22e47d24c189be8e492c still tried to build curve25519-donna-c64.c for me.

Changed 6 years ago by weasel

config.log

comment:3 Changed 6 years ago by weasel

FWIW, the curve25519-donna-c64 conftest builds with the -O2 and exits with 0. As soon as I drop the -O2 it no longer builds - segfaults again. Maybe it might be smart to just ignore what my broken compiler thinks.

comment:4 Changed 6 years ago by nickm

Yeowch. So this is a 32-bit compiler with 128-bit emulation that works sometimes and crashes the compiler other times? Argh.

Nonetheless, I still think this patch is Right.

comment:5 Changed 6 years ago by nickm

Status: newneeds_review

comment:6 Changed 6 years ago by nickm

I added an even more assertive fix to that branch as 3c621a2723e2e. Now it will refuse to use donna-c64 if your sizeof(void *) indicates that you aren't a 64-bit platform.

(Yes I know that one might have 64-bit math with a 64x64->128 multiply without having sizeof(void*)==8. Are there platforms like that which I should care about?)

comment:7 in reply to:  6 Changed 6 years ago by rransom

Replying to nickm:

I added an even more assertive fix to that branch as 3c621a2723e2e. Now it will refuse to use donna-c64 if your sizeof(void *) indicates that you aren't a 64-bit platform.

(Yes I know that one might have 64-bit math with a 64x64->128 multiply without having sizeof(void*)==8. Are there platforms like that which I should care about?)

Google Native Client for AMD64 uses ILP32, and their paper on the AMD64 and ARM versions of Native Client proposes that an ILP32 compilation mode should be made available for use in all programs on AMD64.

comment:8 Changed 6 years ago by nickm

Status: needs_reviewneeds_revision

comment:9 Changed 6 years ago by nickm

Status: needs_revisionneeds_review

okay, trying something smarter. It turns out that the reason that 32-bit clang 3.2 accepts the previous test program is that it can do constant folding and variable substitution to determine that the program is equivalent to "return 0". Adding a function seems to fix that.

It appears to work for me.

comment:10 Changed 6 years ago by nickm

And "bug8587_v3" should work with cross-compilation too.

comment:11 Changed 6 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Weasel likes it; merging.

Note: See TracTickets for help on using tickets.