* [PATCH] c: Fix ICE caused by get_parm_array_spec [PR101702]
@ 2021-08-03 7:17 Jakub Jelinek
2021-08-04 15:51 ` Martin Sebor
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2021-08-03 7:17 UTC (permalink / raw)
To: Joseph S. Myers, Marek Polacek, Martin Sebor; +Cc: gcc-patches
Hi!
The following testcase ICEs, because nelts is NOP_EXPR around INTEGER_CST
- it is a VLA whose extent folds into a constant - and get_parm_array_spec
has specific INTEGER_CST handling and otherwise strips nops from nelts
and stores it into a TREE_LIST that is later asserted to be a DECL_P
or EXPR_P, where the INTEGER_CST is neither of that.
So, either we can strip nops earlier (needs moving the integral type
check first as STRIP_NOPS can alter that e.g. to pointer or from
pointer to integer) and thus handle as INTEGER_CST even the case
of INTEGER_CST wrapped into casts as this patch does, or we need
to change handle_argspec_attribute's assertion to allow INTEGER_CSTs
as well there.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Or do you prefer to change handle_argspec_attribute ?
2021-08-03 Jakub Jelinek <jakub@redhat.com>
PR c/101702
* c-decl.c (get_parm_array_spec): Check for non-integral
type first, then STRIP_NOPS and only afterwards check for
INTEGER_CST.
* gcc.dg/pr101702.c: New test.
--- gcc/c/c-decl.c.jj 2021-07-15 18:50:52.000000000 +0200
+++ gcc/c/c-decl.c 2021-08-02 18:56:35.532045128 +0200
@@ -5842,6 +5842,11 @@ get_parm_array_spec (const struct c_parm
if (pd->u.array.static_p)
spec += 's';
+ if (!INTEGRAL_TYPE_P (TREE_TYPE (nelts)))
+ /* Avoid invalid NELTS. */
+ return attrs;
+
+ STRIP_NOPS (nelts);
if (TREE_CODE (nelts) == INTEGER_CST)
{
/* Skip all constant bounds except the most significant one.
@@ -5859,13 +5864,9 @@ get_parm_array_spec (const struct c_parm
spec += buf;
break;
}
- else if (!INTEGRAL_TYPE_P (TREE_TYPE (nelts)))
- /* Avoid invalid NELTS. */
- return attrs;
/* Each variable VLA bound is represented by a dollar sign. */
spec += "$";
- STRIP_NOPS (nelts);
vbchain = tree_cons (NULL_TREE, nelts, vbchain);
}
--- gcc/testsuite/gcc.dg/pr101702.c.jj 2021-08-02 18:58:24.614534975 +0200
+++ gcc/testsuite/gcc.dg/pr101702.c 2021-08-02 18:57:52.611978024 +0200
@@ -0,0 +1,11 @@
+/* PR c/101702 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+double foo (double x[!__builtin_copysignf (~2, 3)]);
+
+double
+bar (double *x)
+{
+ return foo (x);
+}
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] c: Fix ICE caused by get_parm_array_spec [PR101702]
2021-08-03 7:17 [PATCH] c: Fix ICE caused by get_parm_array_spec [PR101702] Jakub Jelinek
@ 2021-08-04 15:51 ` Martin Sebor
0 siblings, 0 replies; 2+ messages in thread
From: Martin Sebor @ 2021-08-04 15:51 UTC (permalink / raw)
To: Jakub Jelinek, Joseph S. Myers, Marek Polacek; +Cc: gcc-patches
On 8/3/21 1:17 AM, Jakub Jelinek wrote:
> Hi!
>
> The following testcase ICEs, because nelts is NOP_EXPR around INTEGER_CST
> - it is a VLA whose extent folds into a constant - and get_parm_array_spec
> has specific INTEGER_CST handling and otherwise strips nops from nelts
> and stores it into a TREE_LIST that is later asserted to be a DECL_P
> or EXPR_P, where the INTEGER_CST is neither of that.
>
> So, either we can strip nops earlier (needs moving the integral type
> check first as STRIP_NOPS can alter that e.g. to pointer or from
> pointer to integer) and thus handle as INTEGER_CST even the case
> of INTEGER_CST wrapped into casts as this patch does, or we need
> to change handle_argspec_attribute's assertion to allow INTEGER_CSTs
> as well there.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> Or do you prefer to change handle_argspec_attribute ?
I think the bug is in the bound being treated as variable but then
in determining its value to be constant. I expect the following
to be diagnosed again, the same way as in GCC 11:
double foo (double x[!__builtin_copysignf (~2, 3)]);
double foo (double x[]);
Your fix prevents it. So I think the right fix would treat
the bound as constant in this case.
Martin
>
> 2021-08-03 Jakub Jelinek <jakub@redhat.com>
>
> PR c/101702
> * c-decl.c (get_parm_array_spec): Check for non-integral
> type first, then STRIP_NOPS and only afterwards check for
> INTEGER_CST.
>
> * gcc.dg/pr101702.c: New test.
>
> --- gcc/c/c-decl.c.jj 2021-07-15 18:50:52.000000000 +0200
> +++ gcc/c/c-decl.c 2021-08-02 18:56:35.532045128 +0200
> @@ -5842,6 +5842,11 @@ get_parm_array_spec (const struct c_parm
> if (pd->u.array.static_p)
> spec += 's';
>
> + if (!INTEGRAL_TYPE_P (TREE_TYPE (nelts)))
> + /* Avoid invalid NELTS. */
> + return attrs;
> +
> + STRIP_NOPS (nelts);
> if (TREE_CODE (nelts) == INTEGER_CST)
> {
> /* Skip all constant bounds except the most significant one.
> @@ -5859,13 +5864,9 @@ get_parm_array_spec (const struct c_parm
> spec += buf;
> break;
> }
> - else if (!INTEGRAL_TYPE_P (TREE_TYPE (nelts)))
> - /* Avoid invalid NELTS. */
> - return attrs;
>
> /* Each variable VLA bound is represented by a dollar sign. */
> spec += "$";
> - STRIP_NOPS (nelts);
> vbchain = tree_cons (NULL_TREE, nelts, vbchain);
> }
>
> --- gcc/testsuite/gcc.dg/pr101702.c.jj 2021-08-02 18:58:24.614534975 +0200
> +++ gcc/testsuite/gcc.dg/pr101702.c 2021-08-02 18:57:52.611978024 +0200
> @@ -0,0 +1,11 @@
> +/* PR c/101702 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +double foo (double x[!__builtin_copysignf (~2, 3)]);
> +
> +double
> +bar (double *x)
> +{
> + return foo (x);
> +}
>
> Jakub
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-08-04 15:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-03 7:17 [PATCH] c: Fix ICE caused by get_parm_array_spec [PR101702] Jakub Jelinek
2021-08-04 15:51 ` Martin Sebor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).