[PATCH] D33680: [ELF] - Resolve references properly when using .symver directive

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 20 19:53:25 PDT 2017


ruiu added inline comments.


================
Comment at: ELF/SymbolTable.cpp:728
+  // Let them parse and update their names to exclude version suffix.
+  for (size_t I = 0, N = SymVector.size(); I < N; ++I) {
+    SymbolBody *Body = SymVector[I]->body();
----------------
Now that you can use a for-each loop, no?


================
Comment at: ELF/Symbols.cpp:259
   // It is an error if the specified version is not defined.
-  error(toString(File) + ": symbol " + S + " has undefined version " + Verstr);
+  if (Config->Shared)
+    error(toString(File) + ": symbol " + S + " has undefined version " +
----------------
grimar wrote:
> ruiu wrote:
> > Why did you add this?
> Explanation is below (from my earlier answer about the same change to Rafael in llvm-mails)
> Looks I should have add comment here, I did that.
> 
> >Why do you needed to add "if (Config->Shared)"? The patch looks good
> >otherwise, but I would like to understand this part.
> >
> >Cheers,
> >Rafael
> 
> Testcase does not specify --version-script and would fail otherwise with "has undefined version error",
> after doing this change we are consistent with gnu linkers which do not require version script file for such case when
> building executable.
> 
> And reason for that I believe is next. Spec (https://sourceware.org/binutils/docs/as/Symver.html) says:
> "Use the .symver directive to bind symbols to specific version nodes within a source file. This is only supported on ELF platforms,
>  and is typically used when assembling files to be linked into a shared library. There are cases where it may make sense to use this 
> in objects to be bound into an application itself so as to override a versioned symbol from a shared library."
> (last sentence).
> 
> Example. Imagine dso.s:
> 
> ```
> .globl foo1
> foo1:
>   ret
> .globl foo2
> foo2:
>   ret
> ```
> 
> And version script ver.txt: VERSION_1.0 { global : foo1; foo2; local : *; };
> 
> Build DSO:
> llvm-mc -filetype=obj -triple=x86_64-pc-linux dso.s -o dso.o 
> ld.bfd dso.o -shared -o dso.so --version-script ver.txt
> 
> Now we want to override one of foo() in executable. Usually version script is not provided when linking executable,
> so we probably should be able just to do:
> 
> (app.s)
> 
> ```
> .global zed
> .symver zed, foo1@@VERSION_1.0
> zed:
>   .quad 0
> 
> .global _start
> _start:
>  call foo1
>  call foo2
> ```
> 
> llvm-mc -filetype=obj -triple=x86_64-pc-linux app.s -o app.o 
> ld.bfd app.o dso.so -o out
> 
> And end up with overriden foo1:
>     17: 00000000006003b0     0 NOTYPE  GLOBAL DEFAULT   10 foo2@@VERSION_1.0
>     18: 00000000006003b0     0 NOTYPE  GLOBAL DEFAULT   10 _edata
>     19: 00000000006003b0     0 NOTYPE  GLOBAL DEFAULT   10 _end
>     20: 0000000000400278     0 NOTYPE  GLOBAL DEFAULT    8 foo1@@VERSION_1.0
> 
> Without "if (Config->Shared)" we would error out here.
Why do you want to support that behavior?


https://reviews.llvm.org/D33680





More information about the llvm-commits mailing list