Search code examples
c++gcc-warning

Returning a std::tie - dangling reference?


Question on returning std::tie from a function. If I understand correctly, then std::tie only contains references. So, returning a std::tie which points to function-local variables is a very bad idea. Shouldn't the compiler be able to detect this and issue a warning ?

Actually, we had this error in our code and all compilers and sanitizers we have missed to detect it. I was quite puzzled that this didn't get reported by any tool. Or do I understand anything incorrectly?

#include <tuple>

struct s_t {
    int a;
};

int& foo(s_t s) {
    return s.a; // warning: reference to local variable 's' returned
}

int& bar(s_t &s) {
    return s.a; // ok
}

auto bad(s_t s) {
    return std::tie(s.a); // no warning
}

auto fine(s_t &s) {
    return std::tie(s.a); // no warning
}

int main() {

    s_t s1,s2;

    auto bad_references = bad(s1);
    auto good_references = fine(s2);
    // ...

    return 0;
}

Solution

  • Your understanding of the lifetime behavior is correct. std::tie stores only references and you must assure that they are not used after the referenced object is destroyed. By-value function parameters are destroyed either at the end of the function call or at the end of the full-expression containing the function call (implementation-defined). So using the references stored in bad_references will cause undefined behavior.

    You might just be expecting too much from the compiler warning features and linter features. They generally don't do extensive analysis of the code. The analysis here would need to keep track of the reference through multiple function call layers, a store to a member and the return from the function. Such more complex analysis is what a static analyzer is for.

    However, starting with version 12.1 GCC seems to use the result of inlining function calls to report with -O2 -Wall -Wextra:

    In file included from <source>:1:
    In constructor 'constexpr std::_Head_base<_Idx, _Head, false>::_Head_base(const _Head&) [with long unsigned int _Idx = 0; _Head = int&]',
        inlined from 'constexpr std::_Tuple_impl<_Idx, _Head>::_Tuple_impl(const _Head&) [with long unsigned int _Idx = 0; _Head = int&]' at /opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/tuple:435:21,
        inlined from 'constexpr std::tuple< <template-parameter-1-1> >::tuple(const _Elements& ...) [with bool _NotEmpty = true; typename std::enable_if<_TCC<_Dummy>::__is_implicitly_constructible<const _Elements& ...>(), bool>::type <anonymous> = true; _Elements = {int&}]' at /opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/tuple:729:28,
        inlined from 'constexpr std::tuple<_Elements& ...> std::tie(_Elements& ...) [with _Elements = {int}]' at /opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/tuple:1745:44,
        inlined from 'auto bad(s_t)' at <source>:16:24:
    /opt/compiler-explorer/gcc-12.1.0/include/c++/12.1.0/tuple:193:9: warning: storing the address of local variable 's' in '*(std::_Head_base<0, int&, false>*)<return-value>.std::_Head_base<0, int&, false>::_M_head_impl' [-Wdangling-pointer=]
      193 |       : _M_head_impl(__h) { }
          |         ^~~~~~~~~~~~~~~~~
    <source>: In function 'auto bad(s_t)':
    <source>:15:14: note: 's' declared here
       15 | auto bad(s_t s) {
          |          ~~~~^
    <source>:15:14: note: '<unknown>' declared here
    

    I didn't manage to get current Clang and MSVC to produce a diagnostic, though. I guess this will work for GCC also only as long as all the relevant function calls are inlined. With e.g. -O0 the GCC warning is not produced and it probably won't be produced either if there are more complex function call layers inbetween.

    A static analyzer like clang-analyzer reports

    <source>:16:5: warning: Address of stack memory associated with local variable 's' is still referred to by the stack variable 'bad_references' upon returning to the caller.  This will be a dangling reference [clang-analyzer-core.StackAddressEscape]
        return std::tie(s.a); // no warning
        ^
    <source>:27:27: note: Calling 'bad'
        auto bad_references = bad(s1);
    

    See https://godbolt.org/z/zE8Px8vT9 for both.

    With Clang trunk and optimizations disabled ASAN also reports the problem:

    =================================================================
    ==1==ERROR: AddressSanitizer: stack-use-after-return on address 0x7fddb5e00020 at pc 0x55678be240c4 bp 0x7ffe4ed87ef0 sp 0x7ffe4ed87ee8
    READ of size 4 at 0x7fddb5e00020 thread T0
        #0 0x55678be240c3 in main /app/example.cpp:31:12
        #1 0x7fddb849e0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2) (BuildId: 9fdb74e7b217d06c93172a8243f8547f947ee6d1)
        #2 0x55678bd6231d in _start (/app/output.s+0x2131d)
    
    Address 0x7fddb5e00020 is located in stack of thread T0 at offset 32 in frame
        #0 0x55678be23d2f in bad(s_t) /app/example.cpp:15
    
      This frame has 1 object(s):
        [32, 36) 's' <== Memory access at offset 32 is inside this variable
    HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
          (longjmp and C++ exceptions *are* supported)
    SUMMARY: AddressSanitizer: stack-use-after-return /app/example.cpp:31:12 in main
    
    [...]
    

    See https://godbolt.org/z/eYo7cWeaM.

    Probably the inlining makes it harder for the sanitizers to detect this. They probably don't add the checks prior to inlining.