ThinLTO importing function with inline ASM can't lead to duplicate symbols

[joker-eph]

Bugzilla Link	33812
Version	unspecified
OS	All
Attachments	File with inline asm, caller
CC	@ahatanak,@dexonsmith,@rnk

Extended Description

For example the two attached files illustrate the issue. The inline ASM is declaring a global symbol and after importing and inlining we end up with the symbol redeclared. On Mac I see:

$ clang test_caller.cpp test_asm.ll -flto=thin duplicate symbol __dtrace_probe in: /var/folders/yy/prvy9rf51xvbr26mdg9x00q1vtv3gs/T/cc-892b93.o/0.thinlto.o /var/folders/yy/prvy9rf51xvbr26mdg9x00q1vtv3gs/T/cc-892b93.o/1.thinlto.o ld: 1 duplicate symbol for architecture x86_64 clang: error: linker command failed with exit code 1 (use -v to see invocation)

Conservatively: we could disable the importing for functions that contains inline ASM.

[rnk]

Hm, that's tough. Reminds me of what ThinLTO uses GUIDs for when it promotes internal symbols to external symbols.

I still want to say it is user error. There are many passes that assume they can duplicate IR arbitrarily, like loop unswitching, inlining, simplifycfg, speculation, whatever. I think there is a 'noduplicate' attribute that might help here. We might want to expose that to users.

For now, the best we can do might be to tell them to slap 'noinline' on the function with the probe if it can't tolerate duplication.

[joker-eph]

I'm not totally sure yet.

These are from dtrace probe macros inserted in the XNU kernel. I don't see a user-friendly way for the user to express these as module level assembly.

I agree with the "%=" remark, and this is what is in the real-world example I believe, I simplified the repro.

My impression is that it ensures a single ID per blob per module?

That makes it robust against inlining, but only because inlining happens in the same file usually.

Here we import across file and inline, so the "unique blob ID" does not seem to work.

[llvmbot]

Read it more carefully and agree with rnk. Should we close this?

[rnk]

I would say this is user error. They can use module asm if they want to define something exactly once. If they want the labels and probes to be duplicatable, then they can use %=, which gives you a number unique to the asm blob.

[llvmbot]

FWIW, a slightly modified version of this (to make sure the ASM syntax actually assembles) doesn't crash on ELF

s/crash/fail/

[llvmbot]

FWIW, a slightly modified version of this (to make sure the ASM syntax actually assembles) doesn't crash on ELF

i.e.

Replacing: call void asm sideeffect ".section DATA, data\0A\09.globl __dtrace_probe\0A\09__dtrace_probe:.quad 1f\0A\09.text\0A\091:nop\0A\09nop\0A\09nop\0A\09", "r,~{memory},~{rdi},~{dirflag},~{fpsr},~{flags}"(i64* nonnull %0)

with: call void asm sideeffect ".globl __dtrace_probe\0A\09__dtrace_probe:.quad 1f\0A\09.text\0A\091:nop\0A\09nop\0A\09nop\0A\09", "r,~{memory},~{rdi},~{dirflag},~{fpsr},~{flags}"(i64* nonnull %0)

$ ../clang test_caller.cpp test_asm.ll -flto=thin -o blah warning: overriding the module target triple with x86_64-unknown-linux-gnu [-Woverride-module] 1 warning generated.

Otherwise we would get an error assembling:

$ ../clang test_caller.cpp test_asm.ll -flto=thin warning: overriding the module target triple with x86_64-unknown-linux-gnu [-Woverride-module] 1 warning generated.

:1:19: error: expected string in directive .section __DATA, __data ^ LLVM ERROR: Error parsing inline asm clang-5.0: error: linker command failed with exit code 1 (use -v to see invocation) Sorry, I don't have a Mac and this doesn't repro on any of my platforms :)