[PATCH] [lld][ELF] Add support for reading dynamic libraries.
Shankar Kalpathi Easwaran
shankarke at gmail.com
Fri Feb 8 14:38:50 PST 2013
================
Comment at: lib/ReaderWriter/ELF/DynamicFile.h:65-66
@@ +64,4 @@
+
+ virtual const atom_collection<DefinedAtom> &defined() const {
+ return _definedAtoms;
+ }
----------------
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
================
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;
+ }
+
----------------
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 ?
================
Comment at: lib/ReaderWriter/ELF/DynamicFile.h:30-31
@@ +29,4 @@
+
+ static uint32_t lastOrdinal = 0;
+ file->_ordinal = lastOrdinal++;
+
----------------
Shankar Kalpathi Easwaran wrote:
> This should be the order of the file that appears in the link. Having static here would not be correct.
?
================
Comment at: lib/ReaderWriter/ELF/DynamicFile.h:77-79
@@ +76,5 @@
+
+ virtual const atom_collection<AbsoluteAtom> &absolute() const {
+ return _absoluteAtoms;
+ }
+
----------------
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
================
Comment at: lib/ReaderWriter/ELF/DynamicFile.h:85
@@ +84,3 @@
+ auto sym = _nameToSym.find(name);
+ if (sym == _nameToSym.end())
+ return nullptr;
----------------
Michael Spencer wrote:
> Shankar Kalpathi Easwaran wrote:
> > The dynamic symbols may have undefined symbols and absolute symbols too, which the export should not pick.
> It shouldn't pick undefined. It can pick absolute.
See above example. It shouldnt pick absolute
================
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()));
----------------
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.
================
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)
----------------
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
http://llvm-reviews.chandlerc.com/D382
More information about the llvm-commits
mailing list