[PATCH] D30709: Handle IMAGE_REL_AMD64_ADDR32NB in RuntimeDyldCOFF

[Frederich Munch via Phabricator via llvm-commits]

marsupial added inline comments.


================
Comment at: lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFX86_64.h:36-43
+  uint64_t getImageBase() {
+    if (!ImageBase) {
+      ImageBase = std::numeric_limits<uint64_t>::max();
+      for (const SectionEntry &Section : Sections)
+        ImageBase = std::min(ImageBase, Section.getLoadAddress());
+    }
+    return ImageBase;
----------------
compnerd wrote:
> marsupial wrote:
> > compnerd wrote:
> > > This seems wrong.  We must have an `__ImageBase__` in COFF land.  This would attempt to emulate that, resulting in the `IMAGE_REL_AMD64_ADDR32NB` becoming a `IMAGE_REL_AMD64_SECREL`.
> > > 
> > > Note that you want `Sections[0].getLoadAddress()` as an approximation of `__ImageBase__`.
> > While it may happen to work out more than not that Sections[0].getLoadAddress() is the lowest address, it is dependent on the SectionAllocator used and is possible that Sections[//N//].getLoadAddress() > Sections[0].getLoadAddress()
> > 
> > 
> > 
> Do we have any guarantees that the sections are mapped sequentially?  If that is the case, then this is fine.
That guarantee has to be provided by the section allocator itself, and https://reviews.llvm.org/D35103 is a test/example of doing that.

That said, this patch was done before some error handling code was put in for the JIT, so I'll see if the assert can be promoted to returning an error.


================
Comment at: lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldCOFFX86_64.h:167
+    // writeDeltaReloc(Section.getAddressWithOffset(Offset), 0, StubOffset);
+    // .xdata exception handler's aren't having this though.
+
----------------
compnerd wrote:
> Hmm, any chance of having a XFAIL test case for this?
As noted before, https://reviews.llvm.org/D35103 is dependent on this and a use case for the changes.
Any hints on how to generate/test the instruction without the clang codebase would be helpful.


https://reviews.llvm.org/D30709