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