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

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 24 16:41:50 PDT 2017

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";


More information about the llvm-commits mailing list