-
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
Fix GCC 12 and Clang 15 compilation errors #14150
Conversation
6b5450a
to
908514f
Compare
97d6249
to
5326547
Compare
d3d8b79
to
3bb5bd5
Compare
@behlendorf @ryao Errors suppressed in this PR are false positives reported by GCC. Unfortunately, new GCC versions bring them along occasionally. |
@szubersk I am already using the newer GCC on Gentoo. I am confused why I am not seeing the same build failures. If we could figure out what is different between Gentoo and Ubuntu, maybe we could find another way of handling this. |
https://bugs.gentoo.org/847148 I thought that the difference might be that Gentoo is not yet using I tried installing a Ubuntu 22.10 userland locally by doing as root:
This should have reproduced at least one of your failures since you posted that the compiler emitted Note that I had to package debootstrap version 1.0.128 for Gentoo locally before it would install Ubuntu 22.10. Some quick commands verify that my local Ubuntu userland is the correct version for 22.10:
I even tried forcing That leaves me with the question: What is special about the build environment on your Ubuntu 22.10 installation that is triggering these build failures? |
@szubersk One thing that stands out to me is that dpkg-query says that I have gcc 4:12.2.0-1ubuntu1, but gcc -v claims to be 12.2.0-3ubuntu1. What version does gcc -v report for you? |
|
In that case, I am not sure what is different. Would you try running the commands I gave for how I made my Ubuntu 22.10 environment and see if this still reproduces for you in that environment? You would need to install debootstrap, but you can substitute a mkdir command in place of the zfs command. Run Edit: It occurs to me that pivot_root will not be happy if you use mkdir instead of the zfs command since it wants an actual mountpoint. You can probably make a bind mount to “mount” another directory at that location to workaround that. That said, pivot_root is just a fancy alternative to chroot, so you could substitute chroot in place of pivot_root and it should be just as good. |
Sure, I'll reproduce the build process exactly as you wrote and let's compare results then. |
@ryao turns out that the differentiator is ASan/UBSan. GCC changes its behavior if those flags are on. When they are not set I can't observe the issue either. Please try changing the
|
It turns out that reproducing this only needs
Presumably, builds with I would like to study this a little more in depth, but my time this week is incredibly tight, so I cannot spend much more time on it right now. However, the inconsistency in My feeling is that both the That said, I would like to request the following changes:
Someone would need to test it to be certain, but I suspect that builds against Linux kernels built with |
It occurs to me that this will miss the case that someone is passing The compiler warnings are harmless unless |
3bb5bd5
to
b8fdaf1
Compare
It's common for sanitizers to affect diagnostics (and whether or not a bug even occurs) because the instrumentation affects code generation a fair bit sometimes. It would be worth reporting a bug to GCC about this if you think the warning is bogus. |
Unfortunately, doing a good bug report against a compiler has significant enough overhead that it is difficult to file reports for every bug. I have discovered this the hard way with LLVM/Clang’s static analyzer. They want minimal test cases and sometimes, when you make a minimal test case, things are wrong in some other way that is just enough for the compiler authors to say that there is no bug, when the compiler clearly has a bug when run on the original code, but your attempt at cutting out all irrelevant code somehow cut out something important for reproducing the bug. Trying to get it right is a tedious process. Another possibility is getting sucked into a long drawn out conversation about the standard where some ridiculous decision by the standard’s committee to label something that clearly has known behavior as undefined results in a heated disagreement over whose code needs to change to correct the bug. I have experienced both scenarios first hand. :/ I do not blame others for not volunteering to go through that process. It is a huge time drain. |
Sure, I'm suggesting it as someone who has been through it several times too. It's taken several days on a single bug before. It doesn't change that we don't want an unbounded number of compiler workarounds building up in ZFS. At the very least should be able to submit a preprocessed version even if not reduced. |
That is a fair point. I would like to find a volunteer to do this too. Sadly, I cannot be that volunteer as I have no spare bandwidth to handle a compiler bug report right now. |
8dbeb67
to
2fc5e21
Compare
2fc5e21
to
4b60f8c
Compare
4b60f8c
to
a01cba6
Compare
Squelch false positives reported by GCC 12 with UBSan. Signed-off-by: szubersk <szuberskidamian@gmail.com>
- Clang 15 doesn't support `-fno-ipa-sra` anymore. Do a separate check for `-fno-ipa-sra` support by $KERNEL_CC. - Don't enable `-mgeneral-regs-only` for certain module files. Fix openzfs#13260 - Scope `GCC diagnostic ignored` statements to GCC only. Clang doesn't need them to compile the code. Signed-off-by: szubersk <szuberskidamian@gmail.com>
a01cba6
to
87120cc
Compare
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.
@szubersk this seems to be ready. If you don't have any additional changes planned I can get it merged.
I'm all ready, merely rebasing after recent surge of merges to master. Thanks @behlendorf ! |
- Clang 15 doesn't support `-fno-ipa-sra` anymore. Do a separate check for `-fno-ipa-sra` support by $KERNEL_CC. - Don't enable `-mgeneral-regs-only` for certain module files. Fix #13260 - Scope `GCC diagnostic ignored` statements to GCC only. Clang doesn't need them to compile the code. Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: szubersk <szuberskidamian@gmail.com> Closes #13260 Closes #14150
Squelch false positives reported by GCC 12 with UBSan. Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: szubersk <szuberskidamian@gmail.com> Closes openzfs#14150
- Clang 15 doesn't support `-fno-ipa-sra` anymore. Do a separate check for `-fno-ipa-sra` support by $KERNEL_CC. - Don't enable `-mgeneral-regs-only` for certain module files. Fix openzfs#13260 - Scope `GCC diagnostic ignored` statements to GCC only. Clang doesn't need them to compile the code. Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: szubersk <szuberskidamian@gmail.com> Closes openzfs#13260 Closes openzfs#14150
Squelch false positives reported by GCC 12 with UBSan. Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: szubersk <szuberskidamian@gmail.com> Closes openzfs#14150
- Clang 15 doesn't support `-fno-ipa-sra` anymore. Do a separate check for `-fno-ipa-sra` support by $KERNEL_CC. - Don't enable `-mgeneral-regs-only` for certain module files. Fix openzfs#13260 - Scope `GCC diagnostic ignored` statements to GCC only. Clang doesn't need them to compile the code. Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: szubersk <szuberskidamian@gmail.com> Closes openzfs#13260 Closes openzfs#14150
- Clang 15 doesn't support `-fno-ipa-sra` anymore. Do a separate check for `-fno-ipa-sra` support by $KERNEL_CC. - Don't enable `-mgeneral-regs-only` for certain module files. Fix openzfs#13260 - Scope `GCC diagnostic ignored` statements to GCC only. Clang doesn't need them to compile the code. Porting notes: - Moved the stanzas removing -mgeneral-regs-only to Makefile.in since they wouldn't readily work in Kbuild.in and that did. Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: szubersk <szuberskidamian@gmail.com> Closes openzfs#13260 Closes openzfs#14150
- Clang 15 doesn't support `-fno-ipa-sra` anymore. Do a separate check for `-fno-ipa-sra` support by $KERNEL_CC. - Don't enable `-mgeneral-regs-only` for certain module files. Fix openzfs#13260 - Scope `GCC diagnostic ignored` statements to GCC only. Clang doesn't need them to compile the code. Porting notes: - Moved the stanzas removing -mgeneral-regs-only to Makefile.in since they wouldn't readily work in Kbuild.in and that did. Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: szubersk <szuberskidamian@gmail.com> Closes openzfs#13260 Closes openzfs#14150 Closes openzfs#14624 Ported-by: Rich Ercolani <rincebrain@gmail.com Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
- Clang 15 doesn't support `-fno-ipa-sra` anymore. Do a separate check for `-fno-ipa-sra` support by $KERNEL_CC. - Don't enable `-mgeneral-regs-only` for certain module files. Fix openzfs#13260 - Scope `GCC diagnostic ignored` statements to GCC only. Clang doesn't need them to compile the code. Porting notes: - Moved the stanzas removing -mgeneral-regs-only to Makefile.in since they wouldn't readily work in Kbuild.in and that did. Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: szubersk <szuberskidamian@gmail.com> Closes openzfs#13260 Closes openzfs#14150 Closes openzfs#14624 Ported-by: Rich Ercolani <rincebrain@gmail.com Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
- Clang 15 doesn't support `-fno-ipa-sra` anymore. Do a separate check for `-fno-ipa-sra` support by $KERNEL_CC. - Don't enable `-mgeneral-regs-only` for certain module files. Fix #13260 - Scope `GCC diagnostic ignored` statements to GCC only. Clang doesn't need them to compile the code. Porting notes: - Moved the stanzas removing -mgeneral-regs-only to Makefile.in since they wouldn't readily work in Kbuild.in and that did. Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: szubersk <szuberskidamian@gmail.com> Closes #13260 Closes #14150 Closes #14624 Ported-by: Rich Ercolani <rincebrain@gmail.com Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
- Clang 15 doesn't support `-fno-ipa-sra` anymore. Do a separate check for `-fno-ipa-sra` support by $KERNEL_CC. - Don't enable `-mgeneral-regs-only` for certain module files. Fix openzfs#13260 - Scope `GCC diagnostic ignored` statements to GCC only. Clang doesn't need them to compile the code. Porting notes: - Moved the stanzas removing -mgeneral-regs-only to Makefile.in since they wouldn't readily work in Kbuild.in and that did. Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: szubersk <szuberskidamian@gmail.com> Closes openzfs#13260 Closes openzfs#14150 Closes openzfs#14624 Ported-by: Rich Ercolani <rincebrain@gmail.com Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Squelch false positives reported by GCC 12 with UBSan. Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: szubersk <szuberskidamian@gmail.com> Closes #14150
Motivation and Context
Make the code compile on Ubuntu 22.10 (Kinetic).
Description
-fno-ipa-sra
anymore. Do a separatecheck for
-fno-ipa-sra
support by $KERNEL_CC.-mgeneral-regs-only
for certain module files.Fix Unable to compile on ARM64 with clang #13260
GCC diagnostic ignored
statements to GCC only. Clangdoesn't need them to compile the code.
How Has This Been Tested?
make
Types of changes
Checklist:
Signed-off-by
.