[PATCH] D65819: [Driver][Bundler] Improve bundling of object files.

[Alexey Bataev via cfe-commits]

I said this before that most probably this is the bug in partial linking, made by ld. But clang-offload-bundler is also nit quite correct here since the original host code is actually not unbundled. I'll fix the patch later today to keep the partial linking for the host object, but really unbundle it when required.

Best regards,
Alexey Bataev

> 14 авг. 2019 г., в 5:48, Jonas Hahnfeld via Phabricator <reviews at reviews.llvm.org> написал(а):
> 
> Hahnfeld added a comment.
> 
> Okay, so I wasn't happy with the current explanations because I don't like "fixing" an issue without understanding the problem. Here's a small reproducer:
> 
>  $  cat main.cpp 
>  #include "test.h"
> 
>  int main(int argc, char *argv[]) {
>    m[1] = 2;
>    return 0;
>  }
>  $ cat test.h 
>  #include <map>
>  #include <vector>
> 
>  template <typename T>
>  struct B {
>    static std::vector<int> v;
> 
>    virtual void f() {
>      v.push_back(1);
>    }
>  };
> 
>  struct C : public B<int> {
>    C() { }
>  };
> 
>  template <typename T>
>  std::vector<int> B<T>::v;
> 
>  extern std::map<int, int> m;
>  $ cat test.cpp 
>  #include "test.h"
> 
>  std::map<int, int> m;
> 
> Compiling with `clang++ -c main.cpp`, `clang++ -c test.cpp`, `clang++ main.o test.o -o main` works fine and the resulting executable doesn't crash.
> Now if instead partially linking `test.o` like `ld -r test.o -o test.o.ld-r` and then linking the executable with `clang++ main.o test.o.ld-r -o main.ld-r` outputs a binary that crashes at execution because the constructor of `std::map<int> m` is not called.
> 
> Digging in the object files reveals the reason:
> 
>  $ readelf -S test.o | grep -n1 .init_array
>  281-       0000000000000008  0000000000000000 WAG       0     0     8
>  282:  [138] .init_array       INIT_ARRAY       0000000000000000  000009d0
>  283-       0000000000000008  0000000000000000 WAG       0     0     8
>  284:  [139] .rela.init_array  RELA             0000000000000000  00002460
>  285-       0000000000000018  0000000000000018   G      150   138     8
>  286:  [140] .init_array       INIT_ARRAY       0000000000000000  000009d8
>  287-       0000000000000008  0000000000000000  WA       0     0     8
>  288:  [141] .rela.init_array  RELA             0000000000000000  00002478
>  289-       0000000000000018  0000000000000018          150   140     8
>  $ readelf -S test.o.ld-r | grep -n1 .init_array
>  279-       0000000000000078  0000000000000000   A       0     0     4
>  280:  [137] .init_array       INIT_ARRAY       0000000000000000  00001270
>  281-       0000000000000010  0000000000000008 WAG       0     0     8
>  282:  [138] .rela.init_array  RELA             0000000000000000  00003b10
>  283-       0000000000000030  0000000000000018  IG      147   137     8
> 
> I haven't further looked into the contents of the sections, but I'd guess that the first `.init_array` in `test.o` contains the constructor for `template <typename T> std::vector<int> B<T>::v`. Because it might need to be merged with other TUs (in this case, it's also present in `main.o`) it is marked with a group flag (`G`). The second `.init_array` in `test.o` is probably the constructor for `std::map<int, int> m` and not marked with a group, but `ld -r` merges these two sections into one `.init_array`, now with a group. So when linking with `main.o` which also contains a constructor for `template <typename T> std::vector<int> B<T>::v`, the linker drops the call to the constructor of `std::map<int, int> m`:
> 
>  $ readelf -S main | grep -n1 .init_array
>  43-       0000000000000160  0000000000000000   A       0     0     4
>  44:  [19] .init_array       INIT_ARRAY       0000000000006d98  00005d98
>  45-       0000000000000018  0000000000000008  WA       0     0     8
>  $ readelf -S main.ld-r | grep -n1 .init_array
>  43-       0000000000000160  0000000000000000   A       0     0     4
>  44:  [19] .init_array       INIT_ARRAY       0000000000006da0  00005da0
>  45-       0000000000000010  0000000000000008  WA       0     0     8
> 
> (note the difference in size of the two `.init_array` sections!)
> 
> Further notes:
> 
> - Obviously, linking in a different order like `clang++ test.o.ld-r main.o -o main.ld-r2` also results in a working executable, but that's not really a solution.
> - Calling `ld -Ur` doesn't change anything:
> 
>  $ ld -Ur test.o -o test.o.ld-Ur
>  $ md5sum test.o.ld-*
>  a4d5cead3209ef191d5c05de63e398de  test.o.ld-r
>  a4d5cead3209ef191d5c05de63e398de  test.o.ld-Ur
> 
> ---
> 
> Long story short: This very much looks like a bug in `ld` when using partial linking. So the best thing that Clang can do to (kind of) workaround this problem is ensuring that bundling + unbundling results in the bitwise-same host object file. However, I think we should still use partial linking for easy access to the host object file, even if we don't extract it from there (other tools using it from there have to blame `ld -r`, not `clang-offload-bundler`, that the partially linked object file doesn't correctly call global initializers).
> 
> 
> Repository:
>  rC Clang
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D65819/new/
> 
> https://reviews.llvm.org/D65819
> 
> 
>