Fix potential bug in jaotc

[Ludovic Henry]

Hello,

As I was learning and reading over the codebase of src/jdk.aot and src/hotspot/share/aot, I discovered a discrepancy which might lead to future bugs. The issue is around how some of the metadata is generated and loaded between jaotc and the runtime.

In the runtime, `aot_metadata` (src/hotspot/share/aot/aotCompiledMethod.hpp) contains metadata about each method that is compiled in the AOT image - for example, information like the implicit and explicit exception table, or the oop map. Because the size of this data is specific to each method, `aot_metadata` contains fields that store offsets to the `this` pointer, and the data is stored right after the `aot_metadata` structure in the AOT image.

The structure of `aot_metadata` is the following:

class aot_metadata {
  int _size;
  int _code_size;
  int _entry;
  int _verified_entry;
  int _exception_handler_offset;
  int _deopt_handler_offset;
  int _stubs_offset;
  int _frame_size;
  int _orig_pc_offset;
  int _unsafe_access;

  // contain the offset to the `this` pointer
  int _pc_desc_begin;
  int _scopes_begin;
  int _reloc_begin;
  int _exception_table_begin;
  int _nul_chk_table_begin;
  int _oopmap_begin;
}

When creating the AOT image and generating the information in `jdk.tools.jaotc.MetadataBuilder` (src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/MetadataBuilder.java), the `aot_metadata._nul_chk_table_begin` field is conditionally generated. In the case where there is no implicit exceptions in the method, no data/field is generated in the AOT image. However, there is no check for the existence of this field when loading the `aot_metadata` from the AOT image.

This issue is mitigated by the fact that the `HotSpotMetaData.implicitExceptionBytes` is always set, even if only with an empty `byte[]`. Nonetheless, this field is populated in the runtime in `getMetadata` (in src/hotspot/share/jvmci/jvmciCompilerToVM.cpp), making the initialization and usage of this field far apart, complexifying the overall solution.

The following patch proposes to always generate the `aot_metadata._nul_chk_table_begin` field. It also has the advantage to unify how this field and the others are generated.

diff --git a/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/MetadataBuilder.java b/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/MetadataBuilder.java
index 34ce4d8c8b..fa89d34796 100644
--- a/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/MetadataBuilder.java
+++ b/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/MetadataBuilder.java
@@ -98,7 +98,8 @@ final class MetadataBuilder {
             byte[] scopeDesc = metaData.scopesDescBytes();
             byte[] relocationInfo = metaData.relocBytes();
             byte[] oopMapInfo = metaData.oopMaps();
-            byte[] implicitExceptionBytes = HotSpotGraalServices.getImplicitExceptionBytes(metaData);
+            byte[] exceptionBytes = metaData.exceptionBytes();
+            byte[] implicitExceptionBytes = metaData.implicitExceptionBytes();

             // create a global symbol at this position for this method
             NativeOrderOutputStream metadataStream = new NativeOrderOutputStream();
@@ -143,10 +144,7 @@ final class MetadataBuilder {
                 NativeOrderOutputStream.PatchableInt scopeOffset = metadataStream.patchableInt();
                 NativeOrderOutputStream.PatchableInt relocationOffset = metadataStream.patchableInt();
                 NativeOrderOutputStream.PatchableInt exceptionOffset = metadataStream.patchableInt();
-                NativeOrderOutputStream.PatchableInt implictTableOffset = null;
-                if (implicitExceptionBytes != null) {
-                    implictTableOffset = metadataStream.patchableInt();
-                }
+                NativeOrderOutputStream.PatchableInt implictTableOffset = metadataStream.patchableInt();
                 NativeOrderOutputStream.PatchableInt oopMapOffset = metadataStream.patchableInt();
                 metadataStream.align(8);

@@ -160,12 +158,10 @@ final class MetadataBuilder {
                 metadataStream.put(relocationInfo).align(8);

                 exceptionOffset.set(metadataStream.position());
-                metadataStream.put(metaData.exceptionBytes()).align(8);
+                metadataStream.put(exceptionBytes).align(8);

-                if (implicitExceptionBytes != null) {
-                    implictTableOffset.set(metadataStream.position());
-                    metadataStream.put(implicitExceptionBytes).align(8);
-                }
+                implictTableOffset.set(metadataStream.position());
+                metadataStream.put(implicitExceptionBytes).align(8);

                 // oopmaps should be last
                 oopMapOffset.set(metadataStream.position());
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotMetaData.java b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotMetaData.java
index fc54c41312..3acb6191b9 100644
--- a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotMetaData.java
+++ b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotMetaData.java
@@ -41,6 +41,7 @@ public class HotSpotMetaData {
         scopesDescBytes = new byte[0];
         relocBytes = new byte[0];
         exceptionBytes = new byte[0];
+        implicitExceptionBytes = new byte[0];
         oopMaps = new byte[0];
         metadata = new String[0];
         // ...some of them will be overwritten by the VM:

Thank you,

--
Ludovic