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. 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?
