Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

crypto: disable asan for sha256_sse4 with clang and -O0 #30097

Merged
merged 1 commit into from
May 16, 2024
Merged

crypto: disable asan for sha256_sse4 with clang and -O0 #30097

merged 1 commit into from
May 16, 2024

Conversation

theuni
Copy link
Member

@theuni theuni commented May 13, 2024

Clang is unable to compile the Transform function for that combination of options.

Fixes #29801.

@DrahtBot
Copy link
Contributor

DrahtBot commented May 13, 2024 •
edited

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK achow101
Concept ACK fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@theuni theuni mentioned this pull request May 13, 2024
Closed
1 task
@theuni
Copy link
Member Author

theuni commented May 13, 2024

Ping @achow101. Sorry this took so long!

@fanquake
Copy link
Member

Concept ACK - Think I prefer fixing this inline, than in global flags / build. I guess we didn't end up making an issue upstream (https://github.com/llvm/llvm-project/issues)? If there is something to link to, would be good to add it here.

@fanquake
Copy link
Member

Checked this (./configure --enable-debug --with-sanitizers=address CC=clang CXX=clang++) works with Ubuntu clang version 18.1.3 (1) on x86_64.

@theuni theuni mentioned this pull request May 14, 2024
Open
@theuni
Copy link
Member Author

theuni commented May 14, 2024

Concept ACK - Think I prefer fixing this inline, than in global flags / build. I guess we didn't end up making an issue upstream (https://github.com/llvm/llvm-project/issues)? If there is something to link to, would be good to add it here.

Upstream issue filed here: llvm/llvm-project#92182

@fanquake
Copy link
Member

Upstream issue filed here: llvm/llvm-project#92182

Thanks. Could you like to that thread from the comment added to the source, then this is probably good to go.

@theuni
crypto: disable asan for sha256_sse4 with clang and -O0
141df0a 
Clang is unable to compile the Transform function for that combination of
options.
@theuni
Copy link
Member Author

theuni commented May 15, 2024

Could you like to that thread from the comment added to the source, then this is probably good to go.

Done

@achow101
Copy link
Member

ACK 141df0a

It builds 🎉

@DrahtBot DrahtBot requested a review from fanquake May 15, 2024 18:33
@fanquake fanquake merged commit ae2658c into bitcoin:master May 16, 2024
16 checks passed
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request May 16, 2024
@theuni @fanquake
crypto: disable asan for sha256_sse4 with clang and -O0
3c26058 
Clang is unable to compile the Transform function for that combination of
options.

Github-Pull: bitcoin#30097
Rebased-From: 141df0a
@fanquake fanquake mentioned this pull request May 16, 2024
Merged
@fanquake
Copy link
Member

Backported to 27.x in #30092.

fanquake added a commit that referenced this pull request May 29, 2024
@fanquake
Merge #30092: [27.x] Backports and rc1
fccd32e 
22701a4 doc: update manual pages for 27.1rc1 (fanquake)
9e91907 build: bump version to 27.1rc1 (fanquake)
9b4640c doc: update release-notes.md for 27.1 (fanquake)
80032d6 qt: 27.1rc1 translations update (Hennadii Stepanov)
423bd6d windeploy: Renew certificate (Ava Chow)
77b2321 depends: Fetch miniupnpc sources from an alternative website (Hennadii Stepanov)
31adcfa test: add GetAddedNodeInfo() CJDNS regression unit test (Jon Atack)
9cdb9ed p2p, bugfix: detect addnode cjdns peers in GetAddedNodeInfo() (Jon Atack)
3c26058 crypto: disable asan for sha256_sse4 with clang and -O0 (Cory Fields)
0ba11cf rpc: move UniValue in blockToJSON (willcl-ark)
dedf319 gui: don't permit port in proxy IP option (willcl-ark)
d1289a1 gui: fix create unsigned transaction fee bump (furszy)

Pull request description:

  Backports:
  * bitcoin-core/gui#812
  * bitcoin-core/gui#813
  * #30085
  * #30094
  * #30097
  * #30149
  * #30151

  Bump to 27.1rc1.

ACKs for top commit:
  stickies-v:
    re-ACK 22701a4
  willcl-ark:
    reACK 22701a4
  hebasto:
    re-ACK 22701a4.

Tree-SHA512: 6eca44ba7e6664eb4677646597dfdaf56a241c8c3e95e0ab8929ee2fc3671303fc6c2634d359b4523dbd452ac5e54fd1f4c7c2bf7e9c5209395f8cb3b4753fb3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fanquake fanquake Awaiting requested review from fanquake

Assignees
No one assigned
Labels
Utils/log/libs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Compilation failure with -O0 + -fsanitize=address due to inline asm
4 participants
@theuni @DrahtBot @fanquake @achow101