llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.92k stars 11.52k forks source link

.set with an offset inconsistently marked as no-dead-strip on Mach-O #104623

Open smeenai opened 4 weeks ago

smeenai commented 4 weeks ago

For the following C code:

int main(int argc, char *argv[]) {
  return argc ? (long)__builtin___CFStringMakeConstantString("foo")
              : (long)__builtin___CFStringMakeConstantString("bar");
}

If we build it with -O3 on top of a Clang which contains #101222 (which enables private globals to be merged), the resulting object file is different if we compile to an object directly vs. if we compile to assembly and then compile that assembly:

$ clang -target arm64-apple-macos11 -O3 -c merge.c
$ llvm-nm -m merge.o | grep l__unnamed_cfstring_.2
0000000000000040 (__DATA,__cfstring) non-external [alt entry] l__unnamed_cfstring_.2

$ clang -target arm64-apple-macos11 -O3 -S -o merge.s merge.c
$ llvm-mc --triple=arm64-apple-macos11 --filetype=obj -o merge2.o merge.s
$ llvm-nm -m merge2.o | grep l__unnamed_cfstring_.2
0000000000000040 (__DATA,__cfstring) non-external [no dead strip] [alt entry] l__unnamed_cfstring_.2

Notice that the symbol is marked as no-dead-strip when built from assembly but not when built from source directly. The reason is that the generated assembly contains the following:

.set l__unnamed_cfstring_.2, __MergedGlobals+32

Which triggers this logic in MC: https://github.com/llvm/llvm-project/blob/907c7eb311077c5da434bf67858b1173a351d800/llvm/lib/MC/MCParser/AsmParser.cpp#L2959

If I instead change the line to:

l__unnamed_cfstring_.2 = __MergedGlobals+32

the symbol is not marked as no-dead-strip, which matches the Clang output. It's strange for direct object file emission to not produce the same result as generating assembly and then compiling the assembly; perhaps Clang should be using = instead of .set in its generated assembly? CC @MaskRay

MaskRay commented 3 weeks ago

the symbol is not marked as no-dead-strip, which matches the Clang output. It's strange for direct object file emission to not produce the same result as generating assembly and then compiling the assembly; perhaps Clang should be using = instead of .set in its generated assembly? CC @MaskRay

The special behavior for .set looks odd. It's from b7b750d480ff7714e65f4b92ec1a43be4a9acd30 (2012).

I agree that perhaps Clang should emit = (without the strange behavior; matches direct object code emission) instead of .set.

Is it feasible to make this opt-in with a cl::opt option? Unfortunately Apple rdar://12219394 is private.

smeenai commented 3 weeks ago

Thanks for finding the original commit; I was struggling with that. Maybe we can ask someone from Apple if that radar is still relevant? The commit mentions gas compatibility, which I imagine Apple hasn't cared about for a long time, but now there might be places depending on the MC behavior.

That being said, do you think Clang needs a cl::opt for this? I don't think it's very common for end users to use clang -S and then compile the resulting assembly to an object file (vs. just generating the object file directly); I imagine it's mostly compiler developers using that flow, and those developers would probably expect identical results with either flow.

MaskRay commented 3 weeks ago

Thanks for finding the original commit; I was struggling with that. Maybe we can ask someone from Apple if that radar is still relevant? The commit mentions gas compatibility, which I imagine Apple hasn't cared about for a long time, but now there might be places depending on the MC behavior.

That being said, do you think Clang needs a cl::opt for this? I don't think it's very common for end users to use clang -S and then compile the resulting assembly to an object file (vs. just generating the object file directly); I imagine it's mostly compiler developers using that flow, and those developers would probably expect identical results with either flow.

I agree that we should fix the inconsistent behavior (drop no-dead-strip). I hope we don't add a cl::opt:)