Opened 2 years ago

Closed 14 months ago

Last modified 13 months ago

#14821 closed defect (fixed)

--enable-expensive-hardening broken with clang

Reported by: Sebastian Owned by:
Priority: High Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 027-backport, 2016-bug-retrospective
Cc: gk Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


When passing -fsanitize=address, clang internally defines _FORTIFY_SOURCE=0. This conflicts with the _FORTIFY_SOURCE=2 we like to set beforehand, so we don't enable -fsanitize=address at all. Perhaps we should try to use fsanitize-address, if it fails try again with _FORTIFY_SOURCE remoeved, and only then resolve that it isn't available.

I am calling this 0.2.7 territory for now, but if the fix turns out to be easy we could conceivably backport it.

Child Tickets

Change History (14)

comment:2 Changed 2 years ago by Sebastian

Ok, my idea above was not a good one. We should prefer FORTIFY_SOURCE over AddressSanitizer. Will have to use gcc I guess unless there's a great idea for this somewhere

comment:3 Changed 2 years ago by nickm

Is FORTIFY_SOURCE really better than asan?

comment:4 Changed 2 years ago by Sebastian

According to the link it is, and I thought deferring to the asan people on this was smart.

comment:5 Changed 2 years ago by gk

  • Cc gk added

comment:6 Changed 2 years ago by Sebastian

I'm starting to re-evaluate my opinion if we shouldn't just get rid of FORTIFY_SOURCE=2 if it breaks -fsanitize=address. I've recently patched my configure script to allow just that very frequently to track down a bunch of bugs. In production builds which don't use expensive hardening we still have the FORTIFY_SOURCE

comment:7 Changed 2 years ago by nickm

  • Status changed from new to assigned

comment:8 Changed 2 years ago by nickm

  • Keywords 027-triaged-1-out added

Marking triaged-out items from first round of 0.2.7 triage.

comment:9 Changed 2 years ago by nickm

  • Milestone changed from Tor: 0.2.7.x-final to Tor: 0.2.???

Make all non-needs_review, non-needs_revision, 027-triaged-1-out items belong to 0.2.???

comment:10 Changed 16 months ago by nickm

  • Milestone changed from Tor: 0.2.??? to Tor: 0.2.8.x-final
  • Priority changed from Medium to High
  • Severity set to Normal

Given the bugginess of some of GCC's hardening options, I think, we should look harder at this.

comment:11 Changed 14 months ago by nickm

  • Keywords 027-backport added; 027-triaged-1-out removed

ug, apparently clang 3.7 broke everybody by messing up autotools builds, so that clang only builds its sanitizers correctly with cmake. So first I had to fix that on my desktop.

Then I managed to figure out that the only compilation problems were related to strcmp() and to some parenthesis in test_util.

And then I realized that it's going to complain a lot about leaks in the tests unless I say ASAN_OPTIONS=detect_leaks=0 .

And I found a couple of actual bugs with the tests, so I'm going to open tickets for those. But for now, the branch bug14821 could be what we need here. The part of this not applying to test_util.c could be an 0.2.7 backport but I'm a bit nervous.

comment:12 Changed 14 months ago by nickm

  • Status changed from assigned to needs_review

comment:13 Changed 14 months ago by nickm

  • Resolution set to fixed
  • Status changed from needs_review to closed

comment:14 Changed 13 months ago by nickm

  • Keywords 2016-bug-retrospective added

Mark more tickets for severe bug retrospective, based on Priority and date and hand-inspection.

Note: See TracTickets for help on using tickets.