[PATCH] D18772: [lld] Implement --dynamic-list

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/DynamicList.cpp:34
@@ +33,3 @@
+//
+//  { symbol1; symbol2; extern "C++" { symbol3; namespace::symbol4 }; }
+//
----------------
zatrazz wrote:
> ruiu wrote:
> > I also don't like to support C++ mangled names now. There's a chance that we might want that later, but this patch is already too large, so please remove that feature.
> Although llvm itself does not use, I see no direct reason to not support. It does not interfere significantly with default matching rules (for glob patterns it will first check the language mangling and only applying the damangling in such case) if the script does not use it.
Apparently this part of linker script was not designed with speed in mind, and I think the gnu linker went a bit too far on this front. How many users are really using this feature? Is this really what we want to have? We need to think carefully to decide whether we want to do the same thing as the gnu linker for mangled names.

================
Comment at: ELF/ScriptParser.cpp:23-25
@@ -21,1 +22,5 @@
 
+// Returns true if S matches T. S can contain glob meta-characters.
+// The asterisk ('*') matches zero or more characacters, and the question
+// mark ('?') matches one character.
+bool elf::matchGlobStr(StringRef S, StringRef T) {
----------------
zatrazz wrote:
> ruiu wrote:
> > At least for now, I don't want to support glob patterns at all. Glob pattern can have huge performance impact on the linking speed. In most cases users pass a list of complete symbol names, so we should reject any patterns that contains glob metacharacters and do simple hash table lookup instead of glob pattern matching.
> As stated in http://llvm.org/pr26693 it is required for the sanitizers and at least from LLVm side it the main motivator for this patch. I think we can add an additional check on DynamicList to check if an dynamic list and fast return if it is not the case.
Do you need the glob pattern fro the sanitizer?


Repository:
  rL LLVM

http://reviews.llvm.org/D18772