warn_access: Limit waccess2 to dangling pointer checks [PR 123374]

Message ID 20260106191047.3385622-1-siddhesh@gotplt.org
State New
Headers
Series warn_access: Limit waccess2 to dangling pointer checks [PR 123374] |

Commit Message

Siddhesh Poyarekar Jan. 6, 2026, 7:10 p.m. UTC
  The second pass of warn_access (waccess2) was added to implement
dangling pointer checks but it implicitly ran the early checks too,
which issues false warnings on code that has not been fully optimized.

Limit this second run to only dangling pointer checks for call
statements.  This does not break any of the existing warning tests, so
it didn't seem to add any actual value for the additional run anyway.

gcc/ChangeLog:

	PR tree-optimization/123374
	* gimple-ssa-warn-access.cc (pass_waccess::set_pass_param): Add
	a second parameter.
	(pass_waccess::check_call): Skip access checks for waccess2.
	(pass_waccess::execute): Drop initialization of
	M_CHECK_DANGLING_P.
	* passes.def: Adjust.

gcc/testsuite/ChangeLog:

	PR tree-optimization/123374
	* g++.dg/warn/pr123374.cc: New test.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
Testing:
- Bootstrapped and tested on x86_64, no new failures
- Bootstrap for config=ubsan in progress
- Build and tested i686, no new failures.

 gcc/gimple-ssa-warn-access.cc         | 38 +++++++++++++++++----------
 gcc/passes.def                        |  6 ++---
 gcc/testsuite/g++.dg/warn/pr123374.cc | 29 ++++++++++++++++++++
 3 files changed, 56 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/pr123374.cc
  

Comments

Andrew Pinski Jan. 7, 2026, 1:03 a.m. UTC | #1
On Tue, Jan 6, 2026 at 11:11 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> The second pass of warn_access (waccess2) was added to implement
> dangling pointer checks but it implicitly ran the early checks too,
> which issues false warnings on code that has not been fully optimized.
>
> Limit this second run to only dangling pointer checks for call
> statements.  This does not break any of the existing warning tests, so
> it didn't seem to add any actual value for the additional run anyway.
>
> gcc/ChangeLog:
>
>         PR tree-optimization/123374
>         * gimple-ssa-warn-access.cc (pass_waccess::set_pass_param): Add
>         a second parameter.
>         (pass_waccess::check_call): Skip access checks for waccess2.
>         (pass_waccess::execute): Drop initialization of
>         M_CHECK_DANGLING_P.
>         * passes.def: Adjust.
>
> gcc/testsuite/ChangeLog:
>
>         PR tree-optimization/123374
>         * g++.dg/warn/pr123374.cc: New test.

Ok but your testcase is misnamed because of the `.cc` suffix. The only
file name ending that works in the g++.dg testsuite is `.C`.

Thanks,
Andrew


>
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
> Testing:
> - Bootstrapped and tested on x86_64, no new failures
> - Bootstrap for config=ubsan in progress
> - Build and tested i686, no new failures.
>
>  gcc/gimple-ssa-warn-access.cc         | 38 +++++++++++++++++----------
>  gcc/passes.def                        |  6 ++---
>  gcc/testsuite/g++.dg/warn/pr123374.cc | 29 ++++++++++++++++++++
>  3 files changed, 56 insertions(+), 17 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/warn/pr123374.cc
>
> diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
> index 44e62475cfa..df34da2bf96 100644
> --- a/gcc/gimple-ssa-warn-access.cc
> +++ b/gcc/gimple-ssa-warn-access.cc
> @@ -2246,11 +2246,21 @@ pass_waccess::~pass_waccess ()
>  }
>
>  void
> -pass_waccess::set_pass_param (unsigned int n, bool early)
> +pass_waccess::set_pass_param (unsigned int n, bool param)
>  {
> -  gcc_assert (n == 0);
> -
> -  m_early_checks_p = early;
> +  /* Check for dangling pointers in the earliest runs of the pass.
> +     The latest point -Wdangling-pointer should run is just before
> +     loop unrolling which introduces uses after clobbers.  Most cases
> +     can be detected without optimization; cases where the address of
> +     the local variable is passed to and then returned from a user-
> +     defined function before its lifetime ends and the returned pointer
> +     becomes dangling depend on inlining.  */
> +  if (n == 0)
> +    m_early_checks_p = param;
> +  else if (n == 1)
> +    m_check_dangling_p = param;
> +  else
> +    __builtin_unreachable ();
>  }
>
>  /* Return true when any checks performed by the pass are enabled.  */
> @@ -4380,6 +4390,16 @@ pass_waccess::check_call (gcall *stmt)
>        && gimple_call_internal_fn (stmt) == IFN_ASAN_MARK)
>      return;
>
> +  if (m_check_dangling_p)
> +    {
> +      check_call_dangling (stmt);
> +
> +      /* Don't do any other checks when doing dangling pointer checks the
> +        second time.  */
> +      if (!m_early_checks_p)
> +       return;
> +    }
> +
>    if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
>      check_builtin (stmt);
>
> @@ -4397,7 +4417,6 @@ pass_waccess::check_call (gcall *stmt)
>      }
>
>    check_call_access (stmt);
> -  check_call_dangling (stmt);
>
>    if (m_early_checks_p)
>      return;
> @@ -4800,15 +4819,6 @@ pass_waccess::execute (function *fun)
>    m_ptr_qry.rvals = enable_ranger (fun);
>    m_func = fun;
>
> -  /* Check for dangling pointers in the earliest run of the pass.
> -     The latest point -Wdangling-pointer should run is just before
> -     loop unrolling which introduces uses after clobbers.  Most cases
> -     can be detected without optimization; cases where the address of
> -     the local variable is passed to and then returned from a user-
> -     defined function before its lifetime ends and the returned pointer
> -     becomes dangling depend on inlining.  */
> -  m_check_dangling_p = m_early_checks_p;
> -
>    auto_bitmap bb_uids_set (&bitmap_default_obstack);
>    m_bb_uids_set = bb_uids_set;
>
> diff --git a/gcc/passes.def b/gcc/passes.def
> index 54b56b3c0c8..4586d2cf6ad 100644
> --- a/gcc/passes.def
> +++ b/gcc/passes.def
> @@ -61,7 +61,7 @@ along with GCC; see the file COPYING3.  If not see
>        NEXT_PASS (pass_warn_printf);
>        NEXT_PASS (pass_warn_nonnull_compare);
>        NEXT_PASS (pass_early_warn_uninitialized);
> -      NEXT_PASS (pass_warn_access, /*early=*/true);
> +      NEXT_PASS (pass_warn_access, /*early=*/true, /*check_dangling=*/true);
>        NEXT_PASS (pass_ubsan);
>        NEXT_PASS (pass_nothrow);
>        NEXT_PASS (pass_rebuild_cgraph_edges);
> @@ -216,7 +216,7 @@ along with GCC; see the file COPYING3.  If not see
>        NEXT_PASS (pass_object_sizes);
>        NEXT_PASS (pass_post_ipa_warn);
>        /* Must run before loop unrolling.  */
> -      NEXT_PASS (pass_warn_access, /*early=*/true);
> +      NEXT_PASS (pass_warn_access, /*early=*/false, /*check_dangling=*/true);
>        /* Profile count may overflow as a result of inlinining very large
>           loop nests.  This pass should run before any late pass that makes
>          use of profile.  */
> @@ -451,7 +451,7 @@ along with GCC; see the file COPYING3.  If not see
>    NEXT_PASS (pass_gimple_isel);
>    NEXT_PASS (pass_harden_conditional_branches);
>    NEXT_PASS (pass_harden_compares);
> -  NEXT_PASS (pass_warn_access, /*early=*/false);
> +  NEXT_PASS (pass_warn_access, /*early=*/false, /*check_dangling=*/false);
>    NEXT_PASS (pass_cleanup_cfg_post_optimizing);
>    NEXT_PASS (pass_warn_function_noreturn);
>
> diff --git a/gcc/testsuite/g++.dg/warn/pr123374.cc b/gcc/testsuite/g++.dg/warn/pr123374.cc
> new file mode 100644
> index 00000000000..cf777672aa7
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/pr123374.cc
> @@ -0,0 +1,29 @@
> +long ext_f (void *, unsigned long)
> +    __attribute__ ((__access__ (__write_only__, 1, 2)));
> +
> +long f (void *b, unsigned long n)
> +{
> +  unsigned long sz = __builtin_dynamic_object_size(b, 0);
> +
> +  return (__builtin_constant_p (sz) && sz == -1UL) ? ext_f (b, n) : 0;
> +}
> +
> +
> +void test (unsigned limit, long init_off)
> +{
> +  char buf[4096];
> +  unsigned long off = 0;
> +
> +  while (off == 0)
> +    off += init_off;
> +
> +  while (off < limit)
> +    {
> +      long n = f (buf + off, sizeof (buf) - off);
> +
> +      if (n <= 0)
> +       continue;
> +
> +      off += n;
> +    }
> +}
> --
> 2.52.0
>
  
Siddhesh Poyarekar Jan. 7, 2026, 1:09 a.m. UTC | #2
On 2026-01-06 20:03, Andrew Pinski wrote:
> On Tue, Jan 6, 2026 at 11:11 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>>
>> The second pass of warn_access (waccess2) was added to implement
>> dangling pointer checks but it implicitly ran the early checks too,
>> which issues false warnings on code that has not been fully optimized.
>>
>> Limit this second run to only dangling pointer checks for call
>> statements.  This does not break any of the existing warning tests, so
>> it didn't seem to add any actual value for the additional run anyway.
>>
>> gcc/ChangeLog:
>>
>>          PR tree-optimization/123374
>>          * gimple-ssa-warn-access.cc (pass_waccess::set_pass_param): Add
>>          a second parameter.
>>          (pass_waccess::check_call): Skip access checks for waccess2.
>>          (pass_waccess::execute): Drop initialization of
>>          M_CHECK_DANGLING_P.
>>          * passes.def: Adjust.
>>
>> gcc/testsuite/ChangeLog:
>>
>>          PR tree-optimization/123374
>>          * g++.dg/warn/pr123374.cc: New test.
> 
> Ok but your testcase is misnamed because of the `.cc` suffix. The only
> file name ending that works in the g++.dg testsuite is `.C`.
> 

Ack, I'll rename and push then.

Thanks,
Sid
  
Siddhesh Poyarekar Jan. 7, 2026, 1:16 a.m. UTC | #3
On 2026-01-06 20:09, Siddhesh Poyarekar wrote:
> On 2026-01-06 20:03, Andrew Pinski wrote:
>> On Tue, Jan 6, 2026 at 11:11 AM Siddhesh Poyarekar 
>> <siddhesh@gotplt.org> wrote:
>>>
>>> The second pass of warn_access (waccess2) was added to implement
>>> dangling pointer checks but it implicitly ran the early checks too,
>>> which issues false warnings on code that has not been fully optimized.
>>>
>>> Limit this second run to only dangling pointer checks for call
>>> statements.  This does not break any of the existing warning tests, so
>>> it didn't seem to add any actual value for the additional run anyway.
>>>
>>> gcc/ChangeLog:
>>>
>>>          PR tree-optimization/123374
>>>          * gimple-ssa-warn-access.cc (pass_waccess::set_pass_param): Add
>>>          a second parameter.
>>>          (pass_waccess::check_call): Skip access checks for waccess2.
>>>          (pass_waccess::execute): Drop initialization of
>>>          M_CHECK_DANGLING_P.
>>>          * passes.def: Adjust.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>          PR tree-optimization/123374
>>>          * g++.dg/warn/pr123374.cc: New test.
>>
>> Ok but your testcase is misnamed because of the `.cc` suffix. The only
>> file name ending that works in the g++.dg testsuite is `.C`.
>>
> 
> Ack, I'll rename and push then.

Sorry, spoke too soon, I just noticed I didn't have any dg-* directives 
in the test case :facepalm:

I'll fix and repost.

Sid
  

Patch

diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 44e62475cfa..df34da2bf96 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -2246,11 +2246,21 @@  pass_waccess::~pass_waccess ()
 }
 
 void
-pass_waccess::set_pass_param (unsigned int n, bool early)
+pass_waccess::set_pass_param (unsigned int n, bool param)
 {
-  gcc_assert (n == 0);
-
-  m_early_checks_p = early;
+  /* Check for dangling pointers in the earliest runs of the pass.
+     The latest point -Wdangling-pointer should run is just before
+     loop unrolling which introduces uses after clobbers.  Most cases
+     can be detected without optimization; cases where the address of
+     the local variable is passed to and then returned from a user-
+     defined function before its lifetime ends and the returned pointer
+     becomes dangling depend on inlining.  */
+  if (n == 0)
+    m_early_checks_p = param;
+  else if (n == 1)
+    m_check_dangling_p = param;
+  else
+    __builtin_unreachable ();
 }
 
 /* Return true when any checks performed by the pass are enabled.  */
@@ -4380,6 +4390,16 @@  pass_waccess::check_call (gcall *stmt)
       && gimple_call_internal_fn (stmt) == IFN_ASAN_MARK)
     return;
 
+  if (m_check_dangling_p)
+    {
+      check_call_dangling (stmt);
+
+      /* Don't do any other checks when doing dangling pointer checks the
+	 second time.  */
+      if (!m_early_checks_p)
+	return;
+    }
+
   if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
     check_builtin (stmt);
 
@@ -4397,7 +4417,6 @@  pass_waccess::check_call (gcall *stmt)
     }
 
   check_call_access (stmt);
-  check_call_dangling (stmt);
 
   if (m_early_checks_p)
     return;
@@ -4800,15 +4819,6 @@  pass_waccess::execute (function *fun)
   m_ptr_qry.rvals = enable_ranger (fun);
   m_func = fun;
 
-  /* Check for dangling pointers in the earliest run of the pass.
-     The latest point -Wdangling-pointer should run is just before
-     loop unrolling which introduces uses after clobbers.  Most cases
-     can be detected without optimization; cases where the address of
-     the local variable is passed to and then returned from a user-
-     defined function before its lifetime ends and the returned pointer
-     becomes dangling depend on inlining.  */
-  m_check_dangling_p = m_early_checks_p;
-
   auto_bitmap bb_uids_set (&bitmap_default_obstack);
   m_bb_uids_set = bb_uids_set;
 
diff --git a/gcc/passes.def b/gcc/passes.def
index 54b56b3c0c8..4586d2cf6ad 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -61,7 +61,7 @@  along with GCC; see the file COPYING3.  If not see
       NEXT_PASS (pass_warn_printf);
       NEXT_PASS (pass_warn_nonnull_compare);
       NEXT_PASS (pass_early_warn_uninitialized);
-      NEXT_PASS (pass_warn_access, /*early=*/true);
+      NEXT_PASS (pass_warn_access, /*early=*/true, /*check_dangling=*/true);
       NEXT_PASS (pass_ubsan);
       NEXT_PASS (pass_nothrow);
       NEXT_PASS (pass_rebuild_cgraph_edges);
@@ -216,7 +216,7 @@  along with GCC; see the file COPYING3.  If not see
       NEXT_PASS (pass_object_sizes);
       NEXT_PASS (pass_post_ipa_warn);
       /* Must run before loop unrolling.  */
-      NEXT_PASS (pass_warn_access, /*early=*/true);
+      NEXT_PASS (pass_warn_access, /*early=*/false, /*check_dangling=*/true);
       /* Profile count may overflow as a result of inlinining very large
          loop nests.  This pass should run before any late pass that makes
 	 use of profile.  */
@@ -451,7 +451,7 @@  along with GCC; see the file COPYING3.  If not see
   NEXT_PASS (pass_gimple_isel);
   NEXT_PASS (pass_harden_conditional_branches);
   NEXT_PASS (pass_harden_compares);
-  NEXT_PASS (pass_warn_access, /*early=*/false);
+  NEXT_PASS (pass_warn_access, /*early=*/false, /*check_dangling=*/false);
   NEXT_PASS (pass_cleanup_cfg_post_optimizing);
   NEXT_PASS (pass_warn_function_noreturn);
 
diff --git a/gcc/testsuite/g++.dg/warn/pr123374.cc b/gcc/testsuite/g++.dg/warn/pr123374.cc
new file mode 100644
index 00000000000..cf777672aa7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr123374.cc
@@ -0,0 +1,29 @@ 
+long ext_f (void *, unsigned long)
+    __attribute__ ((__access__ (__write_only__, 1, 2)));
+
+long f (void *b, unsigned long n)
+{
+  unsigned long sz = __builtin_dynamic_object_size(b, 0);
+
+  return (__builtin_constant_p (sz) && sz == -1UL) ? ext_f (b, n) : 0;
+}
+
+
+void test (unsigned limit, long init_off)
+{
+  char buf[4096];
+  unsigned long off = 0;
+
+  while (off == 0)
+    off += init_off;
+
+  while (off < limit)
+    {
+      long n = f (buf + off, sizeof (buf) - off);
+
+      if (n <= 0)
+	continue;
+
+      off += n;
+    }
+}