[PATCH] D35639: [LTO] Prevent dead stripping and internalization of symbols with sections

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

`s/__end_/__stop_/` in a bunch of places.



================
Comment at: include/llvm/Object/IRSymtab.h:130
+  /// suppress the generation of the special __start_ and __end_ symbols.
+  bool HasELFCIdentifierSectionName;
 };
----------------
I'd prefer to put the entire section name in here so that we can use it for the things I mentioned earlier.

Otherwise this can be moved to Symbol::Flags, there's plenty of space there.


================
Comment at: test/tools/gold/X86/global_with_section.ll:10
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:     -u foo \
+; RUN:     --plugin-opt=save-temps \
----------------
Is the "-u foo" part necessary? Same below.


================
Comment at: test/tools/gold/X86/global_with_section.ll:45
+; CHECK-DAG: define void @deadfunc() section "some_other_section"
+define void @deadfunc() section "some_other_section" {
+  call void @deadfunc2()
----------------
Probably also want a negative test that shows we don't do this to functions with a section name that isn't a C identifier.


================
Comment at: tools/gold/gold-plugin.cpp:624
   for (ld_plugin_symbol &Sym : F.syms) {
+    assert(SymNum < NumInputFileSyms);
+    const InputFile::Symbol &InpSym = InputFileSyms[SymNum];
----------------
Probably a little more precise to do `assert(InputFileSyms.size() == F.syms.size())` outside of the loop instead.


================
Comment at: tools/gold/gold-plugin.cpp:671
     }
 
     if (Resolution != LDPR_RESOLVED_DYN && Resolution != LDPR_UNDEF &&
----------------
I'd move the section name check here outside the switch statement.


================
Comment at: tools/llvm-lto2/llvm-lto2.cpp:360
+      if (TT.isOSBinFormatELF() && Sym.hasELFCIdentifierSectionName())
+        outs() << "         has C idenfier section name\n";
     }
----------------
identifier


https://reviews.llvm.org/D35639