-
Notifications
You must be signed in to change notification settings - Fork 37.7k
suppressions: note that type:ClassName::MethodName
should be used
#28147
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.
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
Conversation
Now that the symbolizer is back in play, suppressions can once-again be targeted to functions, rather than file-wide.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
lgtm ACK d0c6cc4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK d0c6cc4
…ame` should be used d0c6cc4 suppressions: note that 'type:ClassName::MethodName' should be used (fanquake) Pull request description: Now that the symbolizer is back in play, suppressions can once-again be targeted to functions, rather than file-wide. ACKs for top commit: MarcoFalke: lgtm ACK d0c6cc4 hebasto: ACK d0c6cc4 Tree-SHA512: fb65398eae18a6ebc5f8414275c568cf2664ab5357c2b3160f3bf285b67bc3af788225c5dba3c824c0e098627789450bec775375f52529d71c6ef700a9632d65
Not sure why the symbolizer doesn't work on Fedora, but I guess this should be reverted? Steps to reproduce on a fresh install of Fedora 39:
|
Same on OpenSuse Tumbleweed:
|
It's ont completely clear to me why this needs to be explicitly specified in some environments, and not in others, also at the same time that llvm-symbolizer is already in PATH, but this has fixed the 2 issues outlined in bitcoin#28147 for me.
It's not completely clear to me why this needs to be explicitly specified in some environments, and not in others, while at the same time that `llvm-symbolizer` is already in PATH, but this has fixed the 2 issues outlined in bitcoin#28147. In future, we might be able to use `LLVM_SYMBOLIZER_PATH` instead, however that isn't currently used consistently across LLVM, i.e it's checked for in the asan_symbolize script, but not in in the ubsan script, or from in compiler-rt.
It's not completely clear to me why this needs to be explicitly specified in some environments, and not in others, while at the same time that `llvm-symbolizer` is already in PATH, but this has fixed the 2 issues outlined in bitcoin#28147. In future, we might be able to use `LLVM_SYMBOLIZER_PATH` instead, however that isn't currently used consistently across LLVM, i.e it's checked for in the asan_symbolize script, but not in in the ubsan script, or from in compiler-rt.
It's not completely clear to me why this needs to be explicitly specified in some environments, and not in others, while at the same time that `llvm-symbolizer` is already in PATH, but this has fixed the 2 issues outlined in bitcoin#28147. In future, we might be able to use `LLVM_SYMBOLIZER_PATH` instead, however that isn't currently used consistently across LLVM, i.e it's checked for in the asan_symbolize script, but not in in the ubsan script, or from in compiler-rt.
It's not completely clear to me why this needs to be explicitly specified in some environments, and not in others, while at the same time that `llvm-symbolizer` is already in PATH, but this has fixed the 2 issues outlined in bitcoin#28147. In future, we might be able to use `LLVM_SYMBOLIZER_PATH` instead, however that isn't currently used consistently across LLVM, i.e it's checked for in the asan_symbolize script, but not in in the ubsan script, or from in compiler-rt.
It's not completely clear to me why this needs to be explicitly specified in some environments, and not in others, while at the same time that `llvm-symbolizer` is already in PATH, but this has fixed the 2 issues outlined in bitcoin#28147. Use `LLVM_SYMBOLIZER_PATH` as the env var, as that is somewhat also used inside LLVM, but not consistently, i.e it's checked for in the asan_symbolize script, but not in in the ubsan_symbolize script, or from in compiler-rt.
It's not completely clear to me why this needs to be explicitly specified in some environments, and not in others, while at the same time that `llvm-symbolizer` is already in PATH, but this has fixed the 2 issues outlined in bitcoin#28147. Use `LLVM_SYMBOLIZER_PATH` as the env var, as that is somewhat also used inside LLVM, but not consistently, i.e it's checked for in the asan_symbolize script, but not in in the ubsan_symbolize script, or from in compiler-rt.
It's not completely clear to me why this needs to be explicitly specified in some environments, and not in others, while at the same time that `llvm-symbolizer` is already in PATH, but this has fixed the 2 issues outlined in bitcoin#28147. Use `LLVM_SYMBOLIZER_PATH` as the env var, as that is somewhat also used inside LLVM, but not consistently, i.e it's checked for in the asan_symbolize script, but not in in the ubsan_symbolize script, or from in compiler-rt.
49d9532 fuzz: explicitly specify llvm-symbolizer path in runner (fanquake) Pull request description: It's not completely clear to me why this needs to be explicitly specified in some environments, and not in others, while at the same time that `llvm-symbolizer` is already in PATH, but this has fixed the 2 issues outlined in bitcoin/bitcoin#28147. Use `LLVM_SYMBOLIZER_PATH` as the env var, as that is somewhat also used inside LLVM, but not consistently, i.e it's checked for in the asan_symbolize script, but not in in the ubsan_symbolize script, or from in compiler-rt. Alternative to #28804. ACKs for top commit: maflcko: lgtm ACK 49d9532 Tree-SHA512: c3d5bf1c3629793b342c70754a419b3c7a3cd39f800b9aa69ce3395cc2bf83b4d46f2b329974337b94b99573cd0b8600d3f147ed5c21387bf3812316570d1ee3
Now that the symbolizer is back in play, suppressions can once-again be targeted to functions, rather than file-wide.