[PATCH] [lld][ELF] Add support for reading dynamic libraries.

Michael Spencer bigcheesegs at gmail.com
Fri Feb 8 15:56:18 PST 2013



================
Comment at: lib/ReaderWriter/ELF/DynamicFile.h:51-60
@@ +50,12 @@
+    // it exists.
+    for (auto i = obj.begin_elf_dynamic_symbols(),
+              e = obj.end_elf_dynamic_symbols();
+         i != e; ++i) {
+      StringRef name;
+      if (error_code ec =
+              obj.getSymbolName(obj.getDynamicSymbolTableSectionHeader(), &*i,
+                                name))
+        return ec;
+      file->_nameToSym[name].first = &*i;
+    }
+
----------------
Shankar Kalpathi Easwaran wrote:
> Michael Spencer wrote:
> > Shankar Kalpathi Easwaran wrote:
> > > Just reading the top level library wont be enough. 
> > > 
> > > For example :-
> > > 
> > > cat > 1.c << \!
> > > int a;
> > > !
> > > 
> > > cat > 2.c << \!
> > > int b;
> > > !
> > > 
> > > gcc -fPIC -shared 1.c -o lib1.so
> > > gcc -fPIC -shared  2.c -L. -l1 -o lib2.so
> > > 
> > > $readelf --dyn-syms lib2.so | grep -w a\$
> > > $readelf --dyn-syms lib1.so | grep -w a\$
> > >      9: 0000000000201014     4 OBJECT  GLOBAL DEFAULT   23 a
> > > 
> > > 
> > > The symbol a would be only shown in lib1.so but not in lib2.so.
> > You have to explicitly link against lib2.so anyway.
> > 
> >   $ echo "int a;" | clang -fPIC -shared -x c - -o liba.so
> >   $ echo "extern int a; int foo() {return a;}" | clang -fPIC -shared -x c - -o libfoo.so -L. -la -Wl,-rpath=.
> >   $ echo "extern int a; int foo(); int main() {return foo() + a;} " | clang -fPIE -x c - -o shared -L. -lfoo -Wl,-rpath=.
> >   /usr/bin/ld: /tmp/--pyGKwV.o: undefined reference to symbol 'a'
> >   /usr/bin/ld: note: 'a' is defined in DSO ./liba.so so try adding it to the linker command line
> >   ./liba.so: could not read symbols: Invalid operation
> >   clang-3: error: linker command failed with exit code 1 (use -v to see invocation)
> Not sure when did they change this behaviour, solaris/hp-ux was searching dependencies too, strange. Dont you feel this makes it harder for developers ?
It's the same semantics as static libraries. Link directly to what you use.

================
Comment at: lib/ReaderWriter/ELF/DynamicFile.h:65-66
@@ +64,4 @@
+
+  virtual const atom_collection<DefinedAtom> &defined() const {
+    return _definedAtoms;
+  }
----------------
Shankar Kalpathi Easwaran wrote:
> Michael Spencer wrote:
> > Shankar Kalpathi Easwaran wrote:
> > > definedAtoms. undefinedAtoms are not being populated here.
> > There's no reason to. We only care about symbols if they are referenced. We shouldn't waste time creating them otherwise.
> cat > 1.c << \!
> int main()
> {
>   fn1();
> }
> !
> 
> cat > 2.c << \!
> int fn1() {
>   fn2();
>   return 0;
> }
> !
> 
> cat > 3.c << \!
> int fn2() {
>   return 0;
> }
> !
> 
> 
> gcc -c 3.c
> ar cr lib3.a 3.o
> gcc -shared -fPIC 2.c -o lib2.so
> # passes link
> gcc 1.c -L. -l2 -l3
> # fails because fn2 is not resolved
> gcc 1.c -L. -l2
> 
Ah, so undefined symbols in the dynamic symbol table need to be read. But only if we actually use the shared object. This is also only when creating a final executable, not a shared object.

================
Comment at: lib/ReaderWriter/ELF/Reader.cpp:162-163
@@ -65,4 +161,4 @@
 
     std::size_t MaxAlignment =
         1ULL << llvm::CountTrailingZeros_64(uintptr_t(mb->getBufferStart()));
 
----------------
Shankar Kalpathi Easwaran wrote:
> Michael Spencer wrote:
> > Shankar Kalpathi Easwaran wrote:
> > > Why MaxAlignment for shared libraries too ?
> > Because there's no reason not to.
> The load address would be determined by the dynamic loader only. I dont see why linker is doing that.
The linker isn't doing that. This is purely for reading the file at link time.

================
Comment at: lib/ReaderWriter/ELF/Reader.cpp:168-170
@@ -71,61 +167,5 @@
     case llvm::sys::ELF_Relocatable_FileType: {
-      std::pair<unsigned char, unsigned char> Ident = getElfArchType(&*mb);
-      std::unique_ptr<File> f;
-      // Instantiate the correct File template instance based on the Ident
-      // pair. Once the File is created we push the file to the vector of files
-      // already created during parser's life.
-      if (Ident.first == llvm::ELF::ELFCLASS32 &&
-          Ident.second == llvm::ELF::ELFDATA2LSB) {
-#if !LLVM_IS_UNALIGNED_ACCESS_FAST
-        if (MaxAlignment >= 4)
-          f.reset(new ELFFile<ELFType<llvm::support::little, 4, false> >(
-                          _elfTargetInfo, std::move(mb), ec));
-        else
-#endif
-        if (MaxAlignment >= 2)
-          f.reset(new ELFFile<ELFType<llvm::support::little, 2, false> >(
-                          _elfTargetInfo, std::move(mb), ec));
-        else
-          llvm_unreachable("Invalid alignment for ELF file!");
-      } else if (Ident.first == llvm::ELF::ELFCLASS32 &&
-                 Ident.second == llvm::ELF::ELFDATA2MSB) {
-#if !LLVM_IS_UNALIGNED_ACCESS_FAST
-        if (MaxAlignment >= 4)
-          f.reset(new ELFFile<ELFType<llvm::support::big, 4, false> >(
-                          _elfTargetInfo, std::move(mb), ec));
-        else
-#endif
-        if (MaxAlignment >= 2)
-          f.reset(new ELFFile<ELFType<llvm::support::big, 2, false> >(
-                          _elfTargetInfo, std::move(mb), ec));
-        else
-          llvm_unreachable("Invalid alignment for ELF file!");
-      } else if (Ident.first == llvm::ELF::ELFCLASS64 &&
-                 Ident.second == llvm::ELF::ELFDATA2MSB) {
-#if !LLVM_IS_UNALIGNED_ACCESS_FAST
-        if (MaxAlignment >= 8)
-          f.reset(new ELFFile<ELFType<llvm::support::big, 8, true> >(
-                          _elfTargetInfo, std::move(mb), ec));
-        else
-#endif
-        if (MaxAlignment >= 2)
-          f.reset(new ELFFile<ELFType<llvm::support::big, 2, true> >(
-                          _elfTargetInfo, std::move(mb), ec));
-        else
-          llvm_unreachable("Invalid alignment for ELF file!");
-      } else if (Ident.first == llvm::ELF::ELFCLASS64 &&
-                 Ident.second == llvm::ELF::ELFDATA2LSB) {
-#if !LLVM_IS_UNALIGNED_ACCESS_FAST
-        if (MaxAlignment >= 8)
-          f.reset(new ELFFile<ELFType<llvm::support::little, 8, true> >(
-                          _elfTargetInfo, std::move(mb), ec));
-        else
-#endif
-        if (MaxAlignment >= 2)
-          f.reset(new ELFFile<ELFType<llvm::support::little, 2, true> >(
-                          _elfTargetInfo, std::move(mb), ec));
-        else
-          llvm_unreachable("Invalid alignment for ELF file!");
-      }
-      if (!ec)
-        result.push_back(std::move(f));
+      std::unique_ptr<File> f(createELF<ELFFile>(getElfArchType(&*mb),
+                                                 MaxAlignment, _elfTargetInfo,
+                                                 std::move(mb), ec));
+      if (ec)
----------------
Shankar Kalpathi Easwaran wrote:
> Michael Spencer wrote:
> > Shankar Kalpathi Easwaran wrote:
> > > Why is the return value not taken and compared, may be a TODO action item can be set at this location.
> > Because it can't fail.
> If Line 141,   llvm_unreachable("Invalid ELF type!") is changed to return an error, this could fail too 
The return type of createELF is whatever the type trait says it is, so we have no idea what to return. It would be possible to add a getFailure() function to CreateELFTraits though.

================
Comment at: lib/ReaderWriter/ELF/DynamicFile.h:30-31
@@ +29,4 @@
+
+    static uint32_t lastOrdinal = 0;
+    file->_ordinal = lastOrdinal++;
+
----------------
Shankar Kalpathi Easwaran wrote:
> Shankar Kalpathi Easwaran wrote:
> > This should be the order of the file that appears in the link. Having static here would not be correct.
> ?
This gets reassigned by the order pass, I believe. This is what every other File does currently, and I haven't looked into how it's supposed to actually work.

================
Comment at: lib/ReaderWriter/ELF/DynamicFile.h:77-79
@@ +76,5 @@
+
+  virtual const atom_collection<AbsoluteAtom> &absolute() const {
+    return _absoluteAtoms;
+  }
+
----------------
Shankar Kalpathi Easwaran wrote:
> Shankar Kalpathi Easwaran wrote:
> > same here for absolute atoms
> absolute symbols dont take part in symbol resolution, ld gives a vague error.
> 
> cat > 1.c << \!
> extern int ABS;
> int main()
> {
>   fn1();
>   printf("%d\n",&ABS);
> }
> !
> 
> cat > 2.s << \!
>         .file   "2.c"
>         .global ABS
>          ABS=1
>         .text
>         .p2align 4,,15
>         .globl  fn1
>         .type   fn1, @function
> fn1:
> .LFB0:
>         .cfi_startproc
>         xorl    %eax, %eax
>         ret
>         .cfi_endproc
> .LFE0:
>         .size   fn1, .-fn1
>         .ident  "GCC: (Ubuntu/Linaro 4.7.2-11lucid3) 4.7.2"
>         .section        .note.GNU-stack,"", at progbits
> !
> 
> gcc -shared -fPIC 2.s -o lib2.so
> gcc 1.c -L. -l2
> 
k


http://llvm-reviews.chandlerc.com/D382



More information about the llvm-commits mailing list