On 6/22/20 12:55 PM, Jason Merrill wrote:
On 6/22/20 1:25 PM, Martin Sebor wrote:
The attached fix parallels the one for the equivalent C bug 95580
where the pretty printers don't correctly handle MEM_REF arguments
with type void* or other pointers to an incomplete type.
The incorrect handling was exposed by the recent change to
-Wuninitialized which includes such expressions in diagnostics.
+ if (tree size = TYPE_SIZE_UNIT (TREE_TYPE (argtype)))
+ if (!integer_onep (size))
+ {
+ pp_cxx_left_paren (pp);
+ dump_type (pp, ptr_type_node, flags);
+ pp_cxx_right_paren (pp);
+ }
Don't we want to print the cast if the pointer target type is incomplete?
I suppose, yes, although after some more testing I think what should
be output is the type of the access. The target pointer type isn't
meaningful (at least not in this case).
Here's what the warning looks like in C for the test case in
gcc.dg/pr95580.c:
warning: ‘*((void *)(p)+1)’ may be used uninitialized
and like this in C++:
warning: ‘*(p +1)’ may be used uninitialized
The +1 is a byte offset, which is correct given that incrementing
a void* in GCC is the same as adding 1 to the byte address, but
dereferencing a void* doesn't correspond to what's going on in
the source.
Even for a complete type (with size greater than 1), printing
the type of the argument plus a byte offset is wrong. It ends
up with this for the C++ test case from 95768:
warning: ‘*((int*)<unknown> +4)’ is used uninitialized
when the access is actually ‘*((int*)<unknown> +1)’
So it seems to me for MEM_REF, to make the output meaningful,
it's the type of the access (i.e., the MEM_REF type) that should
be printed here, and the offset should either be in elements of
the accessed type, i.e.,
warning: ‘*((int*)<unknown> +1)’ is used uninitialized
or, if the access is misaligned, the argument should first be
cast to char*, the offset added, and the result then cast to
the access type, like this:
warning: ‘*(T*)((char*)<unknown> +1)’ is used uninitialized
The attached revised and less than fully tested patch implements
this for C++ only for now. If we agree on this approach I'll see
about making the corresponding change in C.
Martin
PR c++/95768 - pretty-printer ICE on -Wuninitialized with allocated storage
gcc/cp/ChangeLog:
PR c++/95768
* error.c (dump_expr): Handle sizeless operand types such as void*.
gcc/testsuite/ChangeLog:
PR c++/95768
* g++.dg/pr95768.C: New test.
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 0d6375e5e14..ea1f3232e3d 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -2374,32 +2374,63 @@ dump_expr (cxx_pretty_printer *pp, tree t, int flags)
break;
case MEM_REF:
- if (TREE_CODE (TREE_OPERAND (t, 0)) == ADDR_EXPR
- && integer_zerop (TREE_OPERAND (t, 1)))
- dump_expr (pp, TREE_OPERAND (TREE_OPERAND (t, 0), 0), flags);
- else
- {
- pp_cxx_star (pp);
- if (!integer_zerop (TREE_OPERAND (t, 1)))
- {
- pp_cxx_left_paren (pp);
- if (!integer_onep (TYPE_SIZE_UNIT
- (TREE_TYPE (TREE_TYPE (TREE_OPERAND (t, 0))))))
- {
- pp_cxx_left_paren (pp);
- dump_type (pp, ptr_type_node, flags);
- pp_cxx_right_paren (pp);
- }
- }
- dump_expr (pp, TREE_OPERAND (t, 0), flags);
- if (!integer_zerop (TREE_OPERAND (t, 1)))
- {
- pp_cxx_ws_string (pp, "+");
- dump_expr (pp, fold_convert (ssizetype, TREE_OPERAND (t, 1)),
- flags);
- pp_cxx_right_paren (pp);
- }
- }
+ {
+ tree type = TREE_TYPE (t);
+ tree arg = TREE_OPERAND (t, 0);
+ tree offset = TREE_OPERAND (t, 1);
+ bool zero_offset = integer_zerop (offset);
+
+ if (TREE_CODE (arg) == ADDR_EXPR && zero_offset)
+ dump_expr (pp, TREE_OPERAND (arg, 0), flags);
+ else
+ {
+ pp_cxx_star (pp);
+ if (!zero_offset)
+ {
+ pp_cxx_left_paren (pp);
+ pp_cxx_left_paren (pp);
+ dump_type (pp, type, flags);
+ pp_cxx_star (pp);
+ pp_cxx_right_paren (pp);
+
+ bool byte_offset = true;
+
+ if (tree size = TYPE_SIZE_UNIT (type))
+ {
+ /* For naturally aligned accesses print the nonzero
+ offset in units of the access type. For unaligned
+ accesses print a byte offset instead. */
+ offset_int wsiz = wi::to_offset (size);
+ offset_int woff = wi::to_offset (offset);
+ offset_int szlg2 = wi::floor_log2 (wsiz);
+ offset_int eltoff = woff >> szlg2;
+ if (eltoff << szlg2 == woff)
+ {
+ offset = wide_int_to_tree (ssizetype, eltoff);
+ byte_offset = false;
+ }
+ }
+
+ if (byte_offset)
+ {
+ /* When printing the byte offset include a cast to
+ a character type first, before printing the cast
+ to the access type. */
+ pp_cxx_left_paren (pp);
+ dump_type (pp, char_type_node, flags);
+ pp_cxx_star (pp);
+ pp_cxx_right_paren (pp);
+ }
+ }
+ dump_expr (pp, arg, flags);
+ if (!zero_offset)
+ {
+ pp_cxx_ws_string (pp, "+");
+ dump_expr (pp, fold_convert (ssizetype, offset), flags);
+ pp_cxx_right_paren (pp);
+ }
+ }
+ }
break;
case NEGATE_EXPR:
diff --git a/gcc/testsuite/g++.dg/pr95768.C b/gcc/testsuite/g++.dg/pr95768.C
new file mode 100644
index 00000000000..23e6988b410
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr95768.C
@@ -0,0 +1,32 @@
+/* PR c++/95768 - pretty-printer ICE on -Wuninitialized with allocated storage
+ { dg-do compile }
+ { dg-options "-O2 -Wall" } */
+
+extern "C" void *malloc (__SIZE_TYPE__);
+
+struct f
+{
+ int i;
+ static int e (int);
+ void operator= (int) { e (i); }
+};
+
+struct m {
+ int i;
+ f length;
+};
+
+struct n {
+ m *o() { return (m *)this; }
+};
+
+struct p {
+ n *header;
+ p () {
+ header = (n *)malloc (0);
+ m b = *header->o(); // { dg-warning "'\\*\\(\\(int\\*\\)<\[a-z\]+> \\+1\\)' is used uninitialized" }
+ b.length = 0;
+ }
+};
+
+void detach2() { p(); }
diff --git a/gcc/testsuite/g++.dg/warn/Wuninitialized-11.C b/gcc/testsuite/g++.dg/warn/Wuninitialized-11.C
new file mode 100644
index 00000000000..6f8a4586adf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-11.C
@@ -0,0 +1,40 @@
+/* Verify that -Wuninitialized warnings about accesses to objects via
+ pointers and offsets mention valid expressions.
+ { dg-do compile }
+ { dg-options "-O2 -Wall" } */
+
+typedef __INT16_TYPE__ int16_t;
+typedef __INT32_TYPE__ int32_t;
+
+void sink (int);
+
+/* Verify properly aligned accesses at offsets that are multiples of
+ the access size. */
+
+void test_aligned (void)
+{
+ char *p1 = (char*)__builtin_malloc (32);
+ p1 += sizeof (int32_t);
+
+ int16_t *p2 = (int16_t*)p1;
+ sink (p2[1]); // { dg-warning "'\\\*\\\(\\\(int16_t\\\*\\\)p1 \\\+ *3\\\)' is used uninitialized" }
+
+ int32_t *p4 = (int32_t*)p1;
+ sink (p4[1]); // { dg-warning "'\\\*\\\(\\\(int32_t\\\*\\\)p1 \\\+ *2\\\)' is used uninitialized" }
+}
+
+
+/* Verify misaligned accesses at offsets that aren't multiples of
+ the access size. */
+
+void test_misaligned (void)
+{
+ char *p1 = (char*)__builtin_malloc (32);
+ p1 += 1;
+
+ int16_t *p2 = (int16_t*)p1;
+ sink (p2[1]); // { dg-warning "'\\\*\\\(\\\(int16_t\\\*\\\)\\\(char\\\*\\\)p1 \\\+ *3\\\)' is used uninitialized" }
+
+ int32_t *p4 = (int32_t*)p1;
+ sink (p4[1]); // { dg-warning "'\\\*\\\(\\\(int32_t\\\*\\)\\\(char\\\*\\\)p1 \\\+ *5\\\)' is used uninitialized" }
+}
