| Message ID | 20200428071833.925638-1-stefansf@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | c-attribs.c: Fix use of uninitialized variable nunits | expand |
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. Thanks, Richard. > gcc/c-family/ChangeLog: > > 2020-04-28 Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> > > * c-attribs.c (handle_vector_size_attribute): Fix use of > unintialized variable nunits. > --- > gcc/c-family/c-attribs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c > index ac936d5bbbb..a8992e76755 100644 > --- a/gcc/c-family/c-attribs.c > +++ b/gcc/c-family/c-attribs.c > @@ -3694,7 +3694,7 @@ handle_vector_size_attribute (tree *node, tree name, tree args, > the number of vector units. */ > unsigned HOST_WIDE_INT nunits; > type = type_valid_for_vector_size (type, name, args, &nunits); > - if (!type) > + if (!type || !args) > return NULL_TREE; > > tree new_type = build_vector_type (type, nunits); > -- > 2.25.3 >
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. 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; | Added attribute nonnull for argument args in order to state the invariant and to silence warning. gcc/c-family/ChangeLog: 2020-04-28 Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> * c-attribs.c (handle_vector_size_attribute): Add attribute nonnull for argument args in order to state invariant and to silence warning of uninitialized variable usage. --- gcc/c-family/c-attribs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index ac936d5bbbb..e49a74c4048 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -117,7 +117,7 @@ static tree handle_tm_attribute (tree *, tree, tree, int, bool *); static tree handle_tm_wrap_attribute (tree *, tree, tree, int, bool *); static tree handle_novops_attribute (tree *, tree, tree, int, bool *); static tree handle_vector_size_attribute (tree *, tree, tree, int, - bool *); + bool *) ATTRIBUTE_NONNULL(3); static tree handle_nonnull_attribute (tree *, tree, tree, int, bool *); static tree handle_nonstring_attribute (tree *, tree, tree, int, bool *); static tree handle_nothrow_attribute (tree *, tree, tree, int, bool *);
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? From e07fe025a710de80cade4ae5c6e4351fbd21b672 Mon Sep 17 00:00:00 2001 From: Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> Date: Sun, 26 Apr 2020 09:42:29 +0200 Subject: [PATCH] c-attribs.c: Fix warning about use of uninitialized variable nunits 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; | Added attribute nonnull for argument args in order to silence warning and added an assert statement in order to check the invariant candidate. gcc/c-family/ChangeLog: 2020-05-04 Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> * c-attribs.c (handle_vector_size_attribute): Add attribute nonnull for argument args in order to silence warning of uninitialized variable usage and add assert statement in order to check for invariant candidate. --- gcc/c-family/c-attribs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index ac936d5bbbb..dafc77cbcb7 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -117,7 +117,7 @@ static tree handle_tm_attribute (tree *, tree, tree, int, bool *); static tree handle_tm_wrap_attribute (tree *, tree, tree, int, bool *); static tree handle_novops_attribute (tree *, tree, tree, int, bool *); static tree handle_vector_size_attribute (tree *, tree, tree, int, - bool *); + bool *) ATTRIBUTE_NONNULL(3); static tree handle_nonnull_attribute (tree *, tree, tree, int, bool *); static tree handle_nonstring_attribute (tree *, tree, tree, int, bool *); static tree handle_nothrow_attribute (tree *, tree, tree, int, bool *); @@ -3697,6 +3697,8 @@ handle_vector_size_attribute (tree *node, tree name, tree args, if (!type) return NULL_TREE; + gcc_checking_assert (args != NULL); + tree new_type = build_vector_type (type, nunits); /* Build back pointers if needed. */
On Mon, May 4, 2020 at 1:44 PM Stefan Schulze Frielinghaus <stefansf@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. Thanks, Richard.
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 > <stefansf@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?
On Mon, May 4, 2020 at 4:07 PM Stefan Schulze Frielinghaus <stefansf@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 > > <stefansf@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.
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 > <stefansf@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 > > > <stefansf@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? Thanks, Stefan
On Tue, May 5, 2020 at 5:47 PM Stefan Schulze Frielinghaus <stefansf@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 > > <stefansf@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 > > > > <stefansf@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
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index ac936d5bbbb..a8992e76755 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -3694,7 +3694,7 @@ handle_vector_size_attribute (tree *node, tree name, tree args, the number of vector units. */ unsigned HOST_WIDE_INT nunits; type = type_valid_for_vector_size (type, name, args, &nunits); - if (!type) + if (!type || !args) return NULL_TREE; tree new_type = build_vector_type (type, nunits);
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? gcc/c-family/ChangeLog: 2020-04-28 Stefan Schulze Frielinghaus <stefansf@linux.ibm.com> * c-attribs.c (handle_vector_size_attribute): Fix use of unintialized variable nunits. --- gcc/c-family/c-attribs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)