-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Regression: systemd is failing to compile with clang-{10,11,12} with --optimization=3 -Db_lto=true #8347
Comments
The cause of the error is pretty obvious:
It's listed as supported at https://clang.llvm.org/docs/ClangCommandLineReference.html though... apparently it is "unused" if you use the same arguments during compile and link? The release notes for meson 0.57 specifically mentioned this change, and referenced @dcbaker should this be moved to |
@eli-schwartz thanks for looking into this!
I opened the issue because I think Regarding -Wunused-command-line-argument, it seems to be used by default when |
I'll get a fix our for this tomorrow PST, should be an easy one |
@dcbaker I wonder if it would be possible to update the regression tests that, as far as I know, are run every time
? |
Forgot to mention that |
We probably should be running our CI with the "no unused arguments" check on, for obvious reasons, which is what failed here. without that we wouldn't have caught this at all anyway. |
@evverx We are trying to catch those regressions earlier by doing RC releases, we would appreciate if more people could test future Meson releases in advance to avoid this kind of issue. If you wish to help us, please follow |
Clang has a hand `-Wunused-command-line-argument` switch, which when turned to an error, gets very grump about `-flto-jobs=0` being set in the compiler arguments (although `-flto=` belongs there). We'll refactor a bit to put that only in the link arguments. GCC doesn't have this probably because, a) it doesn't have an equivalent warning, and b) it uses `-flto=<$numthreads. Fixes: mesonbuild#8347
Clang has a hand `-Wunused-command-line-argument` switch, which when turned to an error, gets very grump about `-flto-jobs=0` being set in the compiler arguments (although `-flto=` belongs there). We'll refactor a bit to put that only in the link arguments. GCC doesn't have this probably because, a) it doesn't have an equivalent warning, and b) it uses `-flto=<$numthreads. Fixes: mesonbuild#8347
@xclaesse While I think it's somewhat helpful in terms of catching bugs earlier I think it would be much better if the meson project could try building large projects like |
https://github.com/jon-turney/meson-corpus-test systemd is being run here, but this will probably only tend to find issues that intersect with your meson.build files... the systemd CI is testing various intersections with meson options so I'm not sure merely trying to compile systemd would have helped here. |
@eli-schwartz I'm not sure how exactly |
@evverx it's difficult to smoke test a variety of projects we have no knowledge about. Each project requires different dependencies, env, platforms, etc... I've been thinking about this a bit, what I realize is most regressions are found when CI of some projects does |
I think it should be doable. I don't think the systemd CI (which contains CentOS CI, packit, Ubuntu/Debian Autopkgtests, CIFuzz, various GH actions and so on and so forth) can be triggered easily that way but I guess it should be possible to gather all systemd use cases and put them into a pipeline of some kind. Could you tell me what the meson project usually uses to run long (pre) release tests? cc @mrc0mmand. |
Another option would be for a meson bot to open PRs against the systemd repository on GitHub with |
We should open a dedicated issue for discussing ci changes, so it doesn't get lost when this issue is closed |
Maybe adding something similar to our build test as another PR GH action here would make sense as well. Just use the default gcc/clang versions instead of the custom version shenanigans and slightly change the build scenarios to cover the "problematic" cases, i.e.: ARGS=(
"--optimization=0"
"--optimization=3 -Db_lto=true"
"--optimization=3 -Db_lto=false"
"-Dc_args='-fno-omit-frame-pointer -ftrapv'"
...
) The builds are fairly quick, and to cover the "who should be CCed" case, adding a note somewhere to ping either me or @evverx (if he agrees) should help. |
@mrc0mmand I think another GH Action should be good enough to more or less cover systemd (though I'd also add OSS-Fuzz builds that always trigger all sorts of issues in meson) but to judge from #8351 (comment) it seems opening PRs against various projects to trigger their CI would be more helpful in general. |
Clang has a hand `-Wunused-command-line-argument` switch, which when turned to an error, gets very grump about `-flto-jobs=0` being set in the compiler arguments (although `-flto=` belongs there). We'll refactor a bit to put that only in the link arguments. GCC doesn't have this probably because, a) it doesn't have an equivalent warning, and b) it uses `-flto=<$numthreads. Fixes: #8347
One of the reasons for this is that OSS-Fuzz refuses to use Meson's builtin options for sanitizers but instead spams CPPFLAGS, LDFLAGS et al with tens of random flags without any rhyme or reason. It's not surprising that it fails, it's surprising that it works at all. |
@jpakkane according to #4542 (comment), the reason OSS-Fuzz uses all those environment variables is to be compatible with different build systems. |
Specifically they want to be compatible with the largest amount of build systems by having just one thing that works the same everywhere. The downside of this is that they offload all work to downstream maintainers like us by saying "it works for other build systems" and have us fix the thing by trying to undo their environment butchering. A proper solution would be to add functionality like this to their end:
This might require us to add some functionality to Meson (I think they use a combination of asan + ubsan, which is not currently supported) and we'd be happy to fix that rather than play the current continuous compiler flag whack-a-mole. |
OSS-Fuzz is not the only massively heterogeneous environment relying on this standard mechanism, and we shouldn't be shaming them for detecting real bugs in something that meson does specifically support for entirely valid reasons. |
@jpakkane given that only 15 projects (out of 422) use meson there I think if I were an OSS-Fuzz maintainer I wouldn't want to add and maintain that "if" clause either. But, to be fair, when it makes sense the OSS-Fuzz project either fixes or helps to fix incompatibilities of various tools with meson. Thanks to them all the projects using meson now can be built with AFL++/afl-clang-fast: google/oss-fuzz#5084 |
Clang has a hand `-Wunused-command-line-argument` switch, which when turned to an error, gets very grump about `-flto-jobs=0` being set in the compiler arguments (although `-flto=` belongs there). We'll refactor a bit to put that only in the link arguments. GCC doesn't have this probably because, a) it doesn't have an equivalent warning, and b) it uses `-flto=<$numthreads. Fixes: #8347
This reverts commit c39e362. Now that meson-0.57.1 (where mesonbuild/meson#8347 is fixed) is out it should be safe to keep rolling forward.
This reverts commit c39e362. Now that meson-0.57.1 (where mesonbuild/meson#8347 is fixed) is out it should be safe to keep rolling forward.
Clang has a hand `-Wunused-command-line-argument` switch, which when turned to an error, gets very grump about `-flto-jobs=0` being set in the compiler arguments (although `-flto=` belongs there). We'll refactor a bit to put that only in the link arguments. GCC doesn't have this probably because, a) it doesn't have an equivalent warning, and b) it uses `-flto=<$numthreads. Fixes: mesonbuild#8347
Describe the bug
Since meson-0.57.0 was released systemd has been failing to compile with
--optimization=3 -Db_lto=true
withTo Reproduce
The script building systemd can be found at https://github.com/systemd/systemd/blob/main/.github/workflows/build_test.sh. It runs something like
I've opened systemd/systemd#18590 to "fix" it.
The text was updated successfully, but these errors were encountered: