From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62b.google.com (mail-ej1-x62b.google.com [IPv6:2a00:1450:4864:20::62b]) by sourceware.org (Postfix) with ESMTPS id 69E0F3858C56 for ; Mon, 28 Mar 2022 08:29:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 69E0F3858C56 Received: by mail-ej1-x62b.google.com with SMTP id lr4so18610642ejb.11 for ; Mon, 28 Mar 2022 01:29:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=1fbN2S+/bx74Dbq2RAcB5DDavKC6F1h+y+llD8ffQ+Y=; b=LMmUtdRIKw8659j2SUgwEnEeHBuRVK8hdK7KPHSX0i5VCtA4BYXYqgjmRDfvnQRT99 6Wg0SnTWs+S/iRPVtsr4c50C5b/SAfpdW+kf94cPNN9Yv0ALWrAVOZ+YgcBIrQsjkIvR ScDBC7CzYzEcUEkBA9RFeBuRD+WLbrD6G8wWIftwLZ4cRoEx+SfvyfwGg0+4aYmS6T3w fMomh6tVrkbsMR2iVsEfpOSZoTMZIHiZHxnNwRVeioHYt1vPqQIy6cjAZDsTw5CziH5Y UAqtTno4uvcCMRl4XqxXuV1dN/N9kT9Nx/NsyBFx+WgSJcpm33JQBGieXbNgEAnwLddi NNsw== X-Gm-Message-State: AOAM533x3qGBOLIFxC0BecwW5U+dRapTyF2Nky6QmOaw1ORwhZ42b+fz KBYBoXr8BrWI1CG/W3qMTNRjhu1qstsliXsXE2Y= X-Google-Smtp-Source: ABdhPJxKNwB/7szlOGhjB8dTNCETwlYyRwRNSvb2vQO1WADOVuC6ReALAVSgnNik+A7MkZpgAYTFyoaJFpkUOFk3hi8= X-Received: by 2002:a17:906:9e11:b0:6df:a9d8:cbad with SMTP id fp17-20020a1709069e1100b006dfa9d8cbadmr26210418ejc.32.1648456151795; Mon, 28 Mar 2022 01:29:11 -0700 (PDT) MIME-Version: 1.0 References: <20220325212643.773729-1-dmalcolm@redhat.com> In-Reply-To: <20220325212643.773729-1-dmalcolm@redhat.com> From: Richard Biener Date: Mon, 28 Mar 2022 10:29:00 +0200 Message-ID: Subject: Re: [PATCH] gimple-fold: fix location of loads for memory ops [PR104308] To: David Malcolm Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Mar 2022 08:29:20 -0000 On Fri, Mar 25, 2022 at 10:27 PM David Malcolm via Gcc-patches wrote: > > PR analyzer/104308 reports that when -Wanalyzer-use-of-uninitialized-valu= e > complains about certain memmove operations where the source is > uninitialized, the diagnostic uses UNKNOWN_LOCATION: > > In function 'main': > cc1: warning: use of uninitialized value '*(short unsigned int *)&s + 1' = [CWE-457] [-Wanalyzer-use-of-uninitialized-value] > 'main': event 1 > | > |pr104308.c:5:8: > | 5 | char s[5]; /* { dg-message "region created on stack here" = } */ > | | ^ > | | | > | | (1) region created on stack here > | > 'main': event 2 > | > |cc1: > | (2): use of uninitialized value '*(short unsigned int *)&s + 1' her= e > | > > The issue is that gimple_fold_builtin_memory_op converts a memmove to: > > _3 =3D MEM [(char * {ref-all})_1]; > MEM [(char * {ref-all})&s] =3D _3; > > but only sets the location of the 2nd stmt, not the 1st. > > Fixed thusly, giving: > > pr104308.c: In function 'main': > pr104308.c:6:3: warning: use of uninitialized value '*(short unsigned int= *)&s + 1' [CWE-457] [-Wanalyzer-use-of-uninitialized-value] > 6 | memmove(s, s + 1, 2); /* { dg-warning "use of uninitialized val= ue" } */ > | ^~~~~~~~~~~~~~~~~~~~ > 'main': events 1-2 > | > | 5 | char s[5]; /* { dg-message "region created on stack here" = } */ > | | ^ > | | | > | | (1) region created on stack here > | 6 | memmove(s, s + 1, 2); /* { dg-warning "use of uninitialize= d value" } */ > | | ~~~~~~~~~~~~~~~~~~~~ > | | | > | | (2) use of uninitialized value '*(short unsigned int *)&s = + 1' here > | > > One side-effect of this change is a change in part of the output of > gcc.dg/uninit-40.c from: > > uninit-40.c:47:3: warning: =E2=80=98*(long unsigned int *)(&u[1][0][0])= =E2=80=99 is used uninitialized [-Wuninitialized] > 47 | __builtin_memcpy (&v[1], &u[1], sizeof (V)); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > uninit-40.c:45:5: note: =E2=80=98*(long unsigned int *)(&u[1][0][0])=E2= =80=99 was declared here > 45 | V u[2], v[2]; > | ^ > > to: > > uninit-40.c:47:3: warning: =E2=80=98u=E2=80=99 is used uninitialized [-= Wuninitialized] > 47 | __builtin_memcpy (&v[1], &u[1], sizeof (V)); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > uninit-40.c:45:5: note: =E2=80=98u=E2=80=99 declared here > 45 | V u[2], v[2]; > | ^ > > What's happening is that pass "early_uninit"(29)'s call to > maybe_warn_operand is guarded by this condition: > 1051 else if (gimple_assign_load_p (stmt) > 1052 && gimple_has_location (stmt)) > > Before the patch, the stmt: > _3 =3D MEM [(char * {ref-all})&u + 8B]; > has no location, and so early_uninit skips this operand at line > 1052 above. Later, pass "uninit"(217) tests the var_decl "u$8", and > emits a warning for it. > > With the patch, the stmt has a location, and so early_uninit emits a > warning for "u" and sets a NW_UNINIT warning suppression at that > location. Later, pass "uninit"(217)'s test of "u$8" is rejected > due to that per-location suppression of uninit warnings, from the > earlier warning. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > OK for stage 4? or for next stage 1? OK for stage4. Thanks, Richard. > gcc/ChangeLog: > PR analyzer/104308 > * gimple-fold.cc (gimple_fold_builtin_memory_op): When optimizing > to loads then stores, set the location of the new load stmt. > > gcc/testsuite/ChangeLog: > PR analyzer/104308 > * gcc.dg/analyzer/pr104308.c: New test. > * gcc.dg/uninit-40.c (foo): Update expression in expected message= . > > Signed-off-by: David Malcolm > --- > gcc/gimple-fold.cc | 1 + > gcc/testsuite/gcc.dg/analyzer/pr104308.c | 8 ++++++++ > gcc/testsuite/gcc.dg/uninit-40.c | 2 +- > 3 files changed, 10 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr104308.c > > diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc > index 5eff7d68ac1..e73bc6a7137 100644 > --- a/gcc/gimple-fold.cc > +++ b/gcc/gimple-fold.cc > @@ -1039,6 +1039,7 @@ gimple_fold_builtin_memory_op (gimple_stmt_iterator= *gsi, > new_stmt); > gimple_assign_set_lhs (new_stmt, srcmem); > gimple_set_vuse (new_stmt, gimple_vuse (stmt)); > + gimple_set_location (new_stmt, loc); > gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT= ); > } > if (dest_align < GET_MODE_ALIGNMENT (mode)) > diff --git a/gcc/testsuite/gcc.dg/analyzer/pr104308.c b/gcc/testsuite/gcc= .dg/analyzer/pr104308.c > new file mode 100644 > index 00000000000..9cd5ee6feee > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/analyzer/pr104308.c > @@ -0,0 +1,8 @@ > +#include > + > +int main() > +{ > + char s[5]; /* { dg-message "region created on stack here" } */ > + memmove(s, s + 1, 2); /* { dg-warning "use of uninitialized value" } *= / > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/uninit-40.c b/gcc/testsuite/gcc.dg/unin= it-40.c > index 8708079d397..567707a885e 100644 > --- a/gcc/testsuite/gcc.dg/uninit-40.c > +++ b/gcc/testsuite/gcc.dg/uninit-40.c > @@ -44,7 +44,7 @@ foo (int *q) > /* memcpy folding is too target dependent to test it everywhere. */ > V u[2], v[2]; > u[0][0][0] =3D 1; > - __builtin_memcpy (&v[1], &u[1], sizeof (V)); /* { dg-warning "= '\\*\\(\(long \)?long unsigned int \\*\\)\\(&u\\\[1\\\]\\\[0\\\]\\\[0\\\]\\= )' is used uninitialized" "" { target i?86-*-* x86_64-*-* } } */ > + __builtin_memcpy (&v[1], &u[1], sizeof (V)); /* { dg-warning "= 'u' is used uninitialized" "" { target i?86-*-* x86_64-*-* } } */ > baz (&v[1]); > #endif > } > -- > 2.26.3 >