Skip to content

Commit

Permalink
8272347: ObjectMethods::bootstrap should specify NPE if any argument …
Browse files Browse the repository at this point in the history 
…except lookup is null

Reviewed-by: mchung, chegar
  • Loading branch information
Vicente Romero committed Aug 30, 2021
1 parent 7fc8540 commit 0609421
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 14 deletions.
19 changes: 15 additions & 4 deletions src/java.base/share/classes/java/lang/runtime/ObjectMethods.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
import java.util.List;
import java.util.Objects;

import static java.util.Objects.requireNonNull;

/**
* Bootstrap methods for state-driven implementations of core methods,
* including {@link Object#equals(Object)}, {@link Object#hashCode()}, and
Expand Down Expand Up @@ -317,22 +319,31 @@ private static MethodHandle makeToString(Class<?> receiverClass,
* @param recordClass the record class hosting the record components
* @param names the list of component names, joined into a string
* separated by ";", or the empty string if there are no
* components. Maybe be null, if the {@code methodName}
* is {@code "equals"} or {@code "hashCode"}.
* components. This parameter is ignored if the {@code methodName}
* parameter is {@code "equals"} or {@code "hashCode"}
* @param getters method handles for the accessor methods for the components
* @return a call site if invoked by indy, or a method handle
* if invoked by a condy
* @throws IllegalArgumentException if the bootstrap arguments are invalid
* or inconsistent
* @throws NullPointerException if any argument but {@code lookup} is {@code null},
* in the case of the {@code getters} argument, its
* contents cannot be {@code null} either
* @throws Throwable if any exception is thrown during call site construction
*/
public static Object bootstrap(MethodHandles.Lookup lookup, String methodName, TypeDescriptor type,
Class<?> recordClass,
String names,
MethodHandle... getters) throws Throwable {
requireNonNull(methodName);
requireNonNull(type);
requireNonNull(recordClass);
requireNonNull(names);
requireNonNull(getters);
Arrays.stream(getters).forEach(Objects::requireNonNull);
MethodType methodType;
if (type instanceof MethodType)
methodType = (MethodType) type;
if (type instanceof MethodType mt)
methodType = mt;
else {
methodType = null;
if (!MethodHandle.class.equals(type))
Expand Down
27 changes: 17 additions & 10 deletions test/jdk/java/lang/runtime/ObjectMethodsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
* @run testng/othervm/java.security.policy=empty.policy ObjectMethodsTest
*/

import java.util.List;
import java.lang.invoke.CallSite;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
Expand Down Expand Up @@ -103,7 +104,7 @@ public void testEqualsEmpty() throws Throwable {
}

public void testHashCodeC() throws Throwable {
CallSite cs = (CallSite)ObjectMethods.bootstrap(LOOKUP, "hashCode", C.HASHCODE_DESC, C.class, null, C.ACCESSORS);
CallSite cs = (CallSite)ObjectMethods.bootstrap(LOOKUP, "hashCode", C.HASHCODE_DESC, C.class, "x;y", C.ACCESSORS);
MethodHandle handle = cs.dynamicInvoker();
C c = new C(6, 7);
int hc = (int)handle.invokeExact(c);
Expand Down Expand Up @@ -151,15 +152,21 @@ public void exceptions() {
assertThrows(IAE, () -> ObjectMethods.bootstrap(LOOKUP, "hashCode", C.TO_STRING_DESC, C.class, "x;y", C.ACCESSORS));
assertThrows(IAE, () -> ObjectMethods.bootstrap(LOOKUP, "equals", C.HASHCODE_DESC, C.class, "x;y", C.ACCESSORS));

assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, "toString", C.TO_STRING_DESC, C.class, "x;y", null) );
assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, "toString", C.TO_STRING_DESC, C.class, null, C.ACCESSORS));
assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, "toString", C.TO_STRING_DESC, null, "x;y", C.ACCESSORS));
assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, "equals", C.EQUALS_DESC, null, "x;y", C.ACCESSORS));
assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, "hashCode", C.HASHCODE_DESC, null, "x;y", C.ACCESSORS));

assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, "toString", null, C.class, "x;y", C.ACCESSORS));
assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, null, C.TO_STRING_DESC, C.class, "x;y", C.ACCESSORS));
//assertThrows(NPE, () -> ObjectMethods.bootstrap(null, "toString", C.TO_STRING_DESC, C.class, "x;y", C.ACCESSORS));
record NamePlusType(String mn, MethodType mt) {}
List<NamePlusType> namePlusTypeList = List.of(
new NamePlusType("toString", C.TO_STRING_DESC),
new NamePlusType("equals", C.EQUALS_DESC),
new NamePlusType("hashCode", C.HASHCODE_DESC)
);

for (NamePlusType npt : namePlusTypeList) {
assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, npt.mn(), npt.mt(), C.class, "x;y", null));
assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, npt.mn(), npt.mt(), C.class, "x;y", new MethodHandle[]{null}));
assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, npt.mn(), npt.mt(), C.class, null, C.ACCESSORS));
assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, npt.mn(), npt.mt(), null, "x;y", C.ACCESSORS));
assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, npt.mn(), null, C.class, "x;y", C.ACCESSORS));
assertThrows(NPE, () -> ObjectMethods.bootstrap(LOOKUP, null, npt.mt(), C.class, "x;y", C.ACCESSORS));
}
}

// Based on the ObjectMethods internal implementation
Expand Down

1 comment on commit 0609421

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.