On Tue, Mar 03, 2026 at 07:08:25PM -0600, Yodel Eldar wrote: > +Daniel (thanks for your comments) > > On 02/03/2026 19:20, Yodel Eldar wrote: > > From: Yodel Eldar <[email protected]> > > > > Builds with --enable-{asan,tsan,safe-stack} fail under GCC, so use > > clang if available, otherwise disable the treatment of warnings as > > errors. > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3006 > > Suggested-by: Peter Maydell <[email protected]> > > Signed-off-by: Yodel Eldar <[email protected]> > > --- > > Hi, > > > > The previous version only disabled Werror whenever `--skip-meson` wasn't > > used and the build occurred in a git repo, but this change should > > probably apply to all types of builds. So, let's use meson_option_add > > to globally disable Werror instead; IIUC (and according to my testing), > > this will override the value in config-meson.cross.new. > > > > I'm still not sure if we should be disabling Werror for ubsan, even > > though it's not currently breaking builds with GCC; please let me know > > what you think. > > > > Special thanks to Peter for looking into the cause of the reports around > > this, for sharing the findings, and suggesting approaches to resolve it. > > I couldn't pick one over the other, so I went with using clang when > > available with Werror disable as a fallback; please let me know if you > > think this is an XOR kind of policy decision. > > > > Link to RFCv1: > > https://lore.kernel.org/qemu-devel/[email protected]/ > > > > Link to mentioned discussion: > > https://lore.kernel.org/qemu-devel/cafeaca88hc4usgpupxbwpben0tw26159kpn7jx2j9erba5d...@mail.gmail.com/ > > > > v2: > > - Fix misnomer in commit message > > - Simplify condition by using the same variable for all sanitizers > > - Use meson_option_add to disable Werror > > > > Thanks, > > Yodel > > --- > > configure | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/configure b/configure > > index 5e114acea2..e457e8a17d 100755 > > --- a/configure > > +++ b/configure > > @@ -762,6 +762,12 @@ for opt do > > ;; > > --wasm64-32bit-address-limit) > > ;; > > + --enable-asan) use_sanitizer="yes" > > + ;; > > + --enable-tsan) use_sanitizer="yes" > > + ;; > > + --enable-safe-stack) use_sanitizer="yes" > > + ;; > > # everything else has the same name in configure and meson > > --*) meson_option_parse "$opt" "$optarg" > > ;; > > @@ -771,6 +777,18 @@ for opt do > > esac > > done > > +if test "$use_sanitizer" = "yes"; then > > + if has clang; then > > + echo "Sanitizer requested: setting compiler suite to clang" > > + cc=clang > > + cxx=clang++ > > + host_cc=clang > > + else > > + echo "Sanitizer requested: disabling Werror for non-clang > > compilers" > > + meson_option_add -Dwerror=false > > + fi > > +fi > > + > > if ! test -e "$source_path/.git" > > then > > git_submodules_action="validate" > > We could treat the rtl8139 as a one-off by carving out the > offending code with a GCC pragma or finagling GCC into cooperation > by substituting eth_payload_data with the expression assigned to it > (thank you, Thomas and Daniel, respectively); indeed, AFAICT the > rtl8139 is currently the only code that triggers the GCC bug > (and is overdue for a refactor/cleanup), so it's tempting to go with > either of these helpful suggestions; *however*, until the GCC team > fixes their buggy pairing between sanitizer and -Werror > (a pairing that they themselves disavow [1]), IMHO there's a > significant risk of recurrence if we went with either option, and > it's not clear to me whether reviewers will be able easily spot the > next one before it's too late (post-acceptance).
There is a risk, but given that we've only got 1 example across the QEMU codebase, I don't think I'd classify that as significant. If we already had 10+ locations where it hit, then that would be a different story As long as the number of situations where we hit this in QEMU remains low (say < 5), then I think a targetted use of Pragmas to temp disable warnings is satisfactory to deal with it. > I'm definitely onboard with Daniel's warning message in meson whenever > the user: 1) explicitly opts for gcc/g++, 2) enables sanitizers, and 3) > -Werror is enabled. At the risk of overengineering, though, what if we > made it an error message instead, and gave users an escape hatch like > `--force-werror-sanitizers` (name TBD)? I can see that being too much, > but it may prevent some grief if they missed the warning, and the build > breaks several minutes later. WDYT? It's not included in the diff below, > but I'll gladly add it to v3 if there's interest; if not, I'll go with > the warning message as suggested. > > Thanks, > Yodel > > [1] https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html (Peter) > [2] > https://gitlab.com/qemu-project/qemu/-/blob/master/configure?ref_type=heads#L302-315 > > @@ -286,15 +292,25 @@ static="no" > # Preferred compiler: > # ${CC} (if set) > # ${cross_prefix}gcc (if cross-prefix specified) > -# system compiler > +# clang if sanitizer requested, otherwise system compiler > if test -z "${CC}${cross_prefix}"; then > - cc="cc" > + if test "$use_sanitizers" = "yes" && has clang; then > + cc=clang > + echo "Sanitizer requested: setting cc to clang" I really don't want to see us changing compiler choice as a side effect of enabling sanitizers. With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
