Refactor: don't return RelocationRecord from markRelocationSite

zakkak · stooke · zakkak · commit 0bfd72224dd3 · 2021-06-22T23:21:38.000+03:00
Removes the need of creating a dummy RelocationRecord to work around the NPE observed in #3419 (comment) Co-Authored-By: Simon Tooke <stooke@redhat.com>
diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/BasicProgbitsSectionImpl.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/BasicProgbitsSectionImpl.java
@@ -35,7 +35,6 @@
 import com.oracle.objectfile.ObjectFile.ProgbitsSectionImpl;
 import com.oracle.objectfile.ObjectFile.RelocatableSectionImpl;
 import com.oracle.objectfile.ObjectFile.RelocationKind;
-import com.oracle.objectfile.ObjectFile.RelocationRecord;
 import com.oracle.objectfile.ObjectFile.Section;
 
 /**
@@ -114,15 +113,15 @@ public List<Section> getElements() {
     }
 
     @Override
-    public RelocationRecord markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
-        return ((RelocatableSectionImpl) getElement()).markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend,
+    public void markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
+        ((RelocatableSectionImpl) getElement()).markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend,
                         explicitAddend);
     }
 
     @Override
-    public final RelocationRecord markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
+    public final void markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
         assert getContent() == null || bb.array() == getContent();
-        return ((RelocatableSectionImpl) getElement()).markRelocationSite(offset, bb, k, symbolName, useImplicitAddend, explicitAddend);
+        ((RelocatableSectionImpl) getElement()).markRelocationSite(offset, bb, k, symbolName, useImplicitAddend, explicitAddend);
     }
 
     @Override
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
@@ -408,9 +408,8 @@ public interface RelocatableSectionImpl extends ElementImpl {
          *            bytes
          * @param useImplicitAddend whether the current bytes are to be used as an addend
          * @param explicitAddend a full-width addend, or null if useImplicitAddend is true
-         * @return the relocation record created (or found, if it exists already)
          */
-        RelocationRecord markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend);
+        void markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend);
 
         /**
          * Force the creation of a relocation section/element for this section, and return it. This
@@ -441,7 +440,7 @@ public interface ProgbitsSectionImpl extends RelocatableSectionImpl {
          * passed a buffer. It uses the byte array accessed by {@link #getContent} and
          * {@link #setContent}.
          */
-        RelocationRecord markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend);
+        void markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend);
     }
 
     public interface NobitsSectionImpl extends ElementImpl {
diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/StringSectionImpl.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/StringSectionImpl.java
@@ -35,7 +35,6 @@
 import com.oracle.objectfile.ObjectFile.Element;
 import com.oracle.objectfile.ObjectFile.ProgbitsSectionImpl;
 import com.oracle.objectfile.ObjectFile.RelocationKind;
-import com.oracle.objectfile.ObjectFile.RelocationRecord;
 import com.oracle.objectfile.io.AssemblyBuffer;
 import com.oracle.objectfile.io.OutputAssembler;
 
@@ -125,12 +124,12 @@ public Element getOrCreateRelocationElement(boolean useImplicitAddend) {
     }
 
     @Override
-    public RelocationRecord markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
+    public void markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
         throw new UnsupportedOperationException("can't mark relocaction sites in string section");
     }
 
     @Override
-    public RelocationRecord markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
+    public void markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
         throw new UnsupportedOperationException("can't mark relocaction sites in string section");
     }
 
diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/ELFProgbitsSection.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/ELFProgbitsSection.java
@@ -69,7 +69,7 @@ public void setContent(byte[] c) {
     }
 
     @Override
-    public ObjectFile.RelocationRecord markRelocationSite(int offset, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
-        return markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend, explicitAddend);
+    public void markRelocationSite(int offset, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
+        markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend, explicitAddend);
     }
 }
diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/ELFUserDefinedSection.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/elf/ELFUserDefinedSection.java
@@ -34,7 +34,6 @@
 import com.oracle.objectfile.LayoutDecisionMap;
 import com.oracle.objectfile.ObjectFile;
 import com.oracle.objectfile.ObjectFile.Element;
-import com.oracle.objectfile.ObjectFile.RelocationRecord;
 import com.oracle.objectfile.elf.ELFObjectFile.ELFSection;
 import com.oracle.objectfile.elf.ELFObjectFile.ELFSectionFlag;
 import com.oracle.objectfile.elf.ELFObjectFile.SectionType;
@@ -148,7 +147,7 @@ public Element getOrCreateRelocationElement(boolean useImplicitAddend) {
     }
 
     @Override
-    public RelocationRecord markRelocationSite(int offset, ByteBuffer bb, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
+    public void markRelocationSite(int offset, ByteBuffer bb, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
         if (useImplicitAddend != (explicitAddend == null)) {
             throw new IllegalArgumentException("must have either an explicit or implicit addend");
         }
@@ -157,6 +156,6 @@ public RelocationRecord markRelocationSite(int offset, ByteBuffer bb, ObjectFile
         assert symbolName != null;
         ELFSymtab.Entry ent = syms.getSymbol(symbolName);
         assert ent != null;
-        return rs.addEntry(this, offset, ELFMachine.getRelocation(getOwner().getMachine(), k), ent, explicitAddend);
+        rs.addEntry(this, offset, ELFMachine.getRelocation(getOwner().getMachine(), k), ent, explicitAddend);
     }
 }
diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORegularSection.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/macho/MachORegularSection.java
@@ -30,7 +30,6 @@
 import com.oracle.objectfile.ObjectFile;
 import com.oracle.objectfile.ObjectFile.ProgbitsSectionImpl;
 import com.oracle.objectfile.ObjectFile.RelocationKind;
-import com.oracle.objectfile.ObjectFile.RelocationRecord;
 import com.oracle.objectfile.macho.MachOObjectFile.SectionFlag;
 import com.oracle.objectfile.macho.MachOObjectFile.Segment64Command;
 
@@ -51,8 +50,8 @@ public byte[] getContent() {
     }
 
     @Override
-    public RelocationRecord markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
-        return markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend, explicitAddend);
+    public void markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
+        markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend, explicitAddend);
     }
 
 }
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
@@ -36,7 +36,6 @@
 import com.oracle.objectfile.ObjectFile;
 import com.oracle.objectfile.ObjectFile.Element;
 import com.oracle.objectfile.ObjectFile.RelocationKind;
-import com.oracle.objectfile.ObjectFile.RelocationRecord;
 import com.oracle.objectfile.ObjectFile.Segment;
 import com.oracle.objectfile.ObjectFile.Symbol;
 import com.oracle.objectfile.io.AssemblyBuffer;
@@ -160,7 +159,7 @@ public MachORelocationElement getOrCreateRelocationElement(boolean useImplicitAd
     }
 
     @Override
-    public RelocationRecord markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
+    public void markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
         MachORelocationElement el = getOrCreateRelocationElement(useImplicitAddend);
         AssemblyBuffer sbb = new AssemblyBuffer(bb);
         sbb.setByteOrder(getOwner().getByteOrder());
@@ -215,7 +214,5 @@ public RelocationRecord markRelocationSite(int offset, ByteBuffer bb, Relocation
         sbb.pop();
         RelocationInfo rec = new RelocationInfo(el, this, offset, length, k, symbolName, createAsLocalReloc);
         el.add(rec);
-
-        return rec;
     }
 }
diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/pecoff/PECoffProgbitsSection.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/pecoff/PECoffProgbitsSection.java
@@ -69,7 +69,7 @@ public void setContent(byte[] c) {
     }
 
     @Override
-    public ObjectFile.RelocationRecord markRelocationSite(int offset, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
-        return markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend, explicitAddend);
+    public void markRelocationSite(int offset, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
+        markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend, explicitAddend);
     }
 }
diff --git a/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/pecoff/PECoffUserDefinedSection.java b/substratevm/src/com.oracle.objectfile/src/com/oracle/objectfile/pecoff/PECoffUserDefinedSection.java
@@ -34,7 +34,6 @@
 import com.oracle.objectfile.LayoutDecisionMap;
 import com.oracle.objectfile.ObjectFile;
 import com.oracle.objectfile.ObjectFile.Element;
-import com.oracle.objectfile.ObjectFile.RelocationRecord;
 import com.oracle.objectfile.io.AssemblyBuffer;
 import com.oracle.objectfile.pecoff.PECoffObjectFile.PECoffSection;
 import com.oracle.objectfile.pecoff.PECoffObjectFile.PECoffSectionFlag;
@@ -141,7 +140,7 @@ public Element getOrCreateRelocationElement(boolean useImplicitAddend) {
     }
 
     @Override
-    public RelocationRecord markRelocationSite(int offset, ByteBuffer bb, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
+    public void markRelocationSite(int offset, ByteBuffer bb, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {
         if (useImplicitAddend != (explicitAddend == null)) {
             throw new IllegalArgumentException("must have either an explicit or implicit addend");
         }
@@ -151,8 +150,7 @@ public RelocationRecord markRelocationSite(int offset, ByteBuffer bb, ObjectFile
         PECoffSymtab.Entry ent = syms.getSymbol(symbolName);
         if (ent == null) {
             warn("attempting to mark relocation site for non-existent symbol " + symbolName);
-            /* Return (but do not add to entry list) dummy relocation entry. It is never used (in GraalVM CE) */
-            return new PECoffRelocationTable.Entry(this, offset, PECoffMachine.getRelocation(getOwner().getMachine(), k), null, 0L);
+            return;
         }
 
         AssemblyBuffer sbb = new AssemblyBuffer(bb);
@@ -198,7 +196,7 @@ public RelocationRecord markRelocationSite(int offset, ByteBuffer bb, ObjectFile
         // return ByteBuffer cursor to where it was
         sbb.pop();
 
-        return rs.addEntry(this, offset, PECoffMachine.getRelocation(getOwner().getMachine(), k), ent, explicitAddend);
+        rs.addEntry(this, offset, PECoffMachine.getRelocation(getOwner().getMachine(), k), ent, explicitAddend);
     }
 
     /**

Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,6 @@`
`35`	`35`	`import com.oracle.objectfile.ObjectFile.Element;`
`36`	`36`	`import com.oracle.objectfile.ObjectFile.ProgbitsSectionImpl;`
`37`	`37`	`import com.oracle.objectfile.ObjectFile.RelocationKind;`
`38`		`-import com.oracle.objectfile.ObjectFile.RelocationRecord;`
`39`	`38`	`import com.oracle.objectfile.io.AssemblyBuffer;`
`40`	`39`	`import com.oracle.objectfile.io.OutputAssembler;`
`41`	`40`
`@@ -125,12 +124,12 @@ public Element getOrCreateRelocationElement(boolean useImplicitAddend) {`
`125`	`124`	`}`
`126`	`125`
`127`	`126`	`@Override`
`128`		`- public RelocationRecord markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {`
	`127`	`+ public void markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {`
`129`	`128`	`throw new UnsupportedOperationException("can't mark relocaction sites in string section");`
`130`	`129`	`}`
`131`	`130`
`132`	`131`	`@Override`
`133`		`- public RelocationRecord markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {`
	`132`	`+ public void markRelocationSite(int offset, ByteBuffer bb, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {`
`134`	`133`	`throw new UnsupportedOperationException("can't mark relocaction sites in string section");`
`135`	`134`	`}`
`136`	`135`
Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,7 @@ public void setContent(byte[] c) {`
`69`	`69`	`}`
`70`	`70`
`71`	`71`	`@Override`
`72`		`- public ObjectFile.RelocationRecord markRelocationSite(int offset, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {`
`73`		`- return markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend, explicitAddend);`
	`72`	`+ public void markRelocationSite(int offset, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {`
	`73`	`+ markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend, explicitAddend);`
`74`	`74`	`}`
`75`	`75`	`}`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,6 @@`
`34`	`34`	`import com.oracle.objectfile.LayoutDecisionMap;`
`35`	`35`	`import com.oracle.objectfile.ObjectFile;`
`36`	`36`	`import com.oracle.objectfile.ObjectFile.Element;`
`37`		`-import com.oracle.objectfile.ObjectFile.RelocationRecord;`
`38`	`37`	`import com.oracle.objectfile.elf.ELFObjectFile.ELFSection;`
`39`	`38`	`import com.oracle.objectfile.elf.ELFObjectFile.ELFSectionFlag;`
`40`	`39`	`import com.oracle.objectfile.elf.ELFObjectFile.SectionType;`
`@@ -148,7 +147,7 @@ public Element getOrCreateRelocationElement(boolean useImplicitAddend) {`
`148`	`147`	`}`
`149`	`148`
`150`	`149`	`@Override`
`151`		`- public RelocationRecord markRelocationSite(int offset, ByteBuffer bb, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {`
	`150`	`+ public void markRelocationSite(int offset, ByteBuffer bb, ObjectFile.RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {`
`152`	`151`	`if (useImplicitAddend != (explicitAddend == null)) {`
`153`	`152`	`throw new IllegalArgumentException("must have either an explicit or implicit addend");`
`154`	`153`	`}`
`@@ -157,6 +156,6 @@ public RelocationRecord markRelocationSite(int offset, ByteBuffer bb, ObjectFile`
`157`	`156`	`assert symbolName != null;`
`158`	`157`	`ELFSymtab.Entry ent = syms.getSymbol(symbolName);`
`159`	`158`	`assert ent != null;`
`160`		`- return rs.addEntry(this, offset, ELFMachine.getRelocation(getOwner().getMachine(), k), ent, explicitAddend);`
	`159`	`+ rs.addEntry(this, offset, ELFMachine.getRelocation(getOwner().getMachine(), k), ent, explicitAddend);`
`161`	`160`	`}`
`162`	`161`	`}`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,6 @@`
`30`	`30`	`import com.oracle.objectfile.ObjectFile;`
`31`	`31`	`import com.oracle.objectfile.ObjectFile.ProgbitsSectionImpl;`
`32`	`32`	`import com.oracle.objectfile.ObjectFile.RelocationKind;`
`33`		`-import com.oracle.objectfile.ObjectFile.RelocationRecord;`
`34`	`33`	`import com.oracle.objectfile.macho.MachOObjectFile.SectionFlag;`
`35`	`34`	`import com.oracle.objectfile.macho.MachOObjectFile.Segment64Command;`
`36`	`35`
`@@ -51,8 +50,8 @@ public byte[] getContent() {`
`51`	`50`	`}`
`52`	`51`
`53`	`52`	`@Override`
`54`		`- public RelocationRecord markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {`
`55`		`- return markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend, explicitAddend);`
	`53`	`+ public void markRelocationSite(int offset, RelocationKind k, String symbolName, boolean useImplicitAddend, Long explicitAddend) {`
	`54`	`+ markRelocationSite(offset, ByteBuffer.wrap(getContent()).order(getOwner().getByteOrder()), k, symbolName, useImplicitAddend, explicitAddend);`
`56`	`55`	`}`
`57`	`56`
`58`	`57`	`}`