[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