[Bug tree-optimization/90905] missing -Wreturn-local-addr returning a local std::string::c_str()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90905
Andrew Pinski changed:
What|Removed |Added
Last reconfirmed|2020-01-28 00:00:00 |2024-4-7
--- Comment #10 from Andrew Pinski ---
Note changing:
```
const char *p = "def";
```
To:
```
static const char *p = "def";
```
we do warn though.
But increasing the size like:
```
static const char *p = "def123123123123123123123123123123";
```
We don't warn.
With the larger size we get:
```
operator delete (_8, _12);
[local count: 1073741824]:
str ={v} {CLOBBER(eob)};
str ={v} {CLOBBER(eos)};
return _8;
```
Which is not exactly a `-Wreturn-local-addr` warning but rather a
`-Wuse-after-free` warning.
[Bug tree-optimization/90905] missing -Wreturn-local-addr returning a local std::string::c_str()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90905 --- Comment #9 from Martin Sebor --- This still isn't diagnosed by GCC 12 even with its -Wuse-after-free and -Wdangling-pointer.
[Bug tree-optimization/90905] missing -Wreturn-local-addr returning a local std::string::c_str()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90905 Martin Liška changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2020-01-28 CC||marxin at gcc dot gnu.org Ever confirmed|0 |1
[Bug tree-optimization/90905] missing -Wreturn-local-addr returning a local std::string::c_str()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90905
--- Comment #8 from Martin Sebor ---
Right. The warning pass sees this:
[local count: 1073612976]:
__dnew ={v} {CLOBBER};
D.29156 ={v} {CLOBBER};
D.29152 ={v} {CLOBBER};
if (_M_local_buf != _23)
goto ; [53.47%]
else
goto ; [46.53%]
[local count: 574060859]:
_5 = str.D.19306._M_allocated_capacity;
_3 = _5 + 1;
operator delete (_23, _3);
[local count: 1073612977]:
str ={v} {CLOBBER};
return _23;
Maybe the alias oracle could be put to use here after all to improve things.
[Bug tree-optimization/90905] missing -Wreturn-local-addr returning a local std::string::c_str()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90905
--- Comment #7 from Marc Glisse ---
(In reply to Martin Sebor from comment #6)
> With str being a local (non-reference) variable this should be diagnosed
> because of the str.D.28972._M_local_buf(12):
>
> # _47 = PHI <_59(9), _M_local_buf(12), _59(8)>
> str ={v} {CLOBBER};
> return _47;
the dump above was obtained from your testcase, compiled with a slightly hacked
libstdc++
+++ basic_string.h 2019-06-18 17:40:06.435533019 +0200
@@ -214,7 +214,9 @@
_M_set_length(size_type __n)
{
_M_length(__n);
+ auto X = _M_data();
traits_type::assign(_M_data()[__n], _CharT());
+ if(X!=_M_data())__builtin_unreachable();
}
bool
+++ basic_string.tcc2019-06-18 17:38:00.755534757 +0200
@@ -221,8 +221,12 @@
}
// Check for out_of_range and length_error exceptions.
+ auto X = _M_data();
__try
- { this->_S_copy_chars(_M_data(), __beg, __end); }
+ {
+ this->_S_copy_chars(_M_data(), __beg, __end);
+ if(X!=_M_data())__builtin_unreachable();
+ }
__catch(...)
{
_M_dispose();
and I am really not seeing a warning. But that's probably because the
path-isolation pass is too early (from a warning PoV), the IL doesn't look that
nice at that time yet.
[Bug tree-optimization/90905] missing -Wreturn-local-addr returning a local std::string::c_str()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90905
--- Comment #6 from Martin Sebor ---
With str being a local (non-reference) variable this should be diagnosed
because of the str.D.28972._M_local_buf(12):
# _47 = PHI <_59(9), _M_local_buf(12), _59(8)>
str ={v} {CLOBBER};
return _47;
In your example a is a reference argument but in this modified version:
struct A { char *p; char c[13]; };
void* f (struct A a, _Bool b)
{
a.p = b ? a.c : (char*)__builtin_malloc (13);
__builtin_memcpy (a.p, "hello world!", 12);
a.p[12] = 0;
return a.p;
}
and the IL:
[local count: 354334802]:
iftmp.0_7 = __builtin_malloc (13);
[local count: 1073741824]:
# iftmp.0_2 = PHI
a.p = iftmp.0_2;
__builtin_memcpy (iftmp.0_2, "hello world!", 12);
_1 = a.p;
MEM[(char *)_1 + 12B] = 0;
return _1;
the only challenge with detecting the bug that I see is making a record of the
rhs of the assignment to _1 = a.p (and others like that) and then checking the
prior assignment to a.p (et al.). With that in place the "may return" warning
will trigger.
[Bug tree-optimization/90905] missing -Wreturn-local-addr returning a local std::string::c_str()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90905
--- Comment #5 from Marc Glisse ---
struct A { char*p; char c[13]; };
void f(A,bool b){
a.p=b?a.c:(char*)__builtin_malloc(13);
__builtin_memcpy(a.p, "hello world!", 12);
a.p[12]=0;
}
gives
if (b_4(D) != 0)
goto ; [67.00%]
else
goto ; [33.00%]
[local count: 719407023]:
iftmp.0_9 = _8(D)->c;
goto ; [100.00%]
[local count: 354334802]:
iftmp.0_7 = __builtin_malloc (13);
[local count: 1073741824]:
# iftmp.0_2 = PHI
a_8(D)->p = iftmp.0_2;
__builtin_memcpy (iftmp.0_2, "hello world!", 12);
_1 = a_8(D)->p;
MEM[(char *)_1 + 12B] = 0;
Note in particular that the compiler fails to notice that memcpy cannot clobber
the first field of p.
If I tweak the libstdc++ implementation to let it know that copying and writing
the final 0 do not clobber the pointer, I can get
# _47 = PHI <_59(9), _M_local_buf(12), _59(8)>
str ={v} {CLOBBER};
return _47;
but that doesn't warn, although you just said that it warns for "maybe"s?
[Bug tree-optimization/90905] missing -Wreturn-local-addr returning a local std::string::c_str()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90905
--- Comment #4 from Martin Sebor ---
I meant for f1 not to see the string pointed to by p. The warning usually
triggers either for the "must return" and "may return" case but in f1 it's too
complicated for it to see that the return pointer may be that to
str._M_local_buf. It might be possible to figure it out from the 'if
(_M_local_buf != _7)' below:
[local count: 346356134]:
# _23 = PHI <_M_local_buf(8), _16(5)>
__builtin_memcpy (_23, p.0_1, _15);
[local count: 1073612976]:
__dnew.6_20 = __dnew;
MEM[(size_type *) + 8B] = __dnew.6_20;
_21 = MEM[(char * *)];
_22 = _21 + __dnew.6_20;
MEM[(char_type &)_22] = 0;
__dnew ={v} {CLOBBER};
D.28812 ={v} {CLOBBER};
D.28808 ={v} {CLOBBER};
_7 = MEM[(char * *)];
if (_M_local_buf != _7)
goto ; [53.47%]
else
goto ; [46.53%]
[local count: 574060859]:
_5 = str.D.19209._M_allocated_capacity;
_3 = _5 + 1;
operator delete (_7, _3);
[local count: 1073612977]:
str ={v} {CLOBBER};
return _7;
}
I've been enhancing the warning to resolve pr71924 (and pr90549) so I might
look into this if I get a chance.
[Bug tree-optimization/90905] missing -Wreturn-local-addr returning a local std::string::c_str()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90905 --- Comment #3 from Marc Glisse --- > compiling both functions in the samne translatin unit suppresses the warning > for f0. It is quite common for extra code to change inlining decisions. You still get the warning at -O3. > const char *p = "def"; Did you mean to write const char * const p = "def"; (which does warn)? Otherwise, the compiler has no way to know this is a short string, so we end up with a function that returns either the address of a local variable or a pointer to deleted memory (although we don't have a nice PHI like that, even with -std=c++2a to disable extern templates, probably some suboptimal aliasing analysis).
[Bug tree-optimization/90905] missing -Wreturn-local-addr returning a local std::string::c_str()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90905 --- Comment #2 from Martin Sebor --- The separate enhancement is pr90906.
[Bug tree-optimization/90905] missing -Wreturn-local-addr returning a local std::string::c_str()
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90905 Martin Sebor changed: What|Removed |Added See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=90906 --- Comment #1 from Martin Sebor --- An analogous test case with std::vector or other containers is, of course, not diagnosed at all because the warning only looks for locally declared variables and not pointers to allocated memory that has been freed. That's a separate enhancement (pr90905).
