Improve null def access error message (#69226)

This change adds a checkNull method that will check any def reference when accessed for null. If the 
def reference is null it throws a more descriptive error message with the name of the method/field 
that is accessed.

For the megamorphic cache, we use the checkNull as the filter instead of just getClass when looking 
up if we need to compute a new MethodHandle for a changed type.

Fixes: #53129

modules/lang-painless/src/main/java/org/elasticsearch/painless/DefBootstrap.java

@@ -126,12 +126,23 @@ static final class PIC extends MutableCallSite {
             setTarget(fallback);
         }
 
+        /**
+         * guard method to give a more descriptive error message when a def receiver is null
+         */
+        static Class<?> checkNull(Object receiver, String name) {
+            if (receiver == null) {
+                throw new NullPointerException("cannot access method/field [" + name + "] from a null def reference");
+            }
+
+            return receiver.getClass();
+        }
+
         /**
          * guard method for inline caching: checks the receiver's class is the same
          * as the cached class
          */
         static boolean checkClass(Class<?> clazz, Object receiver) {
-            return receiver.getClass() == clazz;
+            return receiver != null && receiver.getClass() == clazz;
         }
 
         /**
@@ -177,8 +188,11 @@ protected MethodHandle computeValue(Class<?> receiverType) {
                     }
                 }
             };
-            return MethodHandles.foldArguments(MethodHandles.exactInvoker(type),
-                    MEGAMORPHIC_LOOKUP.bindTo(megamorphicCache));
+            MethodHandle lookup =
+                    MethodHandles.filterArguments(CLASSVALUE_GET.bindTo(megamorphicCache), 0,
+                            MethodHandles.insertArguments(CHECK_NULL, 1, name));
+            lookup = lookup.asType(lookup.type().changeReturnType(MethodHandle.class));
+            return MethodHandles.foldArguments(MethodHandles.exactInvoker(type), lookup);
         }
 
         /**
@@ -195,7 +209,7 @@ Object fallback(final Object[] callArgs) throws Throwable {
                 setTarget(target);
                 return target.invokeWithArguments(callArgs);
             } else {
-                final Class<?> receiver = callArgs[0].getClass();
+                final Class<?> receiver = checkNull(callArgs[0], name);
                 final MethodHandle target = lookup(flavor, name, receiver).asType(type());
 
                 MethodHandle test = CHECK_CLASS.bindTo(receiver);
@@ -208,22 +222,22 @@ Object fallback(final Object[] callArgs) throws Throwable {
             }
         }
 
+        private static final MethodHandle CHECK_NULL;
         private static final MethodHandle CHECK_CLASS;
         private static final MethodHandle FALLBACK;
-        private static final MethodHandle MEGAMORPHIC_LOOKUP;
+        private static final MethodHandle CLASSVALUE_GET;
         static {
             final MethodHandles.Lookup methodHandlesLookup = MethodHandles.lookup();
             final MethodHandles.Lookup publicMethodHandlesLookup = MethodHandles.publicLookup();
             try {
+                CHECK_NULL = methodHandlesLookup.findStatic(PIC.class, "checkNull",
+                        MethodType.methodType(Class.class, Object.class, String.class));
                 CHECK_CLASS = methodHandlesLookup.findStatic(methodHandlesLookup.lookupClass(), "checkClass",
                         MethodType.methodType(boolean.class, Class.class, Object.class));
                 FALLBACK = methodHandlesLookup.findVirtual(methodHandlesLookup.lookupClass(), "fallback",
                         MethodType.methodType(Object.class, Object[].class));
-                MethodHandle mh = publicMethodHandlesLookup.findVirtual(ClassValue.class, "get",
+                CLASSVALUE_GET = publicMethodHandlesLookup.findVirtual(ClassValue.class, "get",
                         MethodType.methodType(Object.class, Class.class));
-                mh = MethodHandles.filterArguments(mh, 1,
-                        publicMethodHandlesLookup.findVirtual(Object.class, "getClass", MethodType.methodType(Class.class)));
-                MEGAMORPHIC_LOOKUP = mh.asType(mh.type().changeReturnType(MethodHandle.class));
             } catch (ReflectiveOperationException e) {
                 throw new AssertionError(e);
             }

modules/lang-painless/src/test/java/org/elasticsearch/painless/WhenThingsGoWrongTests.java

@@ -30,6 +30,21 @@ public void testNullPointer() {
         });
     }
 
+    public void testDefNullPointer() {
+        NullPointerException npe = expectScriptThrows(NullPointerException.class, () -> {
+            exec("def x = null; x.intValue(); return null;");
+        });
+        assertEquals(npe.getMessage(), "cannot access method/field [intValue] from a null def reference");
+        npe = expectScriptThrows(NullPointerException.class, () -> {
+            exec("def x = [1, null]; for (y in x) y.intValue(); return null;");
+        });
+        assertEquals(npe.getMessage(), "cannot access method/field [intValue] from a null def reference");
+        npe = expectScriptThrows(NullPointerException.class, () -> {
+            exec("def x = [1, 2L, 3.0, 'test', (byte)1, (short)1, (char)1, null]; for (y in x) y.toString(); return null;");
+        });
+        assertEquals(npe.getMessage(), "cannot access method/field [toString] from a null def reference");
+    }
+
     /**
      * Test that the scriptStack looks good. By implication this tests that we build proper "line numbers" in stack trace. These line
      * numbers are really 1 based character numbers.