Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate adding lifetimebound attributes to important functions #3754

Open
StephanTLavavej opened this issue Jun 7, 2023 · 2 comments
Open
Labels
enhancement Something can be improved

Comments

@StephanTLavavej
Copy link
Member

The [[clang::lifetimebound]] and [[msvc::lifetimebound]] attributes can detect dangerous usage:

C:\Temp>type woof.cpp
struct X {
    int a;
};

#ifdef __clang__
#define ATTR [[clang::lifetimebound]]
#else
#define ATTR [[msvc::lifetimebound]] // should use _HAS_MSVC_ATTRIBUTE
#endif

const int& f(const X& x ATTR) noexcept {
    return x.a;
}

int main() {
    const int& r = f(X{1729}); // dangerous, emits warning
    (void) r;
}
C:\Temp>clang-cl /EHsc /nologo /W4 /MTd /Od /c woof.cpp
woof.cpp(16,22): warning: temporary bound to local reference 'r' will be destroyed at the end of the full-expression
      [-Wdangling]
    const int& r = f(X{1729}); // dangerous, emits warning
                     ^~~~~~~
1 warning generated.

C:\Temp>set esp.extensions=cppcorecheck.dll

C:\Temp>cl /EHsc /nologo /W4 /MTd /Od /c /analyze:autolog- /analyze:plugin espxengine.dll woof.cpp
woof.cpp
C:\Temp\woof.cpp(16) : warning C26815: The pointer is dangling because it points at a temporary instance which was destroyed.

We should investigate adding these attributes to important functions in the STL.

To avoid regressions, we should:

  • Have an escape hatch
  • Do this gradually
  • Have test coverage (similar to our "include all headers" tests) that verifies that the STL itself is clean with respect this these dangling-reference warnings
  • For each added attribute, manually verify that it detects bogus usage
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Jun 7, 2023
@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Jun 8, 2023

The most obvious candidates are reference casting functions like std::move, to which [[msvc::intrinsic]] should also be applied.

Many member functions of std::array are good candidates.

get functions (except for overloads for subrange) may be good candidates, but we need to restrict the use to the cases where the corresponding tuple_element_t is an object type.

Not sure whether false positive could come from other containers since we may move them. Edit: I think I was confused then. Moving shouldn't be a problem and we should mark data(), front() etc. with lifetimebound.

@frederick-vs-ja
Copy link
Contributor

Seemly related to #2094 (on the test coverage).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

No branches or pull requests

2 participants