[PATCH] [ELF] Implement parsing `-l` prefixed items in the `GROUP` linker script command

Shankar Kalpathi Easwaran shankarke at gmail.com
Fri Jul 11 07:33:41 PDT 2014


================
Comment at: include/lld/ReaderWriter/LinkerScript.h:76
@@ +75,3 @@
+  /// indicate an error.
+  StringRef::size_type sizeOfLibName();
+
----------------
Should this be a ErrorOr<StringRef::size_type> ?

================
Comment at: include/lld/ReaderWriter/LinkerScript.h:76
@@ +75,3 @@
+  /// indicate an error.
+  StringRef::size_type sizeOfLibName();
+
----------------
Shankar Kalpathi Easwaran wrote:
> Should this be a ErrorOr<StringRef::size_type> ?
const.

================
Comment at: include/lld/ReaderWriter/LinkerScript.h:76
@@ +75,3 @@
+  /// indicate an error.
+  StringRef::size_type sizeOfLibName();
+
----------------
Shankar Kalpathi Easwaran wrote:
> Shankar Kalpathi Easwaran wrote:
> > Should this be a ErrorOr<StringRef::size_type> ?
> const.
Do we have a test that covers the error case ?

================
Comment at: lib/ReaderWriter/ELF/ELFLinkingContext.cpp:169
@@ +168,3 @@
+      hasColonPrefix ? libName.drop_front() : Twine("lib", libName) + ".so";
+  Twine aName =
+      hasColonPrefix ? libName.drop_front() : Twine("lib", libName) + ".a";
----------------
aName=> archiveName ?

================
Comment at: lib/ReaderWriter/LinkerScript.cpp:127
@@ +126,3 @@
+      if (end == StringRef::npos || end == 0)
+        break;
+      StringRef word = _buffer.substr(0, end);
----------------
raise an error here.

================
Comment at: lib/ReaderWriter/LinkerScript.cpp:203
@@ +202,3 @@
+
+  if (!canStartName(_buffer[start++]))
+    return StringRef::npos;
----------------
I think we need canStartLibraryName, as canStartName returns true for cases where a library name would be invalid.

For example the below cases.

  case '_': case '.': case '$': case '/': case '\\':
    return true;

We should reject invalid library names IMO

================
Comment at: lib/ReaderWriter/LinkerScript.cpp:209
@@ +208,3 @@
+  if (endIter == _buffer.end())
+    return StringRef::npos;
+
----------------
error here ?

http://reviews.llvm.org/D4469






More information about the llvm-commits mailing list