[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D54222#2313686 <https://reviews.llvm.org/D54222#2313686>, @JonasToth wrote:

> I am confused now, but richard knows this stuff very well! The patches you referenced showed only cases where the static was defined in an inline-header definition (and templates). Does this make a difference on the semantics?
> This should be clear before we continue.

Local static variables in inline functions and function templates are required by the C++ language semantics to result in exactly one variable in the whole program. In easy / typical cases, that works. However, this fails in at least the following cases:

- Under the MS ABI, DLLs behave more like separate programs sharing an address space than like a single program, at least by default, so by default each DLL will see a different static local variable (and similarly for class statics and other vague linkage entities). Marking the class or function as `__declspec(dllexport)` / `__declspec(dllimport)` as appropriate would be sufficient to make the static variable behave properly if it appears on the DLL interface; if you don't mark it suitably, you broke the DLL import/export rules <https://docs.microsoft.com/en-us/cpp/cpp/general-rules-and-limitations?view=vs-2019>.
- On POSIX systems, `dlopen` with `RTLD_LOCAL` (which is, unfortunately, the default for many systems), or linking a shared library with `-Bsymbolic`, can result in symbols being duplicated across the program. That's sort of fundamental -- that's what those options are supposed to do -- but they break C++ language semantics in various ways, this being one of them. (The idea that `dlopen` with `RTLD_LOCAL` creates a local copy of the specified shared library doesn't work, because -- due to C++'s heavy reliance on vague linkage definitions in interfaces -- the shared library will in general also contain symbols that are not part of that library, such as instantiations of someone else's templates or copies of someone else's local static variables, inline functions, type info, etc., and so you can also end up with local copies of those other things that aren't part of the shared library and that are expected to be unique across the program.) The solution is, don't do that -- it's not safe to use those options for shared libraries that either expose or use a vague linkage interface (which is more or less any modern C++ interface) from a different library and for which symbol uniqueness across the boundary matters (which is very hard to falsify in general).

I'm not sure whether we can locally distinguish the first case from harmless cases where the exposed state is not on a DLL boundary. We can detect instances of the second case by looking for `dlopen` calls in C++ code that use `RTLD_LOCAL` and warning on them (but we presumably can't detect `-Bsymbolic` from `clang-tidy`, because it's a linker flag).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54222/new/

https://reviews.llvm.org/D54222