-
Notifications
You must be signed in to change notification settings - Fork 38.7k
test: fix usdt undeclared function errors on mantis #28629
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
Recently usage of undeclared functions became an error rather than a warning, in C2x. https://reviews.llvm.org/D122983?id=420290 This change has migrated into the build tools of Ubuntu 23.10 which now causes the USDT tests to fail to compile, see bitcoin#28600 Fix this by setting `-Wno-error=implicit-function-declaration` for the tracing programs.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. 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. |
Is there an upstream issue tracking this? |
Not to my knowledge. The closest is iovisor/bcc#4740 I'll open one over there (and perhaps get clarification that this is something that they can/will fix on their side). |
|
OK I opened iovisor/bcc#4757 |
|
lgtm ACK 4077e43 CI passed on this, and it should be trivial to revert this temporary workaround. I may test after merge. |
|
Seems fine for now as a workaround. Can follow up if there is activity upstream. |
4077e43 test: fix usdt undeclared function errors on mantis (willcl-ark) Pull request description: This is one way to fix bitcoin#28600 Recently usage of undeclared functions became an error rather than a warning, in C2x. https://reviews.llvm.org/D122983?id=420290 This change has migrated into the build tools of Ubuntu 23.10 which now causes the USDT tests to fail to compile, see bitcoin#28600 I think there are various potential fixes: 1. Manually declare the functions we use 2. Fix imports so that manual declarations aren't needed 3. Revert the new C2X behaviour and don't error on implicit function declarations I would have preferred solution 2, but I believe this will require changes to the upstream bcc package. Having played with the imports I can get things working in a standalone C program, using system headers, but when building the program from a python context as we do in the test it uses its own headers (bundled with the python lib) rather than the system ones, and manually importing (some) system headers results in definition mismatches. I also investigated explicitly importing required headers from the package, which use paths like `#import </virtual/bcc/bcc_helpers.h>`, but this seems more obtuse and brittle than simply ignoring the warning. Therefore I think that until the upstream python pacakge fixes their declarations, we should fix this by setting `-Wno-error=implicit-function-declaration` for the tracing programs. cc maflcko 0xB10C ACKs for top commit: maflcko: lgtm ACK 4077e43 Tree-SHA512: 8368bb1155e920a95db128dc893267f8dab64f1ae53f6d63c6d9294e2e4e92bef8515e3697e9113228bedc51c0afdbc5bbcf558c119bf0eb3293dc2ced86b435
|
Post-merge lgtm ACK. Thanks for tackling this. I'll update #25832 with the same flags. |
This is one way to fix #28600
Recently usage of undeclared functions became an error rather than a warning, in C2x. https://reviews.llvm.org/D122983?id=420290
This change has migrated into the build tools of Ubuntu 23.10 which now causes the USDT tests to fail to compile, see
#28600
I think there are various potential fixes:
I would have preferred solution 2, but I believe this will require changes to the upstream bcc package. Having played with the imports I can get things working in a standalone C program, using system headers, but when building the program from a python context as we do in the test it uses its own headers (bundled with the python lib) rather than the system ones, and manually importing (some) system headers results in definition mismatches. I also investigated explicitly importing required headers from the package, which use paths like
#import </virtual/bcc/bcc_helpers.h>, but this seems more obtuse and brittle than simply ignoring the warning.Therefore I think that until the upstream python pacakge fixes their declarations, we should fix this by setting
-Wno-error=implicit-function-declarationfor the tracing programs.cc maflcko 0xB10C