On Mon, Jul 27, 2026 at 01:07:32AM +0400, Marc-André Lureau wrote: > Hi > > On Mon, Jul 27, 2026 at 12:53 AM Michael S. Tsirkin <[email protected]> wrote: > > > > On Mon, Jul 27, 2026 at 12:44:06AM +0400, Marc-André Lureau wrote: > > > When compiled with -Og, gcc produces many false-positives > > > gcc (GCC) 16.1.1 20260515 (Red Hat 16.1.1-2). > > > > it hurts if you do it? so don't do it then? > > We are not far from getting it working, we can accommodate a bit of > code while making it a bit clearer for the reader too.
I don't know, dead code does not make things clear to me. And it burns tokens for LLMs) I simply do not at all cherish to have yet another weird config that will trip up CI and then I am wasting time either "fixing" it myself or wasting even more time communicating with contributors. Used uninitialized warnings is already not great with -O2. How about simply disabling them with -Og? Is -Wno-error=maybe-uninitialized enough? Default build will catch enough errors and that seems enough for me. > > > > > We already use auto-var-init=zero, but better be explicit. > > > > explicit about false positives? > > Explicit initialization I don't really think it's better. Maybe it's required to make the tool work, but it would be even better to avoid the noise. If I see A = 123; I expect it was worth my time to read it. > > > > > Signed-off-by: Marc-André Lureau <[email protected]> > > > --- > > > target/i386/cpu.c | 3 ++- > > > target/i386/emulate/x86_mmu.c | 4 ++-- > > > 2 files changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > > > index 5805d33ab92d..28e4435df23f 100644 > > > --- a/target/i386/cpu.c > > > +++ b/target/i386/cpu.c > > > @@ -7734,7 +7734,7 @@ static void x86_cpuid_get_avx10_version(Object > > > *obj, Visitor *v, > > > static bool x86_cpu_apply_avx10_features(X86CPU *cpu, uint8_t version, > > > Error **errp) > > > { > > > - const AVX10VersionDefinition *def; > > > + const AVX10VersionDefinition *def = NULL; > > > CPUX86State *env = &cpu->env; > > > > > > if (!version) { > > > @@ -7757,6 +7757,7 @@ static bool x86_cpu_apply_avx10_features(X86CPU > > > *cpu, uint8_t version, > > > break; > > > } > > > } > > > + assert(def != NULL); > > > > > > if (def->version < version) { > > > error_setg(errp, "avx10-version can be at most %d", > > > def->version); > > > > > > what does assert have to do with gcc warnings? > > That's what is expected at this point, of course - it is dereferenced 1 line below. what I worry about is people seeing this and blindly adding these asserts all over. > unfortunately the compiler > doesn't care we help it here. I can drop it > > > > diff --git a/target/i386/emulate/x86_mmu.c b/target/i386/emulate/x86_mmu.c > > > index 8d4371467fd7..65bcccd4751e 100644 > > > --- a/target/i386/emulate/x86_mmu.c > > > +++ b/target/i386/emulate/x86_mmu.c > > > @@ -185,8 +185,8 @@ static MMUTranslateResult walk_gpt(CPUState *cpu, > > > target_ulong addr, MMUTranslat > > > int largeness = 0; > > > target_ulong cr3 = x86_read_cr(cpu, 3); > > > uint64_t page_mask = pae ? PAE_PTE_PAGE_MASK : LEGACY_PTE_PAGE_MASK; > > > - MMUTranslateResult res; > > > - > > > + MMUTranslateResult res = MMU_TRANSLATE_PAGE_NOT_MAPPED; > > > + > > > > > > This is not 0 as the commit log implies. > > Initializing with 0 implies defaulting to MMU_TRANSLATE_SUCCESS - not > the best choice here. when I see this I go "ok there is probably an early exit I better remember it" - and then it turns out there isn't and I am left confused. -- MST
