* [committed] fix another ICE in MEM_REF formatting (PR 98578)
@ 2021-01-07 21:29 Martin Sebor
2021-01-07 21:37 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Martin Sebor @ 2021-01-07 21:29 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 445 bytes --]
Fixing the ICE in MEM_REF formatting (or the enhancements that
came along with the fix) introduced another, ICE plus a plugin
test failure. I have committed the attached simple patch to
fix both.
Martin
PS There are outstanding bugs to fix/improvements to be made
to the MEM_REF formatting (though hopefully no more ICEs) that
I found while testing the attached fix. I xfailed the tests
and opened the referenced bugs to keep track of them.
[-- Attachment #2: gcc-98578.diff --]
[-- Type: text/x-patch, Size: 8731 bytes --]
commit 178f0afce3611282170de380fcea9db9d6e3ff0c
Author: Martin Sebor <msebor@redhat.com>
Date: Thu Jan 7 14:20:39 2021 -0700
PR middle-end/98578 - ICE warning on uninitialized VLA access
gcc/c-family/ChangeLog:
PR middle-end/98578
* c-pretty-print.c (print_mem_ref): Strip array from access type.
Avoid assuming acces type's size is constant. Correct condition
guarding the printing of a parenthesis.
gcc/testsuite/ChangeLog:
PR middle-end/98578
* gcc.dg/plugin/gil-1.c: Adjust expected output.
* gcc.dg/uninit-pr98578.c: New test.
diff --git a/gcc/c-family/c-pretty-print.c b/gcc/c-family/c-pretty-print.c
index e963cf53091..87301a2091c 100644
--- a/gcc/c-family/c-pretty-print.c
+++ b/gcc/c-family/c-pretty-print.c
@@ -1844,22 +1844,25 @@ print_mem_ref (c_pretty_printer *pp, tree e)
}
}
- const tree access_type = TREE_TYPE (e);
+ tree access_type = TREE_TYPE (e);
+ if (TREE_CODE (access_type) == ARRAY_TYPE)
+ access_type = TREE_TYPE (access_type);
tree arg_type = TREE_TYPE (TREE_TYPE (arg));
if (TREE_CODE (arg_type) == ARRAY_TYPE)
arg_type = TREE_TYPE (arg_type);
if (tree access_size = TYPE_SIZE_UNIT (access_type))
- {
- /* For naturally aligned accesses print the nonzero offset
- in units of the accessed type, in the form of an index.
- For unaligned accesses also print the residual byte offset. */
- offset_int asize = wi::to_offset (access_size);
- offset_int szlg2 = wi::floor_log2 (asize);
-
- elt_idx = byte_off >> szlg2;
- byte_off = byte_off - (elt_idx << szlg2);
- }
+ if (TREE_CODE (access_size) == INTEGER_CST)
+ {
+ /* For naturally aligned accesses print the nonzero offset
+ in units of the accessed type, in the form of an index.
+ For unaligned accesses also print the residual byte offset. */
+ offset_int asize = wi::to_offset (access_size);
+ offset_int szlg2 = wi::floor_log2 (asize);
+
+ elt_idx = byte_off >> szlg2;
+ byte_off = byte_off - (elt_idx << szlg2);
+ }
/* True to include a cast to the accessed type. */
const bool access_cast = VOID_TYPE_P (arg_type)
@@ -1924,9 +1927,9 @@ print_mem_ref (c_pretty_printer *pp, tree e)
}
if (elt_idx != 0)
{
- if (byte_off == 0 && char_cast)
+ if (access_cast || char_cast)
pp_c_right_paren (pp);
- pp_c_right_paren (pp);
+
if (addr)
{
pp_space (pp);
diff --git a/gcc/testsuite/gcc.dg/plugin/gil-1.c b/gcc/testsuite/gcc.dg/plugin/gil-1.c
index 4e8f535ba85..66872f07466 100644
--- a/gcc/testsuite/gcc.dg/plugin/gil-1.c
+++ b/gcc/testsuite/gcc.dg/plugin/gil-1.c
@@ -13,7 +13,7 @@ void test_2 (PyObject *obj)
{
Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" } */
- Py_INCREF (obj); /* { dg-warning "use of PyObject '\\*\\(obj\\)' without the GIL" } */
+ Py_INCREF (obj); /* { dg-warning "use of PyObject '\\*obj' without the GIL" } */
Py_DECREF (obj);
Py_END_ALLOW_THREADS
@@ -60,7 +60,7 @@ void test_5 (PyObject *obj)
static void __attribute__((noinline))
called_by_test_6 (PyObject *obj)
{
- Py_INCREF (obj); /* { dg-warning "use of PyObject '\\*\\(obj\\)' without the GIL" } */
+ Py_INCREF (obj); /* { dg-warning "use of PyObject '\\*obj' without the GIL" } */
Py_DECREF (obj);
}
diff --git a/gcc/testsuite/gcc.dg/uninit-pr98578.c b/gcc/testsuite/gcc.dg/uninit-pr98578.c
new file mode 100644
index 00000000000..98d611757ab
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/uninit-pr98578.c
@@ -0,0 +1,110 @@
+/* PR middle-end/98578 - ICE warning on uninitialized VLA access
+ { dg-do compile }
+ { dg-options "-O2 -Wall" } */
+
+void* malloc (__SIZE_TYPE__);
+
+void T (int, ...);
+
+void vla_n (int n, int i)
+{
+ int a1[n];
+
+ /* a1[I] should be formatted as as a1[I] (or, for I == 0, perhaps
+ as *a1), but definitely not as *a1[I]. This is a bug in VLA
+ formatting. */
+ T (a1[0]); // { dg-warning "'a1\\\[0]' is used uninitialized" "pr98587" { xfail *-*-* } }
+ // { dg-warning "'\\*a1\\\[0]' is used uninitialized" "spurious star" { target *-*-* } .-1 }
+ T (a1[1]); // { dg-warning "a1\\\[1]' is used uninitialized" }
+ T (a1[i]); // { dg-warning "a1\\\[i]' is used uninitialized" }
+}
+
+void vla_n_2 (int n, int i)
+{
+ int a2[n][2];
+
+ T (a2[0][0]); // { dg-warning "a2\\\[0]\\\[0]' is used uninitialized" }
+ T (a2[2][1]); // { dg-warning "a2\\\[2]\\\[1]' is used uninitialized" }
+ T (a2[3][i]); // { dg-warning "a2\\\[3]\\\[i]' is used uninitialized" }
+ T (a2[i][0]); // { dg-warning "a2\\\[i]\\\[0]' is used uninitialized" }
+ T (a2[i][i]); // { dg-warning "a2\\\[i]\\\[i]' is used uninitialized" }
+}
+
+
+void vla_3_n (int n, int i)
+{
+ int a2[3][n];
+
+ T (a2[0][0]); // { dg-warning "a2\\\[0]\\\[0]' is used uninitialized" }
+ T (a2[1][2]); // { dg-warning "a2\\\[1]\\\[2]' is used uninitialized" }
+ T (a2[2][i]); // { dg-warning "a2\\\[2]\\\[i]' is used uninitialized" }
+ T (a2[i][3]); // { dg-warning "a2\\\[i]\\\[3]' is used uninitialized" }
+ T (a2[i][i]); // { dg-warning "a2\\\[i]\\\[i]' is used uninitialized" }
+}
+
+
+void vla_n_n (int n, int i)
+{
+ int a2[n][n];
+
+ T (a2[0][0]); // { dg-warning "a2\\\[0]\\\[0]' is used uninitialized" }
+ T (a2[4][5]); // { dg-warning "a2\\\[4]\\\[5]' is used uninitialized" }
+ T (a2[6][i]); // { dg-warning "a2\\\[6]\\\[i]' is used uninitialized" }
+ T (a2[i][7]); // { dg-warning "a2\\\[i]\\\[7]' is used uninitialized" }
+ T (a2[i][i]); // { dg-warning "a2\\\[i]\\\[i]' is used uninitialized" }
+}
+
+
+void char_ptr_n (int n, int i)
+{
+ char *p = malloc (n);
+
+ T (p[0]); // { dg-warning "'\\\*p' is used uninitialized" }
+ T (p[1]); // { dg-warning "'p\\\[1]' is used uninitialized" }
+ T (p[i]); // { dg-warning "'p\\\[i]' is used uninitialized" "pr98587" { xfail *-*-* } }
+ // { dg-warning "is used uninitialized" "POINTER_PLUS_EXPR" { target *-*-* } .-1 }
+}
+
+
+void int_ptr_n (int n, int i)
+{
+ int *p = malloc (n);
+
+ T (p[0]); // { dg-warning "'\\\*p' is used uninitialized" }
+ T (p[1]); // { dg-warning "'p\\\[1]' is used uninitialized" }
+ T (p[i]); // { dg-warning "'p\\\[i]' is used uninitialized" "pr98587" { xfail *-*-* } }
+ // { dg-warning "is used uninitialized" "POINTER_PLUS_EXPR" { target *-*-* } .-1 }
+}
+
+
+void int_arr_ptr_n (int n, int i)
+{
+ int (*p)[n] = malloc (n);
+
+ T ((*p)[0]); // { dg-warning "\\(\\*p\\)\\\[0]' is used uninitialized" "pr98587" { xfail *-*-* } }
+ // { dg-warning "\\*p\\\[0]' is used uninitialized" "missing parens" { target *-*-* } .-1 }
+ T ((*p)[1]); // { dg-warning "\\(\\*p\\)\\\[1]' is used uninitialized" "pr98587" { xfail *-*-* } }
+ // { dg-warning "\\*p\\\[1]' is used uninitialized" "missing parens" { target *-*-* } .-1 }
+ T ((*p)[i]); // { dg-warning "\\(\\*p\\)\\\[i]' is used uninitialized" "pr98587" { xfail *-*-* } }
+ // { dg-warning "\\*p\\\[i]' is used uninitialized" "missing parens" { target *-*-* } .-1 }
+}
+
+
+void int_arr_ptr_n_n (int n, int i)
+{
+ int (*p)[n][n] = malloc (n);
+
+ T ((*p)[0][0]); // { dg-warning "\\(\\*p\\)\\\[0]\\\[0]' is used uninitialized" "pr98587" { xfail *-*-* } }
+ // { dg-warning "\\*p\\\[0]\\\[0]' is used uninitialized" "missing parens" { target *-*-* } .-1 }
+ T ((*p)[1][2]); // { dg-warning "\\(\\*p\\)\\\[1]\\\[2]' is used uninitialized" "pr98587" { xfail *-*-* } }
+ // { dg-warning "\\*p\\\[1]\\\[2]' is used uninitialized" "missing parens" { target *-*-* } .-1 }
+ T ((*p)[0][i]); // { dg-warning "\\(\\*p\\)\\\[0]\\\[i]' is used uninitialized" "pr98587" { xfail *-*-* } }
+ // { dg-warning "\\*p\\\[0]\\\[i]' is used uninitialized" "missing parens" { target *-*-* } .-1 }
+ T ((*p)[3][i]); // { dg-warning "\\(\\*p\\)\\\[3]\\\[i]' is used uninitialized" "pr98587" { xfail *-*-* } }
+ // { dg-warning "\\*p\\\[3]\\\[i]' is used uninitialized" "missing parens" { target *-*-* } .-1 }
+ T ((*p)[i][i]); // { dg-warning "\\(\\*p\\)\\\[i]\\\[i]' is used uninitialized" "pr98587" { xfail *-*-* } }
+ // { dg-warning "\\*p\\\[i]\\\[i]' is used uninitialized" "missing parens" { target *-*-* } .-1 }
+
+ T ((*p)[i][i + 1]); // { dg-warning "\\(\\*p\\)\\\[i]\\\[i \\+ 1]' is used uninitialized" "pr98588" { xfail *-*-* } }
+ // { dg-warning "\\*p\\\[i]\\\[<unknown>]' is used uninitialized" "missing parens" { target *-*-* } .-1 }
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [committed] fix another ICE in MEM_REF formatting (PR 98578)
2021-01-07 21:29 [committed] fix another ICE in MEM_REF formatting (PR 98578) Martin Sebor
@ 2021-01-07 21:37 ` Jakub Jelinek
2021-01-07 23:24 ` Martin Sebor
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2021-01-07 21:37 UTC (permalink / raw)
To: Martin Sebor; +Cc: gcc-patches
On Thu, Jan 07, 2021 at 02:29:50PM -0700, Martin Sebor via Gcc-patches wrote:
> --- a/gcc/c-family/c-pretty-print.c
> +++ b/gcc/c-family/c-pretty-print.c
> @@ -1844,22 +1844,25 @@ print_mem_ref (c_pretty_printer *pp, tree e)
> }
> }
>
> - const tree access_type = TREE_TYPE (e);
> + tree access_type = TREE_TYPE (e);
> + if (TREE_CODE (access_type) == ARRAY_TYPE)
> + access_type = TREE_TYPE (access_type);
> tree arg_type = TREE_TYPE (TREE_TYPE (arg));
> if (TREE_CODE (arg_type) == ARRAY_TYPE)
> arg_type = TREE_TYPE (arg_type);
The array types can be multidimensional, are you sure you don't want
to use strip_array_types instead?
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [committed] fix another ICE in MEM_REF formatting (PR 98578)
2021-01-07 21:37 ` Jakub Jelinek
@ 2021-01-07 23:24 ` Martin Sebor
0 siblings, 0 replies; 3+ messages in thread
From: Martin Sebor @ 2021-01-07 23:24 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 1/7/21 2:37 PM, Jakub Jelinek wrote:
> On Thu, Jan 07, 2021 at 02:29:50PM -0700, Martin Sebor via Gcc-patches wrote:
>> --- a/gcc/c-family/c-pretty-print.c
>> +++ b/gcc/c-family/c-pretty-print.c
>> @@ -1844,22 +1844,25 @@ print_mem_ref (c_pretty_printer *pp, tree e)
>> }
>> }
>>
>> - const tree access_type = TREE_TYPE (e);
>> + tree access_type = TREE_TYPE (e);
>> + if (TREE_CODE (access_type) == ARRAY_TYPE)
>> + access_type = TREE_TYPE (access_type);
>> tree arg_type = TREE_TYPE (TREE_TYPE (arg));
>> if (TREE_CODE (arg_type) == ARRAY_TYPE)
>> arg_type = TREE_TYPE (arg_type);
>
> The array types can be multidimensional, are you sure you don't want
> to use strip_array_types instead?
Pretty sure.
access_type is used to figure out the element index and residual
byte offset into the argument, so that needs the size of the array.
Both access_type and arg_type are then checked for compatibility,
to decide if the type of the access needs to be included as a cast.
So there again I think the outer array bounds need to be preserved.
There are a few simple tests involving multidimensional VLAs but
a more involved example I just tried triggers another ICE, this
time in gimple_canonical_types_compatible_p. Apparently
the default invocation of the function doesn't like mixed arrays
and scalars. So clearly there's a whole bounty of ICEs here and
more to do.
Martin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-07 23:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-07 21:29 [committed] fix another ICE in MEM_REF formatting (PR 98578) Martin Sebor
2021-01-07 21:37 ` Jakub Jelinek
2021-01-07 23:24 ` 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).