[PATCH] D155659: [WPD][LLD] Add option to validate RTTI is enabled on all native types and prevent devirtualization on types with native RTTI

[Di Mo via Phabricator via llvm-commits]

modimo added inline comments.


================
Comment at: lld/ELF/Driver.cpp:1098
+  for (auto &s : vtableSymbolsWithNoRTTI) {
+    warn("--lto-validate-all-vtables-have-type-infos: RTTI missing for vtable "
+         "_ZTV" +
----------------
MaskRay wrote:
> modimo wrote:
> > tejohnson wrote:
> > > modimo wrote:
> > > > tejohnson wrote:
> > > > > modimo wrote:
> > > > > > tejohnson wrote:
> > > > > > > Prefer message() over warn() because the latter causes builds using -Werror to fail.
> > > > > > Is that the case for lld? Looking through the equivalent functionality is under `--fatal-warnings` and on a small example `-Werror` doesn't affect this flag/cause the build to fail.
> > > > > Sorry, you are correct. We use --fatal-warnings for this on linker actions (and -Werror on compile actions). In general, I think this should be an informational message, not a warning, since it is being handled automatically.
> > > > The case I want to catch is when an existing project changes its native dependencies and disables the optimization which would better fit a warning. In the general case agreed that this isn't a warning. I don't have much experience in linker protocol here, @MaskRay thoughts? 
> > > The problem is if it is a warning, then we have to go in and manually change options or builds will fail. Can you intercept message output, which should always be emitted (i.e. don't need verbose options).
> > Sure, changed to message.
> This long list here makes me nervous. Will try to learn it.
Taking a look again I don't think these names need special exclusion. They're defined in libstdc++/libc++abi and the release packages in my scenarios have RTTI enabled meaning their type info symbols are present during linking. Testing E2E linking on some of our large services with these removed succeeds.

If RTTI is disabled on these libraries considerably more symbols would not have matching type infos and --lto-whole-program-visibility should be disabled. Looking back I think these exclusions came about when I was only examining the .symtab/.dynsym of individual object/shared files which didn't take into account that these symbols would be resolved by libstdc++/libc++abi.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155659