[JITLINK]Fix endiannes issue with MachO_ptrauth_noolloc_sections.yaml

[anoopkg6]

Fix endianness issue with MachO_ptrauth_noolloc_sections.yaml relocations. Semantics of the r_word1 field is different depending on the endianness.

[uweigand]

@lhames after enabling JITLink tests on SystemZ, one MachO test started failing. It looks like JITLink doesn't take relocation endianness into account like the main MachO code does - can you have a look at this PR?

[lhames]

@uweigand This is unexpected:

MachO::any_relocation_info ARI =
        getObject().getRelocation(RelItr->getRawDataRefImpl());
    MachO::relocation_info RI;

getRelocation() should be returning an any_relocation_info that has already been swapped into the native endianness (by the call to getStruct here:

llvm-project/llvm/lib/Object/MachOObjectFile.cpp

Line 4824 in 4fe79a7

return getStruct<MachO::any_relocation_info>(

, with getStruct performing the swap here:

llvm-project/llvm/lib/Object/MachOObjectFile.cpp

Line 78 in 4fe79a7

if (O.isLittleEndian() != sys::IsLittleEndianHost)

). If this endianness swap wasn't kicking in then I'd expect many more tests to fail.

Given that it is a yaml test that is failing I suspect that the endianness issue is in the MachO YAML reader / writer.

[uweigand]

@lhames , yes, a byte-swap is already happening. However, there's apparently also an endian-dependent bitfield difference that is not handled automatically. If you look at the full code in MachOObjectFile.cpp, you see e.g.

uint64_t MachOObjectFile::getRelocationType(DataRefImpl Rel) const {
  MachO::any_relocation_info RE = getRelocation(Rel);
  return getAnyRelocationType(RE);
}

This calls getRelocation, which performs the byte swap. But then

unsigned
MachOObjectFile::getAnyRelocationType(
                                   const MachO::any_relocation_info &RE) const {
  if (isRelocationScattered(RE))
    return getScatteredRelocationType(RE);
  return getPlainRelocationType(*this, RE);
} 

calls

static unsigned getPlainRelocationType(const MachOObjectFile &O,
                                       const MachO::any_relocation_info &RE) { 
  if (O.isLittleEndian())
    return RE.r_word1 >> 28;
  return RE.r_word1 & 0xf;
} 

which gets different bits depending on target endianness.

This latter part is not currently not by the JITLink code.

[lhames]

@lhames , yes, a byte-swap is already happening. However, there's apparently also an endian-dependent bitfield difference that is not handled automatically.

Oh, of course! Thanks for pointing that out.

I want to avoid any external calls and further dependence on MachOObjectFile here. Does #168304 solve the issue for you? If so we can go for that instead.

[lhames]

Scratch that -- I just got LLVM building on ppc64be to test this out and my "fix" breaks other test cases. I should have an update shortly.

[lhames]

Previous PR was a bum steer. I believe that the "bitfields" in this struct have the same layout in both endiannesses (see comment at https://github.com/apple-oss-distributions/cctools/blob/920a2b45080fb9badf31bf675f03b19973f0dd4f/include/mach-o/reloc.h#L140). I think the real fix for this is #168323

[lhames]

#168323 has been merged. If that fixes your issue then I think that we can close this.

[uweigand]

#168323 has been merged. If that fixes your issue then I think that we can close this.

Yes, this does indeed fix the issue for us. Thanks!