On 12/17/24 1:44 PM, Torbjorn SVENSSON wrote:
Hi Jason,

Thanks for the quick feedback!

On 2024-12-16 17:11, Jason Merrill wrote:
On 12/16/24 7:16 AM, Torbjörn SVENSSON wrote:
Hi,

I've reg-tested this patch on both the trunk and the releases/gcc-14
branches for x86_64-linux-gnu and arm-none-eabi and it no longer fails
for any of the out-of-bounds-diagram* tests on any of the 2 platforms.

I'm a bit puzzled if the C++ part is enough, but I can't think of a way
to trigger anything that show the wrong output after my change.
Do you think that I need to add any additional tests? I think the
existing test covers the problem well enough.

Ok for trunk and releases/gcc-14?

This won't be a candidate for backporting to 14.

Ok!


--

gcc/ChangeLog:

    PR c/116060
    c/c-typeck.cc: Make sure left hand side and right hand side has
    identical named types to aid diagnostic output.
    cp/call.cc: Likewise.

I've also split this into one block for gcc/c/ChangeLog and one for gcc/ cp/ChangeLog as mentioned by Marek in the other review.


gcc/testsuite/ChangeLog:

    PR c/116060
    c-c++-common/analyzer/out-of-bounds-diagram-8.c: Update to
    correct type.
    c-c++-common/analyzer/out-of-bounds-diagram-11.c: Likewise.
    gcc.dg/analyzer/out-of-bounds-diagram-10.c: Likewise.

Signed-off-by: Torbjörn SVENSSON <torbjorn.svens...@foss.st.com>
---
  gcc/c/c-typeck.cc                             |  3 ++
  gcc/cp/call.cc                                |  9 ++++++
  .../analyzer/out-of-bounds-diagram-11.c       | 28 +++++++++----------
  .../analyzer/out-of-bounds-diagram-8.c        | 28 +++++++++----------
  .../analyzer/out-of-bounds-diagram-10.c       | 28 +++++++++----------
  5 files changed, 54 insertions(+), 42 deletions(-)

diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 902898d1944..e3e85d1ecde 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -7831,6 +7831,9 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
    if (TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (rhstype))
      {
        warn_for_address_of_packed_member (type, orig_rhs);
+      if (type != rhstype)
+    /* Convert RHS to TYPE in order to not loose TYPE in diagnostics.  */
+    rhs = convert (type, rhs);
        return rhs;
      }
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index c8420db568e..d859ce9a2d6 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -1319,6 +1319,9 @@ standard_conversion (tree to, tree from, tree expr, bool c_cast_p,
      {
        if (CLASS_TYPE_P (to) && conv->kind == ck_rvalue)
      conv->type = qualified_to;
+      else if (from != to)
+    /* Use TO in order to not loose TO in diagnostics.  */

"lose"

+    conv->type = to;
        return conv;
      }
@@ -8741,6 +8744,12 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,          continue to warn about uses of EXPR as an integer, rather than as a
         pointer.  */
      expr = build_int_cst (totype, 0);
+      if (TREE_CODE (expr) == NON_LVALUE_EXPR && TREE_TYPE (expr) != totype)

You might check !obvalue_p (expr) instead of just NON_LVALUE_EXPR?

Appears to work as expected with !obvalue_p(expr), thanks!


+    {
+      /* Use TOTYPE in order to not loose TOTYPE in diagnostics.  */

"lose"

+       expr = copy_node (expr);
+       TREE_TYPE (expr) = totype;
+    }

Let's use cp_fold_convert instead of manually optimizing the conversion.

I've tried to use cp_fold_convert and cp_convert, but neither of them work (either during when building, or in the case of cp_fold_convert, ICE when running the regression test). For cp_fold_convert, I see the following stack trace in the ICE (just one of many examples):

Testing torture/pr47333.C,   -O2 -flto -fuse-linker-plugin -fno-fat-lto- objects
doing compile
Executing on host: /tmp/build/gcc/testsuite/g++/../../xg++ -B/tmp/build/ gcc/testsuite/g++/../../ /home/user/gcc/gcc/testsuite/g++.dg/torture/ pr47333.C -fdiagnostics-plain-output  -nostdinc++ -I/tmp/build/x86_64- pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu -I/tmp/build/ x86_64-pc-linux-gnu/libstdc++-v3/include -I/home/user/gcc/libstdc++-v3/ libsupc++ -I/home/user/gcc/libstdc++-v3/include/backward -I/home/user/ gcc/libstdc++-v3/testsuite/util -fmessage-length=0   -O2 -flto -fuse- linker-plugin -fno-fat-lto-objects  -Wno-template-body  -S -o pr47333.s    (timeout = 300) spawn -ignore SIGHUP /tmp/build/gcc/testsuite/g++/../../xg++ -B/tmp/ build/gcc/testsuite/g++/../../ /home/user/gcc/gcc/testsuite/g++.dg/ torture/pr47333.C -fdiagnostics-plain-output -nostdinc++ -I/tmp/build/ x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu -I/tmp/ build/x86_64-pc-linux-gnu/libstdc++-v3/include -I/home/user/gcc/libstdc+ +-v3/libsupc++ -I/home/user/gcc/libstdc++-v3/include/backward -I/home/ user/gcc/libstdc++-v3/testsuite/util -fmessage-length=0 -O2 -flto -fuse- linker-plugin -fno-fat-lto-objects -Wno-template-body -S -o pr47333.s
pid is 3620856 -3620856
/home/user/gcc/gcc/testsuite/g++.dg/torture/pr47333.C: In member function 'void std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_Rb_tree_impl<_Key_compare, _Is_pod_comparator>::_M_initialize()': /home/user/gcc/gcc/testsuite/g++.dg/torture/pr47333.C:774:32: internal compiler error: tree check: expected tree that contains 'decl common' structure, have 'identifier_node' in get_inner_reference, at expr.cc:8416
0x2952f5f internal_error(char const*, ...)
         /home/user/gcc/gcc/diagnostic-global-context.cc:517
0x992d99 tree_contains_struct_check_failed(tree_node const*, tree_node_structure_enum, char const*, int, char const*)
         /home/user/gcc/gcc/tree.cc:9212
0x89c4c8 contains_struct_check(tree_node*, tree_node_structure_enum, char const*, int, char const*)
         /home/user/gcc/gcc/tree.h:3815
0x89c4c8 get_inner_reference(tree_node*, poly_int<1u, long>*, poly_int<1u, long>*, tree_node**, machine_mode*, int*, int*, int*)
         /home/user/gcc/gcc/expr.cc:8416
0x10066df fold_unary_loc(unsigned long, tree_code, tree_node*, tree_node*)
         /home/user/gcc/gcc/fold-const.cc:9807
0x1007799 fold_build1_loc(unsigned long, tree_code, tree_node*, tree_node*)
         /home/user/gcc/gcc/fold-const.cc:14369
0xb1f814 cp_fold_convert(tree_node*, tree_node*)
         /home/user/gcc/gcc/cp/cvt.cc:628
0xa9ebb7 convert_like
         /home/user/gcc/gcc/cp/call.cc:9193
0xa9ebb7 perform_implicit_conversion_flags(tree_node*, tree_node*, int, int)
         /home/user/gcc/gcc/cp/call.cc:13791
0xd72098 cp_build_modify_expr(unsigned long, tree_node*, tree_code, tree_node*, int)
         /home/user/gcc/gcc/cp/typeck.cc:9872
0xd73447 build_x_modify_expr(unsigned long, tree_node*, tree_code, tree_node*, tree_node*, int)
         /home/user/gcc/gcc/cp/typeck.cc:9947
0xc4490a cp_parser_assignment_expression
         /home/user/gcc/gcc/cp/parser.cc:11023
0xc44af3 cp_parser_expression
         /home/user/gcc/gcc/cp/parser.cc:11168
0xc4d94d cp_parser_expression_statement
         /home/user/gcc/gcc/cp/parser.cc:13440
0xc554dd cp_parser_statement
         /home/user/gcc/gcc/cp/parser.cc:13215
0xc5862f cp_parser_statement_seq_opt
         /home/user/gcc/gcc/cp/parser.cc:13703
0xc588bf cp_parser_compound_statement
         /home/user/gcc/gcc/cp/parser.cc:13550
0xc82445 cp_parser_function_body
         /home/user/gcc/gcc/cp/parser.cc:26482
0xc82445 cp_parser_ctor_initializer_opt_and_function_body
         /home/user/gcc/gcc/cp/parser.cc:26533
0xc82e9e cp_parser_function_definition_after_declarator
         /home/user/gcc/gcc/cp/parser.cc:33365
Please submit a full bug report, with preprocessed source (by using - freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.


If I revert back to:

expr = copy_node (expr); TREE_TYPE (expr) = totype;

I no longer see these crashes. Is it that bad to do the above?

It's bad combined with the obvalue_p change, expr might not be something unsharable.

I think the crash you're seeing is from doing this when parsing a template; what happens if you add !processing_template_decl to the condition?


You might also adjust the ck_rvalue case later in the function, i.e. here:

      if (! MAYBE_CLASS_TYPE_P (totype))
        return expr;

I've added the same if-statement to this block as for ck_identity.


I'm in deep water here, so I'm not sure how to proceed. Please advice what else I could try, or if you would like to take over the patch to sort this out, I would be happy to let you do that too. :-)


Kind regards,
Torbjörn


Jason



Reply via email to