Re: [PATCH] fold-const-call.cc: add strnlen in fold_const_call [PR86937]
Daniel Henrique Barboza <[email protected]>
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <[email protected]> |
On 8/14/2026 9:08 AM, Daniel Henrique Barboza wrote: > Hi, > > > On 8/12/2026 8:48 PM, Andrea Pinski wrote: >> On Wed, Aug 12, 2026 at 11:01 AM Daniel Barboza >> <[email protected]> wrote: >>> >>> We're not trying to fold builtin_strnlen calls into constants like we do >>> with builtin_strlen. This results in missed optimizations where we >>> have a strnlen that uses a PHI as input: >>> >>> const char a[4] = "123"; >>> >>> int g (int i) >>> { >>> return __builtin_strnlen (i ? a : "", 4); >>> } >>> >>> The strnlen call could be folded into a single PHI <0, 3> but instead >>> we're calling strnlen with the PHI result: >>> >>> # iftmp.1_2 = PHI <&aD.4472(3), ""(2)> >>> # VUSE <.MEM_4(D)> >>> # RANGE [irange] long unsigned int [0, 4] MASK 0x7 VALUE 0x0 >>> # USE = nonlocal escaped const-pool { D.4472 } (nonlocal) >>> _1 = strnlenD.1862 (iftmp.1_2, 4); >>> --- >>> gcc/fold-const-call.cc | 11 ++++++++++ >>> gcc/testsuite/gcc.dg/tree-ssa/pr86937.c | 15 +++++++++++++ >>> gcc/testsuite/gcc.dg/warn-strnlen-no-nul.c | 25 +++++++++++----------- >>> 3 files changed, 39 insertions(+), 12 deletions(-) >>> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr86937.c >>> >>> diff --git a/gcc/fold-const-call.cc b/gcc/fold-const-call.cc >>> index b3a9d7715d6..c483b80e704 100644 >>> --- a/gcc/fold-const-call.cc >>> +++ b/gcc/fold-const-call.cc >>> @@ -1911,6 +1911,17 @@ fold_const_call (combined_fn fn, tree type, tree arg0, tree arg1) >>> } >>> return NULL_TREE; >>> >>> + case CFN_BUILT_IN_STRNLEN: >>> + if ((p0 = c_getstr (arg0))) >>> + { >>> + unsigned HOST_WIDE_INT s1 = 0; >>> + if (!size_t_cst_p (arg1, &s1)) >>> + return NULL_TREE; >>> + >>> + return build_int_cst (type, strnlen (p0, s1)); >> >> >> >>> + } >>> + return NULL_TREE; >>> + >>> case CFN_FOLD_LEFT_PLUS: >>> return fold_const_fold_left (type, arg0, arg1, PLUS_EXPR); >>> >>> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr86937.c b/gcc/testsuite/gcc.dg/tree-ssa/pr86937.c >>> new file mode 100644 >>> index 00000000000..fcd1c9d0eb7 >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr86937.c >>> @@ -0,0 +1,15 @@ >>> +/* { dg-do compile } */ >>> +/* { dg-options "-O2 -fdump-tree-pre" } */ >>> +const char a[4] = "123"; >>> + >>> +int f (int i) >>> +{ >>> + return __builtin_strlen (i ? a : ""); >>> +} >>> + >>> +int g (int i) >>> +{ >>> + return __builtin_strnlen (i ? a : "", 4); >>> +} >>> + >>> +/* { dg-final { scan-tree-dump-times "strnlen" 0 "pre" } } */ >> >> Please add a space after strnlen just in case it matches something >> else. Basically there should be no calls left to strnlen. The dumps >> are always `functionname (...)`. > > The output this test produces is : > > _1 = strnlenD.1862 (iftmp.1_2, 4); > > So adding a space after won't match the dump. > > Grepping the existing files I see that gcc.dg/strlenopt-46.c does: > > { dg-final { scan-tree-dump-times "= strnlen" 7 "optimized" } } */ > > > Thus I think we can do "= strnlen" here too. Thanks, Nevermind that :D using the actual testsuite the output will be _1 = __builtin_strnlen (iftmp.1_2, 4); Your suggestion was correct. I'll make the change for v2. Cheers, Daniel > > Daniel > > >> >>> diff --git a/gcc/testsuite/gcc.dg/warn-strnlen-no-nul.c b/gcc/testsuite/gcc.dg/warn-strnlen-no-nul.c >>> index 70f6a432b97..fde2fa0e189 100644 >>> --- a/gcc/testsuite/gcc.dg/warn-strnlen-no-nul.c >>> +++ b/gcc/testsuite/gcc.dg/warn-strnlen-no-nul.c >>> @@ -147,14 +147,14 @@ T (v0 ? b[3] : "", bsz); >>> the strnlen calls are safe because the reads are bounded by >>> the length of the constant arguments. Most of the calls are >>> not diagnosed anymore as a result of the fix for PR 103215. */ >>> -T (v0 ? "" : b[0], bsz + 1); /* { dg-warning "bound 6 exceeds source size 5" } */ >>> +T (v0 ? "" : b[0], bsz + 1); /* { dg-warning "bound 6 exceeds source size 5" "pr86937" { xfail *-*-* } } */ >> >> So if I read the comment, I think the dg-warning can just be removed. >> >>> T (v0 ? "" : b[1], bsz + 1); >>> T (v0 ? "" : b[2], bsz + 1); >>> -T (v0 ? "" : b[3], bsz + 1); >>> -T (v0 ? b[0] : "", bsz + 1); /* { dg-warning "bound 6 exceeds source size 5" } */ >> Remove this warning rather than changing xfailing. >>> +T (v0 ? "" : b[3], bsz + 1); /* { dg-warning "unterminated" } */ >> This new warning is correct as b[3] is "54321" in a array size of 5. >> >>> +T (v0 ? b[0] : "", bsz + 1); /* { dg-warning "bound 6 exceeds source size 5" "pr86937" { xfail *-*-* } } */ >>> T (v0 ? b[1] : "", bsz + 1); >>> T (v0 ? b[2] : "", bsz + 1); >>> -T (v0 ? b[3] : "", bsz + 1); >>> +T (v0 ? b[3] : "", bsz + 1); /* { dg-warning "unterminated" } */ >> Likewise. >> >>> >>> T (v0 ? "" : b[i0], bsz); >>> T (v0 ? "" : b[i1], bsz); >>> @@ -168,11 +168,11 @@ T (v0 ? b[i3] : "", bsz); >>> T (v0 ? "" : b[i0], bsz + 1); >>> T (v0 ? "" : b[i1], bsz + 1); >>> T (v0 ? "" : b[i2], bsz + 1); >>> -T (v0 ? "" : b[i3], bsz + 1); >>> +T (v0 ? "" : b[i3], bsz + 1); /* { dg-warning "unterminated" } */ >> Likewise. >>> T (v0 ? b[i0] : "", bsz + 1); >>> T (v0 ? b[i1] : "", bsz + 1); >>> T (v0 ? b[i2] : "", bsz + 1); >>> -T (v0 ? b[i3] : "", bsz + 1); >>> +T (v0 ? b[i3] : "", bsz + 1); /* { dg-warning "unterminated" } */ >> Likewise. >>> >>> T (v0 ? "1234" : b[3], bsz); >>> T (v0 ? "1234" : b[i3], bsz); >>> @@ -184,17 +184,18 @@ T (v0 ? b[0] : b[2], bsz); >>> T (v0 ? b[2] : b[3], bsz); >>> T (v0 ? b[3] : b[2], bsz); >>> >>> -T (v0 ? "1234" : b[3], bsz + 1); >>> -T (v0 ? "1234" : b[i3], bsz + 1); >>> -T (v0 ? b[3] : "1234", bsz + 1); >>> -T (v0 ? b[i3] : "1234", bsz + 1); >>> +/* New warnings being thrown after PR86937. */ >>> +T (v0 ? "1234" : b[3], bsz + 1); /* { dg-warning "unterminated" } */ >>> +T (v0 ? "1234" : b[i3], bsz + 1); /* { dg-warning "unterminated" } */ >>> +T (v0 ? b[3] : "1234", bsz + 1); /* { dg-warning "unterminated" } */ >>> +T (v0 ? b[i3] : "1234", bsz + 1); /* { dg-warning "unterminated" } */ >> These are all valid warnings. >> >>> >>> /* That the following are not diagnosed is a bug/limitation resulting from >>> the fix for PR 103215. */ >>> T (v0 ? a : b[3], bsz + 1); /* { dg-warning "bound 6 exceeds source size 5" "pr103215" { xfail *-*-* } } */ >>> T (v0 ? b[0] : b[2], bsz + 1); /* { dg-warning "bound 6 exceeds source size 5" "pr103215" { xfail *-*-* } } */ >>> -T (v0 ? b[2] : b[3], bsz + 1); /* { dg-warning "bound 6 exceeds source size 5" "pr103215" { xfail *-*-* } } */ >>> -T (v0 ? b[3] : b[2], bsz + 1); /* { dg-warning "bound 6 exceeds source size 5" "pr103215" { xfail *-*-* } } */ >>> +T (v0 ? b[2] : b[3], bsz + 1); /* { dg-warning "unterminated" } */ >>> +T (v0 ? b[3] : b[2], bsz + 1); /* { dg-warning "unterminated" } */ >> Yes the same new valid warning b[3] is unterminated. >> >> So ok with the removal of the newly xfailed dg-warning since those >> warnings won't show up ever since the values are all calculated at >> compile time. >> >> Thanks, >> Andrea >> >>> >>> struct A { char a[5], b[5]; }; >>> >>> -- >>> 2.43.0 >>> >