ThinLTO + CFI can introduce duplicate symbols

[ilovepi]

In https://reviews.llvm.org/D137982 we found that introducing an alias to a function could interact poorly w/ ThinLTO + PGO + CFI, and introduced duplicate symbols into the binary.

The original bug can be found in the chromium bug tracker: https://bugs.chromium.org/p/chromium/issues/detail?id=1408161

In our investigation we found that GlobalOpt replaced the alias we generated in InstrProfiling.cpp. This only appears to manifest w/ non-local linkage types. Note we only observed this w/ weak/linkonce_odr in comdat groups where the alias and function had hidden visibility. It's not entirely clear this is the correct behavior, however, since the surrounding code in GlobalOpt doesn't appear to only do this for functions with hidden visibility. The other issue was that, despite it replacing all the uses of the alias, the alias was not removed from the module.

Later in LowerTypeTests, there is some fairly complex logic to generate and/or replace existing aliases. This appears to be where the duplicate symbol is introduced, since it was not completely removed in GlobalOpt. I think it's also possible that the duplicate symbol is introduced through use of a Twine, but that is only speculation at this point.

We should investigate this more thoroughly and make sure that the implementations in both GlobalOpt and LowerTypeTests correctly handle these cases and work well together. While we've mitigated this problem in https://reviews.llvm.org/D137982 for the aliases we introduced during PGO instrumentation, the alias types we introduced were legal and should not have resulted duplicate symbols.

Looping in relevant parties from the original bug report. CC @aeubanks @teresajohnson @petrhosek

[llvmbot]

@llvm/issue-subscribers-bug

[teresajohnson]

@pcc can you take a look? I'm less familiar with the CFI code in LowerTypeTests referenced here.

[pcc]

I reproduced the bug, looking.