[PATCH] D41684: [llvm-objcopy] Add --localize-hidden option

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 3 03:58:31 PST 2018

jhenderson added inline comments.

Comment at: test/tools/llvm-objcopy/localize-hidden.test:35
+  Global:
+    - Name:     _start
Why do you have so many different symbols here?

Also, you should have symbols with hidden visibility with both local and weak bindings, to show that a) local symbols remain unchanged, and b) weak symbols are changed.

Finally, a symbol with STV_PROTECTED visibility should also be included to show that it is not localized.

Comment at: tools/llvm-objcopy/Object.cpp:184-188
+  // Now that the local symbols aren't grouped at the start we have to reorder
+  // the symbols to respect this property.
+  std::stable_partition(
+      std::begin(Symbols), std::end(Symbols),
+      [](const SymPtr &Sym) { return Sym->Binding == STB_LOCAL; });
I'd add blank lines either side of this block.



More information about the llvm-commits mailing list