Reduce compiler ability to optimise to statisfy gcc 9.5#1442
Merged
torben-hansen merged 2 commits intoaws:mainfrom Feb 20, 2024
Merged
Reduce compiler ability to optimise to statisfy gcc 9.5#1442torben-hansen merged 2 commits intoaws:mainfrom
torben-hansen merged 2 commits intoaws:mainfrom
Conversation
dkostic
previously approved these changes
Feb 19, 2024
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1442 +/- ##
=======================================
Coverage 76.85% 76.86%
=======================================
Files 425 425
Lines 71533 71533
=======================================
+ Hits 54980 54985 +5
+ Misses 16553 16548 -5 ☔ View full report in Codecov by Sentry. |
skmcgrail
previously approved these changes
Feb 19, 2024
WillChilds-Klein
previously approved these changes
Feb 20, 2024
c3160cf
47a9a78 to
c3160cf
Compare
WillChilds-Klein
approved these changes
Feb 20, 2024
dkostic
approved these changes
Feb 20, 2024
WillChilds-Klein
pushed a commit
to WillChilds-Klein/aws-lc
that referenced
this pull request
Feb 21, 2024
gcc 9.5 has a bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95189 that has not been backported. Essentially, the compiler will falsely reason that it's comparing a string and optimise out everything behind a null-character. Dropping static const removes optimisation options for the compiler. In my tests, gcc 9.5 now emits a direct memcmp instead of attempting any optimisations. Add X509_STORE_get1_objects This will be needed for python/cpython#114573. Along the way, document the various functions that expose "query from X509_STORE". Most of them unfortunately leak the weird caching thing that hash_dir does, as well as OpenSSL's generally poor handling of issuers with the same name and CRL lookup, but I don't think it's really worth trying to unexport these APIs. Change-Id: I18137bdc4cbaa4bd20ff55116a18f350df386e4a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65787 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Revert "Add X509_STORE_get1_objects" This reverts commit cd5439a827a29320f7005786a26454904e4b12dc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues:
P118709392
Description of changes:
This PR is an attempt to fix a transient issue with gcc 9.5 compilation. This is a test workflow, so no hot-path performance regression concerns from me.
gcc 9.5 has a bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95189 that has not been backported. Essentially, the compiler will falsely reason that it's comparing a string and optimise out everything behind a null-character.
In the test fixture
SSLVersionTest.DefaultTicketKeyInitializationit appears the gcc 9.5 compiler only compares the first byte and short-circuits there. What was observed:Dropping
static constremoves optimisation options for the compiler. In my tests, gcc 9.5 now emits a directmemcmpinstead of attempting any optimisations:Testing:
Tested on machines with gcc 9.5 runtime. Before failed, after this patch, no failure (even when forcing first byte to 0).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.