8294931: JFR: Simplify SettingInfo

Reviewed-by: mgronlun
This commit is contained in:
Erik Gahlin 2022-10-10 17:56:34 +00:00
parent eb90c4fc04
commit 1bfcc2790a
5 changed files with 34 additions and 48 deletions

View File

@ -44,7 +44,6 @@ import jdk.jfr.SettingDefinition;
import jdk.jfr.StackTrace; import jdk.jfr.StackTrace;
import jdk.jfr.Threshold; import jdk.jfr.Threshold;
import jdk.jfr.events.ActiveSettingEvent; import jdk.jfr.events.ActiveSettingEvent;
import jdk.jfr.internal.EventInstrumentation.SettingInfo;
import jdk.jfr.internal.settings.CutoffSetting; import jdk.jfr.internal.settings.CutoffSetting;
import jdk.jfr.internal.settings.EnabledSetting; import jdk.jfr.internal.settings.EnabledSetting;
import jdk.jfr.internal.settings.PeriodSetting; import jdk.jfr.internal.settings.PeriodSetting;
@ -66,7 +65,7 @@ public final class EventControl {
private static final Type TYPE_CUTOFF = TypeLibrary.createType(CutoffSetting.class); private static final Type TYPE_CUTOFF = TypeLibrary.createType(CutoffSetting.class);
private static final Type TYPE_THROTTLE = TypeLibrary.createType(ThrottleSetting.class); private static final Type TYPE_THROTTLE = TypeLibrary.createType(ThrottleSetting.class);
private final ArrayList<SettingInfo> settingInfos = new ArrayList<>(); private final ArrayList<SettingControl> settingControls = new ArrayList<>();
private final ArrayList<NamedControl> namedControls = new ArrayList<>(5); private final ArrayList<NamedControl> namedControls = new ArrayList<>(5);
private final PlatformEventType type; private final PlatformEventType type;
private final String idName; private final String idName;
@ -163,7 +162,6 @@ public final class EventControl {
try { try {
Module settingModule = settingsClass.getModule(); Module settingModule = settingsClass.getModule();
Modules.addReads(settingModule, EventControl.class.getModule()); Modules.addReads(settingModule, EventControl.class.getModule());
int index = settingInfos.size();
SettingControl settingControl = instantiateSettingControl(settingsClass); SettingControl settingControl = instantiateSettingControl(settingsClass);
Control c = new Control(settingControl, null); Control c = new Control(settingControl, null);
c.setDefault(); c.setDefault();
@ -180,7 +178,7 @@ public final class EventControl {
aes.trimToSize(); aes.trimToSize();
addControl(settingName, c); addControl(settingName, c);
eventType.add(PrivateAccess.getInstance().newSettingDescriptor(settingType, settingName, defaultValue, aes)); eventType.add(PrivateAccess.getInstance().newSettingDescriptor(settingType, settingName, defaultValue, aes));
settingInfos.add(new SettingInfo(FIELD_SETTING_PREFIX + index, index, null, null, settingControl)); settingControls.add(settingControl);
} }
} catch (InstantiationException e) { } catch (InstantiationException e) {
// Programming error by user, fail fast // Programming error by user, fail fast
@ -308,7 +306,14 @@ public final class EventControl {
return idName; return idName;
} }
public List<SettingInfo> getSettingInfos() { /**
return settingInfos; * A malicious user must never be able to run a callback in the wrong
* context. Methods on SettingControl must therefore never be invoked directly
* by JFR, instead use jdk.jfr.internal.Control.
*
* The returned list is only to be used inside EventConfiguration
*/
public List<SettingControl> getSettingControls() {
return settingControls;
} }
} }

View File

@ -60,18 +60,10 @@ import jdk.jfr.internal.event.EventWriter;
*/ */
public final class EventInstrumentation { public final class EventInstrumentation {
record SettingInfo(String fieldName, int index, Type paramType, String methodName, SettingControl settingControl) { record SettingInfo(Type paramType, String methodName) {
/**
* A malicious user must never be able to run a callback in the wrong
* context. Methods on SettingControl must therefore never be invoked directly
* by JFR, instead use jdk.jfr.internal.Control.
*/
public SettingControl settingControl() {
return this.settingControl;
}
} }
record FieldInfo(String fieldName, String fieldDescriptor, String internalClassName) { record FieldInfo(String name, String descriptor) {
} }
public static final String FIELD_EVENT_THREAD = "eventThread"; public static final String FIELD_EVENT_THREAD = "eventThread";
@ -136,8 +128,8 @@ public final class EventInstrumentation {
public static Method findStaticCommitMethod(ClassNode classNode, List<FieldInfo> fields) { public static Method findStaticCommitMethod(ClassNode classNode, List<FieldInfo> fields) {
StringBuilder sb = new StringBuilder(); StringBuilder sb = new StringBuilder();
sb.append("("); sb.append("(");
for (FieldInfo v : fields) { for (FieldInfo field : fields) {
sb.append(v.fieldDescriptor); sb.append(field.descriptor);
} }
sb.append(")V"); sb.append(")V");
Method m = new Method("commit", sb.toString()); Method m = new Method("commit", sb.toString());
@ -244,10 +236,8 @@ public final class EventInstrumentation {
Type[] args = Type.getArgumentTypes(m.desc); Type[] args = Type.getArgumentTypes(m.desc);
if (args.length == 1) { if (args.length == 1) {
Type paramType = args[0]; Type paramType = args[0];
String fieldName = EventControl.FIELD_SETTING_PREFIX + settingInfos.size();
int index = settingInfos.size();
methodSet.add(m.name); methodSet.add(m.name);
settingInfos.add(new SettingInfo(fieldName, index, paramType, m.name, null)); settingInfos.add(new SettingInfo(paramType, m.name));
} }
} }
} }
@ -263,10 +253,8 @@ public final class EventInstrumentation {
if (method.getParameterCount() == 1) { if (method.getParameterCount() == 1) {
Parameter param = method.getParameters()[0]; Parameter param = method.getParameters()[0];
Type paramType = Type.getType(param.getType()); Type paramType = Type.getType(param.getType());
String fieldName = EventControl.FIELD_SETTING_PREFIX + settingInfos.size();
int index = settingInfos.size();
methodSet.add(method.getName()); methodSet.add(method.getName());
settingInfos.add(new SettingInfo(fieldName, index, paramType, method.getName(), null)); settingInfos.add(new SettingInfo(paramType, method.getName()));
} }
} }
} }
@ -285,11 +273,11 @@ public final class EventInstrumentation {
// control in which order they occur and we can add @Name, @Description // control in which order they occur and we can add @Name, @Description
// in Java, instead of in native. It also means code for adding implicit // in Java, instead of in native. It also means code for adding implicit
// fields for native can be reused by Java. // fields for native can be reused by Java.
fieldInfos.add(new FieldInfo("startTime", Type.LONG_TYPE.getDescriptor(), classNode.name)); fieldInfos.add(new FieldInfo("startTime", Type.LONG_TYPE.getDescriptor()));
fieldInfos.add(new FieldInfo("duration", Type.LONG_TYPE.getDescriptor(), classNode.name)); fieldInfos.add(new FieldInfo("duration", Type.LONG_TYPE.getDescriptor()));
for (FieldNode field : classNode.fields) { for (FieldNode field : classNode.fields) {
if (!fieldSet.contains(field.name) && isValidField(field.access, Type.getType(field.desc).getClassName())) { if (!fieldSet.contains(field.name) && isValidField(field.access, Type.getType(field.desc).getClassName())) {
FieldInfo fi = new FieldInfo(field.name, field.desc, classNode.name); FieldInfo fi = new FieldInfo(field.name, field.desc);
fieldInfos.add(fi); fieldInfos.add(fi);
fieldSet.add(field.name); fieldSet.add(field.name);
} }
@ -303,7 +291,7 @@ public final class EventInstrumentation {
if (!fieldSet.contains(fieldName)) { if (!fieldSet.contains(fieldName)) {
Type fieldType = Type.getType(field.getType()); Type fieldType = Type.getType(field.getType());
String internalClassName = ASMToolkit.getInternalName(c.getName()); String internalClassName = ASMToolkit.getInternalName(c.getName());
fieldInfos.add(new FieldInfo(fieldName, fieldType.getDescriptor(), internalClassName)); fieldInfos.add(new FieldInfo(fieldName, fieldType.getDescriptor()));
fieldSet.add(fieldName); fieldSet.add(fieldName);
} }
} }
@ -574,7 +562,7 @@ public final class EventInstrumentation {
// stack: [EW] [EW] // stack: [EW] [EW]
methodVisitor.visitVarInsn(Opcodes.ALOAD, 0); methodVisitor.visitVarInsn(Opcodes.ALOAD, 0);
// stack: [EW] [EW] [this] // stack: [EW] [EW] [this]
methodVisitor.visitFieldInsn(Opcodes.GETFIELD, getInternalClassName(), field.fieldName, field.fieldDescriptor); methodVisitor.visitFieldInsn(Opcodes.GETFIELD, getInternalClassName(), field.name, field.descriptor);
// stack: [EW] [EW] <T> // stack: [EW] [EW] <T>
EventWriterMethod eventMethod = EventWriterMethod.lookupMethod(field); EventWriterMethod eventMethod = EventWriterMethod.lookupMethod(field);
invokeVirtual(methodVisitor, TYPE_EVENT_WRITER, eventMethod.asmMethod); invokeVirtual(methodVisitor, TYPE_EVENT_WRITER, eventMethod.asmMethod);
@ -633,8 +621,8 @@ public final class EventInstrumentation {
methodVisitor.visitFieldInsn(Opcodes.GETFIELD, getInternalClassName(), FIELD_DURATION, "J"); methodVisitor.visitFieldInsn(Opcodes.GETFIELD, getInternalClassName(), FIELD_DURATION, "J");
invokeVirtual(methodVisitor, TYPE_EVENT_CONFIGURATION, METHOD_EVENT_CONFIGURATION_SHOULD_COMMIT); invokeVirtual(methodVisitor, TYPE_EVENT_CONFIGURATION, METHOD_EVENT_CONFIGURATION_SHOULD_COMMIT);
methodVisitor.visitJumpInsn(Opcodes.IFEQ, fail); methodVisitor.visitJumpInsn(Opcodes.IFEQ, fail);
int index = 0; for (int index = 0; index < settingInfos.size(); index++) {
for (SettingInfo si : settingInfos) { SettingInfo si = settingInfos.get(index);
// if (!settingsMethod(eventConfiguration.settingX)) goto fail; // if (!settingsMethod(eventConfiguration.settingX)) goto fail;
methodVisitor.visitIntInsn(Opcodes.ALOAD, 0); methodVisitor.visitIntInsn(Opcodes.ALOAD, 0);
if (untypedEventConfiguration) { if (untypedEventConfiguration) {
@ -648,7 +636,6 @@ public final class EventInstrumentation {
methodVisitor.visitTypeInsn(Opcodes.CHECKCAST, si.paramType().getInternalName()); methodVisitor.visitTypeInsn(Opcodes.CHECKCAST, si.paramType().getInternalName());
methodVisitor.visitMethodInsn(Opcodes.INVOKEVIRTUAL, getInternalClassName(), si.methodName, "(" + si.paramType().getDescriptor() + ")Z", false); methodVisitor.visitMethodInsn(Opcodes.INVOKEVIRTUAL, getInternalClassName(), si.methodName, "(" + si.paramType().getDescriptor() + ")Z", false);
methodVisitor.visitJumpInsn(Opcodes.IFEQ, fail); methodVisitor.visitJumpInsn(Opcodes.IFEQ, fail);
index++;
} }
// return true // return true
methodVisitor.visitInsn(Opcodes.ICONST_1); methodVisitor.visitInsn(Opcodes.ICONST_1);

View File

@ -67,16 +67,16 @@ public enum EventWriterMethod {
* *
* @return the method * @return the method
*/ */
public static EventWriterMethod lookupMethod(FieldInfo v) { public static EventWriterMethod lookupMethod(FieldInfo field) {
// event thread // event thread
if (v.fieldName().equals(EventInstrumentation.FIELD_EVENT_THREAD)) { if (field.name().equals(EventInstrumentation.FIELD_EVENT_THREAD)) {
return EventWriterMethod.PUT_EVENT_THREAD; return EventWriterMethod.PUT_EVENT_THREAD;
} }
for (EventWriterMethod m : EventWriterMethod.values()) { for (EventWriterMethod m : EventWriterMethod.values()) {
if (v.fieldDescriptor().equals(m.typeDescriptor)) { if (field.descriptor().equals(m.typeDescriptor)) {
return m; return m;
} }
} }
throw new Error("Unknown type " + v.fieldDescriptor()); throw new Error("Unknown type " + field.descriptor());
} }
} }

View File

@ -188,15 +188,15 @@ public final class MetadataRepository {
return Utils.getConfiguration(eventClass); return Utils.getConfiguration(eventClass);
} }
private EventConfiguration newEventConfiguration(EventType eventType, EventControl ec, SettingControl[] settings) { private EventConfiguration newEventConfiguration(EventType eventType, EventControl ec) {
try { try {
if (cachedEventConfigurationConstructor == null) { if (cachedEventConfigurationConstructor == null) {
var argClasses = new Class<?>[] { EventType.class, EventControl.class, SettingControl[].class }; var argClasses = new Class<?>[] { EventType.class, EventControl.class};
Constructor<EventConfiguration> c = EventConfiguration.class.getDeclaredConstructor(argClasses); Constructor<EventConfiguration> c = EventConfiguration.class.getDeclaredConstructor(argClasses);
SecuritySupport.setAccessible(c); SecuritySupport.setAccessible(c);
cachedEventConfigurationConstructor = c; cachedEventConfigurationConstructor = c;
} }
return cachedEventConfigurationConstructor.newInstance(eventType, ec, settings); return cachedEventConfigurationConstructor.newInstance(eventType, ec);
} catch (NoSuchMethodException | SecurityException | InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) { } catch (NoSuchMethodException | SecurityException | InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
throw new InternalError(e); throw new InternalError(e);
} }
@ -209,13 +209,7 @@ public final class MetadataRepository {
} }
EventType eventType = PrivateAccess.getInstance().newEventType(pEventType); EventType eventType = PrivateAccess.getInstance().newEventType(pEventType);
EventControl ec = new EventControl(pEventType, eventClass); EventControl ec = new EventControl(pEventType, eventClass);
List<SettingInfo> settingInfos = ec.getSettingInfos(); EventConfiguration configuration = newEventConfiguration(eventType, ec);
SettingControl[] settings = new SettingControl[settingInfos.size()];
int index = 0;
for (var settingInfo : settingInfos) {
settings[index++] = settingInfo.settingControl();
}
EventConfiguration configuration = newEventConfiguration(eventType, ec, settings);
PlatformEventType pe = configuration.getPlatformEventType(); PlatformEventType pe = configuration.getPlatformEventType();
pe.setRegistered(true); pe.setRegistered(true);
// If class is instrumented or should not be instrumented, mark as instrumented. // If class is instrumented or should not be instrumented, mark as instrumented.

View File

@ -40,11 +40,11 @@ public final class EventConfiguration {
private final SettingControl[] settings; private final SettingControl[] settings;
// Private constructor so user code can't instantiate // Private constructor so user code can't instantiate
private EventConfiguration(EventType eventType, EventControl eventControl, SettingControl[] settings) { private EventConfiguration(EventType eventType, EventControl eventControl) {
this.eventType = eventType; this.eventType = eventType;
this.platformEventType = PrivateAccess.getInstance().getPlatformEventType(eventType); this.platformEventType = PrivateAccess.getInstance().getPlatformEventType(eventType);
this.eventControl = eventControl; this.eventControl = eventControl;
this.settings = settings; this.settings = eventControl.getSettingControls().toArray(new SettingControl[0]);
} }
// Class jdk.jfr.internal.PlatformEventType is not // Class jdk.jfr.internal.PlatformEventType is not