Has this review been abandoned? If so, is there any other tooling to generate CG profile input to lld from an iPGO profile?

Updated after recent commits.

@pcc : poke

Added a new CodeGetCXX test case

Any chance this gets fixed soon? I'm hitting the same problem.

Changed the checks to look for dso_local instread of visibility.

Can you guys please give it a try with Chromium? Do you get any measurable performance improvements with this change?

Changed the code not to short circuit functions that can be overridden at run time and not to expose <func>.cfi names to the runtime.
Added a test point to check for that.

Oh, this must be with cross-DSO CFI. I see what the problem is. We need to skip symbols not resolved internally from replacing direct calls to calls to <name>.cfi. An extra test case wound't hurt, so if you have it please send it to me.

In D46326#1105694, @vlad.tsyrklevich wrote:

Based on a quick look at a couple of stack traces it looks like some direct
calls now call symbols that are no longer overriden by their replacements
in chromium, e.g. calls to realloc() from a DSO previously resolved to the
tcmalloc implementation in chromium instead of realloc.cfi(), the locally
linked tcmalloc implementation in that library.

Sorry, I'm traveling this week, didn't see this until now. I didn't get any bot failure notifications after the commit, so I assumed it was good. Do you have a test case I can use to reproduce it?

Fixed a few missed cases where direct calls were routed through jump tables.
Added more test points for thin and full LTO.

Added a test case.
Skip direct calls in addition to blockaddr uses when dealing with CFI.
Don't set linkage to internal for .cfi symbols as they may be needed by direct calls.

ToT LTO and ThinLTO builds both work fine for me. Can you post the error you get? I'm not sure how ToT would produce a direct call to a function with a jumptable entry.

$ clang++ -fuse-ld=lld -flto=thin -fsanitize=cfi-icall -fvisibility=hidden -o ex ex1.cpp ex2.cpp
lld: error: undefined symbol: void X<double>() (.cfi)

referenced by ex1.cpp

lto.tmp:(main)

No, that statement is doing the reverse of what you're doing. It's replacing directs calls to a function to calls to the jumptable entry. (F/FDecl have different meanings in the 2 other cases than how you're using them.)

Sounds like we need to skip direct calls in replaceUsesExceptBlockAddr(). Make it replaceUsesExceptBlockAddrOrDirectCalls()

No, that statement is doing the reverse of what you're doing. It's replacing directs calls to a function to calls to the jumptable entry. (F/FDecl have different meanings in the 2 other cases than how you're using them.)

In D46326#1084593, @vlad.tsyrklevich wrote:

Also adding a FDecl->replaceDirectCalls(F) in the else block after line 1016 seems like it would also correctly rewrite direct calls for the other two cases (external functions, and local functions declared in the current TU.)

Yeah, need a test case.

Addressed review comments.

Changed deleteMember() to erase_if()

Changed the implementation to avoid writing the archive twice.

llvm-ar supports only a subset of MRI commands, that's why 'delete' as implemented here only works for a specific use case: deleting members added with 'addlib' or 'addmod' commands during archive creation.

Fixed the test.

Re-wrote the test.

Changed the test to create linker script with "echo" instead.

Minor tweaks.

Addressed review comments

And the set type is changed to SmallSetVector.

Changed the fix to save unique Constant users and process them separately outside of the UseList iterator.

Nag... Can you guys please review this?

Can someone please click "Approve"? ;)