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

[Jonas Hahnfeld via Phabricator via cfe-commits]

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