MCJIT handling of linkonce_odr

Hi,

I'm finally moving cling to MCJIT - and MCJIT is wonderful! So far I
only ran into this issue:

$ cat linkonceodr.cxx
extern "C" int printf(const char*,...);

template <class T> struct StaticStuff {
  static T s_data;
};
template <class T> T StaticStuff<T>::s_data = 42;

int compareAddr(int* mcjit);

#ifdef BUILD_SHARED
int compareAddr(int* mcjit) {
  if (mcjit != &StaticStuff<int>::s_data) {
    printf("Wrong address, %ld in shared lib, %ld in mcjit!\n",
           (long)&StaticStuff<int>::s_data, (long)mcjit);
    return 1;
  }
  return 0;
}
#else
int main(int, char**) {
  return compareAddr(&StaticStuff<int>::s_data);
}
#endif

$ clang++ -fPIC -shared -DBUILD_SHARED -o liblinkonceodr.so linkonceodr.cxx
$ clang++ -emit-llvm -c linkonceodr.cxx -o - |
LD_PRELOAD=./liblinkonceodr.so lli -
Wrong address, 140449908087496 in shared lib, 140449908076544 in mcjit!

I.e. while compareAddr is resolved from the dylib, this:

@_ZN11StaticStuffIiE6s_dataE = linkonce_odr global i32 42, align 4

is not: it is re-emitted by the MCJIT. When building a binary and
linking against that dylib, _ZN11StaticStuffIiE6s is correctly picked up
from the dylib. I would expect MCJIT to behave the same.

I'll try to fix that myself, but I'd appreciate a hint where to do that.
Should I collect linkonce_odr-s and "replace" their emitted code as part
of llvm::RuntimeDyldImpl::resolveRelocations()? Or is this just a
missing case somewhere?

That's e.g. with

$ lli --version
LLVM (http://llvm.org/):
  LLVM version 3.6.0svn
  Optimized build.
  Built Jan 12 2015 (10:52:59).
  Default target: x86_64-unknown-linux-gnu
  Host CPU: corei7-avx

Cheers, Axel.

Hi Axel,

the problem is that weak symbols are not handled in RuntimeDyld at all.
The proper solution is probably to add a separate case for weak symbols
(where we’re already taking care of common symbols) in RuntimeDyld::loadObjectImpl and then adjust the other methods to not
bail out two quickly when encountering a weak symbol.

Please let me know if you’re planning to work on a patch for this, otherwise I’ll have a go tomorrow.

Thanks,
Keno

Hi Keno,

The part that scares me a bit is

and then adjust the other methods to not
bail out two quickly when encountering a weak symbol.

I would very much appreciate if you could implement this; I don't have
enough knowledge of the MCJIT nor llvm CodeGen internals... I will
happily try it out and provide you with feedback, though! :slight_smile:

Thank you *so* much for your fast reaction!

Cheers, Axel

Hi Axel.

I put up a patch at http://reviews.llvm.org/D6950. Have a look and see if it works for you.

Thanks,
Keno

Hi Keno,

Works like a charm!

Except for one minor issue: we need to catch symbol resolution failures[*].

Before D6950, that was possible by reimplementing our own
RTDyldMemoryManager::getSymbolAddress() that the LinkingMemoryManager
would forward to. Now we don't know whether the symbol is really
unresolved or whether there is a weak one waiting to be generated right
after the call to getSymbolAddress(). Thus I'd like to suggest to
include something similar to the attached patch, maybe in a subsequent
round.

Thanks a lot for coming up with that implementation so quickly! And
apologies that it took me so long to validate it on our side...

Cheers, Axel

* For "known" symbols we dlopen the relevant shared library, or else we
provide a dummy symbol and make sure that the code never gets called -
interpreter hacks.

MCJIT-missing-symbol.diff (2.16 KB)