On Tue, May 5, 2020 at 5:47 PM Stefan Schulze Frielinghaus <stefa...@linux.ibm.com> wrote: > > On Tue, May 05, 2020 at 02:58:51PM +0200, Richard Biener wrote: > > On Mon, May 4, 2020 at 4:07 PM Stefan Schulze Frielinghaus > > <stefa...@linux.ibm.com> wrote: > > > > > > On Mon, May 04, 2020 at 03:19:02PM +0200, Richard Biener wrote: > > > > On Mon, May 4, 2020 at 1:44 PM Stefan Schulze Frielinghaus > > > > <stefa...@linux.ibm.com> wrote: > > > > > > > > > > On Tue, Apr 28, 2020 at 08:27:12PM +0200, Stefan Schulze Frielinghaus > > > > > wrote: > > > > > > On Tue, Apr 28, 2020 at 11:33:31AM +0200, Richard Biener wrote: > > > > > > > On Tue, Apr 28, 2020 at 10:03 AM Stefan Schulze Frielinghaus via > > > > > > > Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > > > > > > > > > > > > > In function handle_vector_size_attribute local variable nunits > > > > > > > > is > > > > > > > > supposed to be initialized by function > > > > > > > > type_valid_for_vector_size. > > > > > > > > However, in case ARGS is null the function may return with a > > > > > > > > non-null > > > > > > > > value and leave nunits uninitialized. This results in > > > > > > > > warning/error: > > > > > > > > > > > > > > > > gcc/poly-int.h: In function 'tree_node* > > > > > > > > handle_vector_size_attribute(tree_node**, tree, tree, int, > > > > > > > > bool*)': > > > > > > > > gcc/poly-int.h:330:3: error: 'nunits' may be used uninitialized > > > > > > > > in this function [-Werror=maybe-uninitialized] > > > > > > > > 330 | ((void) (&(RES).coeffs[0] == (C *) 0), \ > > > > > > > > | ^ > > > > > > > > gcc/c-family/c-attribs.c:3695:26: note: 'nunits' was declared > > > > > > > > here > > > > > > > > 3695 | unsigned HOST_WIDE_INT nunits; > > > > > > > > | > > > > > > > > > > > > > > > > This is fixed by also checking whether ARGS is null or not. > > > > > > > > > > > > > > > > Bootstrapped and regtested on S/390. Ok for master? > > > > > > > > > > > > > > I think it's better to assert that it is not null for example by > > > > > > > adding a > > > > > > > nonnull attribute? Can you check if that works? If it doesn't > > > > > > > the > > > > > > > patch is OK. > > > > > > > > > > > > Yes, that works, too. Please find an updated version attached. If > > > > > > you > > > > > > think it is useful I could also add a gcc_assert (!args) for minimal > > > > > > testing. > > > > > > > > > > I guess initializing nunits to zero does not really solve the problem > > > > > and while failing to identify all call sides (which isn't future-proof > > > > > anyway), I went with adding a checking assert statement. Bootstrapped > > > > > and regtested on S/390. Ok for master? > > > > > > > > You now have both, the atttribute and the assert. The assert alone is > > > > OK. > > > > > > The assert alone is not enough. For a release build we would again run > > > into a warning. Of course we could change gcc_checking_assert into > > > gcc_assert. However, then at least with no checking at all we would run > > > again into the same warning. > > > > I see. > > > > > Using only the attribute in order to silence the warning, then we should > > > be very clear that there exists no call side where ARGS equals NULL and > > > where type_valid_for_vector_size returns a non-null value. I was not > > > able to show this. Thus I went for both: attribute+assert. Since the > > > attribute is local to the compilation unit, the compiler has no chance > > > to enforce this restriction at call sides. If you think this will never > > > happen, then we could just use patch from > > > https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544774.html > > > or the very initial patch which just checks for args being NULL > > > https://gcc.gnu.org/pipermail/gcc-patches/2020-April/544691.html > > > > > > What do you prefer? > > > > The last posted patch with the assert and the attribute is OK then. > > Pushed it to master. Is this Ok for releases/gcc-10 branch, too?
Please wait until after the 10.1 release for these changes, they are harmless since in release mode we do not use -Werror. Richard. > Thanks, Stefan
