| Message ID | 75656f1e-4e82-4987-aea1-d0c5602f094c@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Strlen pass should set current range query. | expand |
On Tue, May 28, 2024 at 1:24 AM Andrew MacLeod <amacleod@redhat.com> wrote: > > The strlen pass currently has a local ranger instance, but when it > invokes SCEV or any other shared component, SCEV will not be able to > access to this ranger as it uses get_range_query(). They will be stuck > with global ranges. > > Enable/disable ranger should be used instead of a local version which > allows other components to use the current range_query. > > Bootstraps on 86_64-pc-linux-gnu, but there is one regression. The > regression is from gcc.dg/Wstringop-overflow-10.c. the function in > question: > > void > baz (char *a) > { > char b[16] = "abcdefg"; > __builtin_strncpy (a, b, __builtin_strnlen (b, 7)); /* { dg-bogus > "specified bound depends on the length of the source argument" } */ > } > > when compiled with -O2 -Wstringop-overflow -Wstringop-truncation > > it now spits out: > > b2.c: In function ‘baz’: > b2.c:24:3: warning: ‘__builtin_strncpy’ output 2 truncated before > terminating nul copying <unknown> bytes from a string of the same length > [-Wstringop-truncation] > 24 | __builtin_strncpy (a, b, __builtin_strnlen (b, 7)); /* { > dg-bogus "specified bound depends on the length of the source argument" } */ > > It seems like maybe something got smarter by setting the current range > query and this is a legitimate warning for this line of code? There > will indeed not be a NULL copied as there are 7 characters in the string... > > Is this a testcase issue where this warning should have been issued > before, or am I misunderstanding the warning? I think the warning makes sense in this case. But I'm not sure why the dg-bogus is there, that looks like a valid complaint as well?! I think the patch is OK. Richard. > Andrew > > PS im afraid of adjusting the status quo in this pass... :-P Not > allowing sSCEV to access the current ranger is causing me other issues > with the fix for 115221. This *should* have been a harmless change > sigh. :-( The whole mechanism should just use the current range-query > instad of passing a ranger pointer aorund. But that a much bigger > issue. one thing at a time. > > >
From 308d5a6b9937c36300e22fcab1ecb67af55dce9e Mon Sep 17 00:00:00 2001 From: Andrew MacLeod <amacleod@redhat.com> Date: Mon, 27 May 2024 13:20:13 -0400 Subject: [PATCH 1/2] Strlen pass should set current range query. The strlen pass currently has a local ranger instance, but when it invokes SCEV, scev will not be able to access to this ranger. Enable/disable ranger shoud be used, allowing other components to use the current range_query. * tree-ssa-strlen.cc (strlen_pass::strlen_pass): Add function pointer and initialize ptr_qry with current range_query. (strlen_pass::m_ranger): Remove. (printf_strlen_execute): Enable and disable ranger. --- gcc/tree-ssa-strlen.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gcc/tree-ssa-strlen.cc b/gcc/tree-ssa-strlen.cc index 7596dd80942..c43a2da2836 100644 --- a/gcc/tree-ssa-strlen.cc +++ b/gcc/tree-ssa-strlen.cc @@ -235,9 +235,9 @@ get_range (tree val, gimple *stmt, wide_int minmax[2], class strlen_pass : public dom_walker { public: - strlen_pass (cdi_direction direction) + strlen_pass (function *fun, cdi_direction direction) : dom_walker (direction), - ptr_qry (&m_ranger), + ptr_qry (get_range_query (fun)), m_cleanup_cfg (false) { } @@ -299,8 +299,6 @@ public: unsigned HOST_WIDE_INT lenrng[2], unsigned HOST_WIDE_INT *size, bool *nulterm); - gimple_ranger m_ranger; - /* A pointer_query object to store information about pointers and their targets in. */ pointer_query ptr_qry; @@ -5912,9 +5910,10 @@ printf_strlen_execute (function *fun, bool warn_only) ssa_ver_to_stridx.safe_grow_cleared (num_ssa_names, true); max_stridx = 1; + enable_ranger (fun); /* String length optimization is implemented as a walk of the dominator tree and a forward walk of statements within each block. */ - strlen_pass walker (CDI_DOMINATORS); + strlen_pass walker (fun, CDI_DOMINATORS); walker.walk (ENTRY_BLOCK_PTR_FOR_FN (fun)); if (dump_file && (dump_flags & TDF_DETAILS)) @@ -5939,6 +5938,7 @@ printf_strlen_execute (function *fun, bool warn_only) strlen_to_stridx = NULL; } + disable_ranger (fun); scev_finalize (); loop_optimizer_finalize (); -- 2.41.0
The strlen pass currently has a local ranger instance, but when it invokes SCEV or any other shared component, SCEV will not be able to access to this ranger as it uses get_range_query(). They will be stuck with global ranges. Enable/disable ranger should be used instead of a local version which allows other components to use the current range_query. Bootstraps on 86_64-pc-linux-gnu, but there is one regression. The regression is from gcc.dg/Wstringop-overflow-10.c. the function in question: void baz (char *a) { char b[16] = "abcdefg"; __builtin_strncpy (a, b, __builtin_strnlen (b, 7)); /* { dg-bogus "specified bound depends on the length of the source argument" } */ } when compiled with -O2 -Wstringop-overflow -Wstringop-truncation it now spits out: b2.c: In function ‘baz’: b2.c:24:3: warning: ‘__builtin_strncpy’ output 2 truncated before terminating nul copying <unknown> bytes from a string of the same length [-Wstringop-truncation] 24 | __builtin_strncpy (a, b, __builtin_strnlen (b, 7)); /* { dg-bogus "specified bound depends on the length of the source argument" } */ It seems like maybe something got smarter by setting the current range query and this is a legitimate warning for this line of code? There will indeed not be a NULL copied as there are 7 characters in the string... Is this a testcase issue where this warning should have been issued before, or am I misunderstanding the warning? Andrew PS im afraid of adjusting the status quo in this pass... :-P Not allowing sSCEV to access the current ranger is causing me other issues with the fix for 115221. This *should* have been a harmless change sigh. :-( The whole mechanism should just use the current range-query instad of passing a ranger pointer aorund. But that a much bigger issue. one thing at a time.