[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.


================
Comment at: clang/docs/LTOVisibility.rst:45
+build system, and the binary will not dlopen any libraries deriving from the
+binary’s classes. This is useful in situations where it is not safe to specify
+``-fvisibility=hidden`` at compile time.
----------------
tejohnson wrote:
> pcc wrote:
> > I thought that the motivation was avoiding duplicate work in the case where you need varying LTO visibility? Otherwise you could just build with and without `-fvisibility=hidden` and it would be the same from an LTO visibility perspective.
> Yeah I started to write that out but it seemed too verbose (essentially we can't share a single bitcode object anymore without this, because it isn't safe for all dependent links, which to me is included in "situations where it is not safe to specify -fvisibility=hidden"). Also, on D71913, @evgeny777 mentioned he had a case were the symbol visibility constraints didn't allow for -fvisibility=hidden, but where LTO visibility for vtables can be hidden. I could add something more verbose about my case if you prefer.
> @evgeny777 mentioned he had a case were the symbol visibility constraints didn't allow for -fvisibility=hidden, but where LTO visibility for vtables can be hidden.

That case seems somewhat questionable to me. If the symbols are being exported, it is presumably for the purpose of allowing the symbols to be used outside of the defining DSO. But LTO visibility based optimizations could make any such use of the symbols unsafe. For example with WPD it's unsafe to derive outside of the defining DSO and with dead virtual function elimination it's unsafe to call virtual functions outside of the defining DSO.

That makes me think that `-lto-whole-program-visibility` should imply that associated symbols are forced to hidden to prevent such use, unless we can clearly define what it means to export such a symbol.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655