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. Thanks, Richard.
