From 5bf80ce2648e4c250d5c1030f407e2a5071cb5d9 Mon Sep 17 00:00:00 2001 From: Noisyfox Date: Sat, 14 Aug 2021 22:51:14 +1000 Subject: [PATCH 1/6] POC: Fix MachO AARCH64 relocation --- .../src/com/oracle/objectfile/ObjectFile.java | 6 ++ .../macho/MachORelocationElement.java | 62 ++++++++++++- .../macho/MachOUserDefinedSection.java | 88 ++++++++++++------- 3 files changed, 121 insertions(+), 35 deletions(-) diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/ObjectFile.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/ObjectFile.java index 3c879bec2cb9..8abf9a154967 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/ObjectFile.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/ObjectFile.java @@ -344,6 +344,12 @@ public static int getRelocationSize(RelocationKind kind) { case DIRECT_4: case PC_RELATIVE_4: case SECREL_4: + case AARCH64_R_AARCH64_ADR_PREL_PG_HI21: + case AARCH64_R_AARCH64_LDST64_ABS_LO12_NC: + case AARCH64_R_AARCH64_LDST32_ABS_LO12_NC: + case AARCH64_R_AARCH64_LDST16_ABS_LO12_NC: + case AARCH64_R_AARCH64_LDST8_ABS_LO12_NC: + case AARCH64_R_AARCH64_ADD_ABS_LO12_NC: return 4; case DIRECT_8: case PC_RELATIVE_8: diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java index 12328ce998c7..de5e50799420 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java @@ -54,7 +54,17 @@ private static int compareSectionThenOffset(RelocationInfo p, RelocationInfo q) if (!p.getRelocatedSection().equals(q.getRelocatedSection())) { return p.getRelocatedSection().hashCode() - q.getRelocatedSection().hashCode(); } - return Math.toIntExact(p.getOffset() - q.getOffset()); + if (p.getOffset() != q.getOffset()) { + return Math.toIntExact(p.getOffset() - q.getOffset()); + } + + if (Objects.equals(p.getAddend(), q.getAddend())) { + return 0; + } else if (p.getAddend() == null) { + return 1; + } else { + return -1; + } } private Map infos = new TreeMap<>(MachORelocationElement::compareSectionThenOffset); @@ -169,6 +179,40 @@ final class RelocationInfo implements RelocationRecord, RelocationMethod { private final MachOSection targetSection; private final byte log2length; + public Long getAddend() { + return addend; + } + + private final Long addend; + + public static RelocationInfo newAddend(MachORelocationElement containingElement, MachOSection relocatedSection, int offset, int requestedLength, long addend) { + return new RelocationInfo(containingElement, relocatedSection, offset, requestedLength, addend); + } + + private RelocationInfo(MachORelocationElement containingElement, MachOSection relocatedSection, int offset, int requestedLength, long addend) { + this.containingElement = containingElement; + this.relocatedSection = relocatedSection; + this.sectionOffset = offset; // gets turned into a vaddr on write-out + /* + * NOTE: the Mach-O spec claims that r_length == 3 means a 4-byte length and not an 8-byte + * length. But it doesn't say how to encode an 8-bytes-long relocation site. And the + * following link seems to suggest that in the case of x86-64, r_length==3 does encode the + * 8-byte case. + * http://www.opensource.apple.com/source/xnu/xnu-1456.1.26/EXTERNAL_HEADERS/mach + * -o/x86_64/reloc.h + * + * Experimenting.... + */ + if (requestedLength != 8 && requestedLength != 4 && requestedLength != 2 && requestedLength != 1) { + throw new IllegalArgumentException("Mach-O cannot represent relocation lengths other than {1,2,4,8} bytes"); + } + this.log2length = (byte) ((requestedLength == 8) ? 3 : (requestedLength == 4) ? 2 : (requestedLength == 2) ? 1 : 0); + this.kind = RelocationKind.UNKNOWN; + this.targetSection = null; + this.sym = null; + this.addend = addend; + } + /** * Construct a relocation record. * @@ -205,6 +249,7 @@ final class RelocationInfo implements RelocationRecord, RelocationMethod { // if the symbol is defined in the same file, i.e. locally, we have a target section assert !asLocalReloc || this.sym.isDefined(); this.targetSection = asLocalReloc ? (MachOSection) this.sym.getDefinedSection() : null; + this.addend = null; } public static int getEncodedSize() { @@ -216,8 +261,13 @@ public void write(OutputAssembler oa, @SuppressWarnings("unused") Map Date: Tue, 17 Aug 2021 20:38:19 +1000 Subject: [PATCH 2/6] Addend reloc should not be external --- .../objectfile/macho/MachORelocationElement.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java index de5e50799420..972a7bd93ed0 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java @@ -261,17 +261,18 @@ public void write(OutputAssembler oa, @SuppressWarnings("unused") Map= 4) { throw new IllegalArgumentException("length must be in {1,2,4,8} bytes, so log2length must be in [0,3]"); @@ -327,7 +328,7 @@ public MachOSection getRelocatedSection() { private boolean isExtern() { // we record localness by grabbing the target section (see constructor) - return targetSection == null; + return targetSection == null && addend == null; } private boolean isPCRelative() { From 1493aaac8eb995de243b01aa0f7bb8cdf7cf7f93 Mon Sep 17 00:00:00 2001 From: Noisyfox Date: Wed, 18 Aug 2021 20:16:43 +1000 Subject: [PATCH 3/6] Reformat code --- .../macho/MachORelocationElement.java | 59 ++++++++++--------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java index 972a7bd93ed0..b0dd55f7bade 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java @@ -178,21 +178,20 @@ final class RelocationInfo implements RelocationRecord, RelocationMethod { private final Symbol sym; private final MachOSection targetSection; private final byte log2length; + private final Long addend; // `null` for kinds other than ADDEND - public Long getAddend() { - return addend; - } - - private final Long addend; - + /** + * Construct a relocation record for ARM64_RELOC_ADDEND. + * + * @param containingElement the containing relocation element + * @param relocatedSection the section being relocated + * @param offset the offset, within the relocated section, of the relocation site + * @param addend the addend to be encoded + */ public static RelocationInfo newAddend(MachORelocationElement containingElement, MachOSection relocatedSection, int offset, int requestedLength, long addend) { return new RelocationInfo(containingElement, relocatedSection, offset, requestedLength, addend); } - - private RelocationInfo(MachORelocationElement containingElement, MachOSection relocatedSection, int offset, int requestedLength, long addend) { - this.containingElement = containingElement; - this.relocatedSection = relocatedSection; - this.sectionOffset = offset; // gets turned into a vaddr on write-out + private static byte encodeRequestedLength(int requestedLength) { /* * NOTE: the Mach-O spec claims that r_length == 3 means a 4-byte length and not an 8-byte * length. But it doesn't say how to encode an 8-bytes-long relocation site. And the @@ -206,8 +205,23 @@ private RelocationInfo(MachORelocationElement containingElement, MachOSection re if (requestedLength != 8 && requestedLength != 4 && requestedLength != 2 && requestedLength != 1) { throw new IllegalArgumentException("Mach-O cannot represent relocation lengths other than {1,2,4,8} bytes"); } - this.log2length = (byte) ((requestedLength == 8) ? 3 : (requestedLength == 4) ? 2 : (requestedLength == 2) ? 1 : 0); - this.kind = RelocationKind.UNKNOWN; + return (byte) ((requestedLength == 8) ? 3 : (requestedLength == 4) ? 2 : (requestedLength == 2) ? 1 : 0); + } + + /** + * Construct a relocation record for ARM64_RELOC_ADDEND. + * + * @param containingElement the containing relocation element + * @param relocatedSection the section being relocated + * @param offset the offset, within the relocated section, of the relocation site + * @param addend the addend to be encoded + */ + private RelocationInfo(MachORelocationElement containingElement, MachOSection relocatedSection, int offset, int requestedLength, long addend) { + this.containingElement = containingElement; + this.relocatedSection = relocatedSection; + this.sectionOffset = offset; // gets turned into a vaddr on write-out + this.log2length = encodeRequestedLength(requestedLength); + this.kind = RelocationKind.UNKNOWN; // Placeholder this.targetSection = null; this.sym = null; this.addend = addend; @@ -227,20 +241,7 @@ private RelocationInfo(MachORelocationElement containingElement, MachOSection re this.containingElement = containingElement; this.relocatedSection = relocatedSection; this.sectionOffset = offset; // gets turned into a vaddr on write-out - /* - * NOTE: the Mach-O spec claims that r_length == 3 means a 4-byte length and not an 8-byte - * length. But it doesn't say how to encode an 8-bytes-long relocation site. And the - * following link seems to suggest that in the case of x86-64, r_length==3 does encode the - * 8-byte case. - * http://www.opensource.apple.com/source/xnu/xnu-1456.1.26/EXTERNAL_HEADERS/mach - * -o/x86_64/reloc.h - * - * Experimenting.... - */ - if (requestedLength != 8 && requestedLength != 4 && requestedLength != 2 && requestedLength != 1) { - throw new IllegalArgumentException("Mach-O cannot represent relocation lengths other than {1,2,4,8} bytes"); - } - this.log2length = (byte) ((requestedLength == 8) ? 3 : (requestedLength == 4) ? 2 : (requestedLength == 2) ? 1 : 0); + this.log2length = encodeRequestedLength(requestedLength);; this.kind = kind; SymbolTable symtab = relocatedSection.getOwner().getSymbolTable(); // FIXME: also allow section numbers here, for non-extern symbols @@ -326,6 +327,10 @@ public MachOSection getRelocatedSection() { return relocatedSection; } + public Long getAddend() { + return addend; + } + private boolean isExtern() { // we record localness by grabbing the target section (see constructor) return targetSection == null && addend == null; From 5d773998672ae5c7641ef6ee03dbf39788bd0b36 Mon Sep 17 00:00:00 2001 From: Noisyfox Date: Wed, 18 Aug 2021 20:22:21 +1000 Subject: [PATCH 4/6] Add more comments --- .../oracle/objectfile/macho/MachOUserDefinedSection.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachOUserDefinedSection.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachOUserDefinedSection.java index 7b84b7fad3f6..173d0399b5c9 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachOUserDefinedSection.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachOUserDefinedSection.java @@ -217,7 +217,13 @@ public void markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, Stri case AARCH64_R_AARCH64_LDST8_ABS_LO12_NC: case AARCH64_R_AARCH64_ADD_ABS_LO12_NC: if (explicitAddend != 0) { - // Create ARM64_RELOC_ADDEND + /* + * These relocations should use an explicit addend reloc record instead of an embedded addend, + * according to the Mach-O ld code at + * https://opensource.apple.com/source/ld64/ld64-274.2/src/ld/parsers/macho_relocatable_file.cpp.auto.html + * + * > xxxx instruction at xxxx has embedded addend. ARM64_RELOC_ADDEND should be used instead + */ RelocationInfo addend = RelocationInfo.newAddend(el, this, offset, length, explicitAddend); el.add(addend); } From 027a51f318c5ee13af3b8851b38bebf55adc41ff Mon Sep 17 00:00:00 2001 From: Noisyfox Date: Wed, 18 Aug 2021 20:36:43 +1000 Subject: [PATCH 5/6] Remove unnecessary semicolon --- .../src/com/oracle/objectfile/macho/MachORelocationElement.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java index b0dd55f7bade..12dbe0cc67b2 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java @@ -241,7 +241,7 @@ private RelocationInfo(MachORelocationElement containingElement, MachOSection re this.containingElement = containingElement; this.relocatedSection = relocatedSection; this.sectionOffset = offset; // gets turned into a vaddr on write-out - this.log2length = encodeRequestedLength(requestedLength);; + this.log2length = encodeRequestedLength(requestedLength); this.kind = kind; SymbolTable symtab = relocatedSection.getOwner().getSymbolTable(); // FIXME: also allow section numbers here, for non-extern symbols From 4ace9361fb465b85317bb284bbd6bbacf81a783f Mon Sep 17 00:00:00 2001 From: Noisyfox Date: Wed, 18 Aug 2021 21:49:34 +1000 Subject: [PATCH 6/6] Reformat --- .../macho/MachORelocationElement.java | 1 + .../macho/MachOUserDefinedSection.java | 28 ++++++++++--------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java index 12dbe0cc67b2..c75e591083fc 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORelocationElement.java @@ -191,6 +191,7 @@ final class RelocationInfo implements RelocationRecord, RelocationMethod { public static RelocationInfo newAddend(MachORelocationElement containingElement, MachOSection relocatedSection, int offset, int requestedLength, long addend) { return new RelocationInfo(containingElement, relocatedSection, offset, requestedLength, addend); } + private static byte encodeRequestedLength(int requestedLength) { /* * NOTE: the Mach-O spec claims that r_length == 3 means a 4-byte length and not an 8-byte diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachOUserDefinedSection.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachOUserDefinedSection.java index 173d0399b5c9..4611d3635b8e 100644 --- a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachOUserDefinedSection.java +++ b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachOUserDefinedSection.java @@ -170,9 +170,9 @@ public void markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, Stri if (getOwner().cpuType == MachOCpuType.X86_64) { /* - * NOTE: X86_64 Mach-O does not support explicit addends, and inline addends are applied even - * during dynamic linking. So if the caller supplies an explicit addend, we turn it into an - * implicit one by updating our content. + * NOTE: X86_64 Mach-O does not support explicit addends, and inline addends are applied + * even during dynamic linking. So if the caller supplies an explicit addend, we turn it + * into an implicit one by updating our content. */ long currentInlineAddendValue = sbb.readTruncatedLong(length); long desiredInlineAddendValue; @@ -190,12 +190,12 @@ public void markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, Stri /* * One more complication: for PC-relative relocation, at least on x86-64, Mach-O linkers - * (both AOT ld and dyld) adjust the calculation to compensate for the fact that it's the - * *next* instruction that the PC-relative reference gets resolved against. Note that ELF - * doesn't do this compensation. Our interface duplicates the ELF behaviour, so we have to - * act against this Mach-O-specific fixup here, by *adding* a little to the addend. The - * amount we add is always the length in bytes of the relocation site (since on x86-64 the - * reference is always the last field in a PC-relative instruction). + * (both AOT ld and dyld) adjust the calculation to compensate for the fact that it's + * the *next* instruction that the PC-relative reference gets resolved against. Note + * that ELF doesn't do this compensation. Our interface duplicates the ELF behaviour, so + * we have to act against this Mach-O-specific fixup here, by *adding* a little to the + * addend. The amount we add is always the length in bytes of the relocation site (since + * on x86-64 the reference is always the last field in a PC-relative instruction). */ if (RelocationKind.isPCRelative(k)) { desiredInlineAddendValue += length; @@ -218,11 +218,13 @@ public void markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, Stri case AARCH64_R_AARCH64_ADD_ABS_LO12_NC: if (explicitAddend != 0) { /* - * These relocations should use an explicit addend reloc record instead of an embedded addend, - * according to the Mach-O ld code at - * https://opensource.apple.com/source/ld64/ld64-274.2/src/ld/parsers/macho_relocatable_file.cpp.auto.html + * These relocations should use an explicit addend reloc record instead of + * an embedded addend, according to the Mach-O ld code at + * https://opensource.apple.com/source/ld64/ld64-274.2/src/ld/parsers/ + * macho_relocatable_file.cpp.auto.html * - * > xxxx instruction at xxxx has embedded addend. ARM64_RELOC_ADDEND should be used instead + * > xxxx instruction at xxxx has embedded addend. ARM64_RELOC_ADDEND should + * be used instead */ RelocationInfo addend = RelocationInfo.newAddend(el, this, offset, length, explicitAddend); el.add(addend);