Re: [PATCH v3] handle MEM_REF with void* arguments (PR c++/95768)
On 1/7/21 1:26 AM, Jakub Jelinek wrote: On Sat, Jan 02, 2021 at 03:22:25PM -0700, Martin Sebor via Gcc-patches wrote: PR c++/95768 - pretty-printer ICE on -Wuninitialized with allocated storage gcc/c-family/ChangeLog: PR c++/95768 * c-pretty-print.c (c_pretty_printer::primary_expression): For SSA_NAMEs print VLA names and GIMPLE defining statements. (print_mem_ref): New function. (c_pretty_printer::unary_expression): Call it. This broke: +FAIL: gcc.dg/plugin/gil-1.c -fplugin=./analyzer_gil_plugin.so (test for warnings, line 16) +FAIL: gcc.dg/plugin/gil-1.c -fplugin=./analyzer_gil_plugin.so (test for warnings, line 63) +FAIL: gcc.dg/plugin/gil-1.c -fplugin=./analyzer_gil_plugin.so (test for excess errors) and +FAIL: g++.dg/cpp0x/constexpr-trivial2.C -std=c++11 (test for errors, line 13) +FAIL: g++.dg/cpp0x/constexpr-trivial2.C -std=c++11 (test for excess errors) The former one is just a different printing of the MEM_REF from what the test expects, but the latter is an ICE (one needs make check-c++-all RUNTESTFLAGS=dg.exp=constexpr-trivial2.C to reproduce or GXX_TESTSUITE_STDS=11 or similar as C++11 is not tested by default). I saw the gcc.dg/plugin/gil-1.c failure but missed the subtle difference in the output. Rerunning the test didn't show any failures so I assumed it was something transient. But when I tried gain today and looked more carefully at the output I saw the test doesn't run at all by itself. This doesn't work: $ nice make -C /ssd/test/build/gcc-95768/gcc check-c RUNTESTFLAGS="plugin.exp=gil-1.c" Is there some special magic to get it to run by itself or do these tests just not do that? Running all the plugin tests does work but also shows the failures below that don't show up in the final summary (or on gcc-testresults). I assume those are unrelated (I think I've seen this test fail in a similarly mysterious way before). FAIL: gcc.dg/plugin/diagnostic-test-expressions-1.c -fplugin=./diagnostic_plugin_test_tree_expression_range.so 1 blank line(s) in output FAIL: gcc.dg/plugin/diagnostic-test-expressions-1.c -fplugin=./diagnostic_plugin_test_tree_expression_range.so expected multiline pattern lines 550-551 not found: " __builtin_types_compatible_p \(long, int\) \+ f \(i\)\);.*\n ~\^~~\n" FAIL: gcc.dg/plugin/diagnostic-test-expressions-1.c -fplugin=./diagnostic_plugin_test_tree_expression_range.so (test for excess errors) Martin PS I committed a fix for both the ICE and the gil-1.c failure in r11-6532.
Re: [PATCH v3] handle MEM_REF with void* arguments (PR c++/95768)
On Sat, Jan 02, 2021 at 03:22:25PM -0700, Martin Sebor via Gcc-patches wrote: > PR c++/95768 - pretty-printer ICE on -Wuninitialized with allocated storage > > gcc/c-family/ChangeLog: > > PR c++/95768 > * c-pretty-print.c (c_pretty_printer::primary_expression): For > SSA_NAMEs print VLA names and GIMPLE defining statements. > (print_mem_ref): New function. > (c_pretty_printer::unary_expression): Call it. This broke: +FAIL: gcc.dg/plugin/gil-1.c -fplugin=./analyzer_gil_plugin.so (test for warnings, line 16) +FAIL: gcc.dg/plugin/gil-1.c -fplugin=./analyzer_gil_plugin.so (test for warnings, line 63) +FAIL: gcc.dg/plugin/gil-1.c -fplugin=./analyzer_gil_plugin.so (test for excess errors) and +FAIL: g++.dg/cpp0x/constexpr-trivial2.C -std=c++11 (test for errors, line 13) +FAIL: g++.dg/cpp0x/constexpr-trivial2.C -std=c++11 (test for excess errors) The former one is just a different printing of the MEM_REF from what the test expects, but the latter is an ICE (one needs make check-c++-all RUNTESTFLAGS=dg.exp=constexpr-trivial2.C to reproduce or GXX_TESTSUITE_STDS=11 or similar as C++11 is not tested by default). Jakub
Re: [PATCH v3] handle MEM_REF with void* arguments (PR c++/95768)
On 1/2/21 3:22 PM, Martin Sebor via Gcc-patches wrote: > Attached is another revision of a patch I posted last July to keep > the pretty-printer from crashing on MEM_REFs with void* arguments: > https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549746.html > > Besides avoiding the ICE and enhancing the MEM_REF detail and > improving its format, this revision implements the suggestions > in that discussion. To avoid code duplication it moves > the handling to the C pretty-printer and changes the C++ front > end to delegate to it. In addition, it includes a cast to > the accessed type if it's different from/incompatible with > (according to GIMPLE) that of the dereferenced pointer, or if > the object is typeless. Lastly, it replaces the in > the output with either VLA names or the RHS of the GIMPLE > expression (this improves the output when for dynamically > allocated objects). > > As an aside, In my experience, MEM_REFs in warnings are limited > to -Wuninitialized. I think other middle end warnings tend to > avoid them. Those that involve invalid/out-of-bounds accesses > replace them with either the target DECL (e.g., local variable, > or FIELD_DECL), the allocation call (e.g., malloc), or the DECL > of the pointer (e.g., PARM_DECL), followed by a note mentioning > the offset into the object. I'd like to change -Wuninitialized > at some point to follow the same style. So I see the value of > the MEM_REF formatting enhancement mainly as a transient solution > until that happens. > > Martin > > gcc-95768.diff > > PR c++/95768 - pretty-printer ICE on -Wuninitialized with allocated storage > > gcc/c-family/ChangeLog: > > PR c++/95768 > * c-pretty-print.c (c_pretty_printer::primary_expression): For > SSA_NAMEs print VLA names and GIMPLE defining statements. > (print_mem_ref): New function. > (c_pretty_printer::unary_expression): Call it. > > gcc/cp/ChangeLog: > > PR c++/95768 > * error.c (dump_expr): Call c_pretty_printer::unary_expression. > > gcc/testsuite/ChangeLog: > > PR c++/95768 > * g++.dg/pr95768.C: New test. > * g++.dg/warn/Wuninitialized-12.C: New test. > * gcc.dg/uninit-38.c: New test. OK jeff
[PATCH v3] handle MEM_REF with void* arguments (PR c++/95768)
Attached is another revision of a patch I posted last July to keep
the pretty-printer from crashing on MEM_REFs with void* arguments:
https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549746.html
Besides avoiding the ICE and enhancing the MEM_REF detail and
improving its format, this revision implements the suggestions
in that discussion. To avoid code duplication it moves
the handling to the C pretty-printer and changes the C++ front
end to delegate to it. In addition, it includes a cast to
the accessed type if it's different from/incompatible with
(according to GIMPLE) that of the dereferenced pointer, or if
the object is typeless. Lastly, it replaces the in
the output with either VLA names or the RHS of the GIMPLE
expression (this improves the output when for dynamically
allocated objects).
As an aside, In my experience, MEM_REFs in warnings are limited
to -Wuninitialized. I think other middle end warnings tend to
avoid them. Those that involve invalid/out-of-bounds accesses
replace them with either the target DECL (e.g., local variable,
or FIELD_DECL), the allocation call (e.g., malloc), or the DECL
of the pointer (e.g., PARM_DECL), followed by a note mentioning
the offset into the object. I'd like to change -Wuninitialized
at some point to follow the same style. So I see the value of
the MEM_REF formatting enhancement mainly as a transient solution
until that happens.
Martin
PR c++/95768 - pretty-printer ICE on -Wuninitialized with allocated storage
gcc/c-family/ChangeLog:
PR c++/95768
* c-pretty-print.c (c_pretty_printer::primary_expression): For
SSA_NAMEs print VLA names and GIMPLE defining statements.
(print_mem_ref): New function.
(c_pretty_printer::unary_expression): Call it.
gcc/cp/ChangeLog:
PR c++/95768
* error.c (dump_expr): Call c_pretty_printer::unary_expression.
gcc/testsuite/ChangeLog:
PR c++/95768
* g++.dg/pr95768.C: New test.
* g++.dg/warn/Wuninitialized-12.C: New test.
* gcc.dg/uninit-38.c: New test.
diff --git a/gcc/c-family/c-pretty-print.c b/gcc/c-family/c-pretty-print.c
index 3027703056b..3a3f2f7bdcc 100644
--- a/gcc/c-family/c-pretty-print.c
+++ b/gcc/c-family/c-pretty-print.c
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3. If not see
#include "system.h"
#include "coretypes.h"
#include "c-pretty-print.h"
+#include "gimple-pretty-print.h"
#include "diagnostic.h"
#include "stor-layout.h"
#include "stringpool.h"
@@ -1334,6 +1335,34 @@ c_pretty_printer::primary_expression (tree e)
pp_c_right_paren (this);
break;
+case SSA_NAME:
+ if (SSA_NAME_VAR (e))
+ {
+ tree var = SSA_NAME_VAR (e);
+ const char *name = IDENTIFIER_POINTER (SSA_NAME_IDENTIFIER (e));
+ const char *dot;
+ if (DECL_ARTIFICIAL (var) && (dot = strchr (name, '.')))
+ {
+ /* Print the name without the . suffix (such as in VLAs).
+ Use pp_c_identifier so that it can be converted into
+ the appropriate encoding. */
+ size_t size = dot - name;
+ char *ident = XALLOCAVEC (char, size + 1);
+ memcpy (ident, name, size);
+ ident[size] = '\0';
+ pp_c_identifier (this, ident);
+ }
+ else
+ primary_expression (var);
+ }
+ else
+ {
+ /* Print only the right side of the GIMPLE assignment. */
+ gimple *def_stmt = SSA_NAME_DEF_STMT (e);
+ pp_gimple_stmt_1 (this, def_stmt, 0, TDF_RHS_ONLY);
+ }
+ break;
+
default:
/* FIXME: Make sure we won't get into an infinite loop. */
if (location_wrapper_p (e))
@@ -1780,6 +1809,139 @@ pp_c_call_argument_list (c_pretty_printer *pp, tree t)
pp_c_right_paren (pp);
}
+/* Print the MEM_REF expression REF, including its type and offset.
+ Apply casts as necessary if the type of the access is different
+ from the type of the accessed object. Produce compact output
+ designed to include both the element index as well as any
+ misalignment by preferring
+ ((int*)((char*)p + 1))[2]
+ over
+ *(int*)((char*)p + 9)
+ The former is more verbose but makes it clearer that the access
+ to the third element of the array is misaligned by one byte. */
+
+static void
+print_mem_ref (c_pretty_printer *pp, tree e)
+{
+ tree arg = TREE_OPERAND (e, 0);
+
+ /* The byte offset. Initially equal to the MEM_REF offset, then
+ adjusted to the remainder of the division by the byte size of
+ the access. */
+ offset_int byte_off = wi::to_offset (TREE_OPERAND (e, 1));
+ /* The result of dividing BYTE_OFF by the size of the access. */
+ offset_int elt_idx = 0;
+ /* True to include a cast to char* (for a nonzero final BYTE_OFF). */
+ bool char_cast = false;
+ const bool addr = TREE_CODE (arg) == ADDR_EXPR;
+ if (addr)
+{
+ arg = TREE_OPERAND (arg, 0);
+ if (byte_off == 0)
+ {
+ pp->expression (arg);
+ return;
+ }
+}
+
+ const tree access_type = TREE_TYPE (e);
+ tree arg_type = TREE_TYPE (TREE_TYPE (arg));
+ if (TREE_CODE (arg_type) == ARRAY_TYPE)
+arg_type = TREE_TYPE (arg_type);
+
+
