[PATCH] D48771: [ELF] Don't create empty output section for unreferenced PROVIDE symbol

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Rather than rush this in before I go home today, I'll wait until Monday before putting it in, assuming there are no objections by then.



================
Comment at: ELF/LinkerScript.cpp:841
+      // symbols.
+      if (Cmd->Provide)
+        continue;
----------------
grimar wrote:
> I also was wondered if we might want to abstract from `Provide` here and use something like
> 
> ```
> if (Cmd->Name != "." && !Cmd->Sym)
> ...
> ```
> 
> But I am not sure it worth doing.
My one concern with this change is that it relies on us setting Provide to false when we define a symbol because of it. I'll try with your suggested change on Monday before putting this in, as I think it feels a little safer. Are you happy with it going straight in if I make that change?


================
Comment at: test/ELF/linkerscript/provide-empty-section.s:5
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %tdefined.o -defsym="foo=42"
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %S/Inputs/provide-empty-section.s -o %treference.o
+
----------------
grimar wrote:
> I would avoid using additional input file and `-defsym` and be more explicit
> (Just echoing code to llvm-mc is the common way when there is a little amount of code I believe).
> 
> ```
> # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %tundefined.o
> # RUN: echo "foo=42" | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %tdefined.o
> # RUN: echo "call foo" | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %treference.o
> ```
Ah, I didn't know about that technique. I'll make that change, no problem.


================
Comment at: test/ELF/linkerscript/provide-empty-section.s:11
+# RUN: echo "  }" >> %t.script
+# RUN: echo "}" >> %t.script
+
----------------
grimar wrote:
> Let's do it a single line. That is what we usually try to do if possible and also it avoids multiple echo calls.
> 
> `# RUN: echo "SECTIONS { .bar : { PROVIDE(foo = .); } }" > %t.script`
Will do.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D48771