heap checks

[jensdietrich]

instrument putfield, can duplicate object and value on stack with dup2, be careful with double and long , those will need special handling -- write tests for those

Basically instrumentation should consist of a sequence:

dup2
invokestatic JDala::check(..)

[qwsmit]

This has been implemented the bytecode instructions injected are

dup2
swap
getfield               // Field ---
invokestatic           // Method nz/ac/wgtn/ecs/jdala/JDala.validate:(Ljava/lang/Object;Ljava/lang/Object;)V

with --- being the name of the variable that is being validated

once it is injected it looks something like this

aload_0                // this
iload_3
dup2
swap
getfield               // Field ---
invokestatic           // Method nz/ac/wgtn/ecs/jdala/JDala.validate:(Ljava/lang/Object;Ljava/lang/Object;)V
outfield               // Field ---

Problem

I'm currently working on assuring that this isn't an uninitializedThis with the stack looking like stack: { uninitializedThis, integer, integer, uninitializedThis }

This shouldn't be a problem because before Java 23 the superclass must be initialized first and the Java compiler enforces this. (Looks like this will change soon but not currently relevant)

The JVM however doesn't follow this rule, and Java code that is compiled can have putfield before the invokespecial calling the super constructor.

e.g.

AppMainV2$1(String arg0, int var2, boolean var3) {
        super(arg0);
        this.val$portNumber = var2;
        this.val$helperLibLoaded = var3;
    }

can compile to

com.intellij.rt.execution.application.AppMainV2$1(java.lang.String, int, boolean);
    descriptor: (Ljava/lang/String;IZ)V
    flags: (0x0000)
    Code:
      stack=2, locals=4, args_size=4
         0: aload_0
         1: iload_2
         2: putfield      #1                  // Field val$portNumber:I
         5: aload_0
         6: iload_3
         7: putfield      #7                  // Field val$helperLibLoaded:Z
        10: aload_0
        11: aload_1
        12: invokespecial #11                 // Method java/lang/Thread."<init>":(Ljava/lang/String;)V
        15: return
      LineNumberTable:
        line 47: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0      16     0  this   Lcom/intellij/rt/execution/application/AppMainV2$1;
            0      16     1  arg0   Ljava/lang/String;

with two putfields before the super constructor call, these fields can't be accessed with getfield until after the super constructor is called.

@jensdietrich do you have any thoughts on how I could approach this?

[jensdietrich]

@qwsmit Why do we need the additional getfield instruction here ? I guess the swap is also redundant if we swap the arguments in JDala::check.

[qwsmit]

@jensdietrich Adding in getfield and swap allows me to view the actual field it is changing

With getfield

Box obj = new Box("foo");
JDala.validate("bar", obj.value);
obj.value = "bar";

vs
Without getfield

Box obj = new Box("foo");
JDala.validate("bar", obj);
obj.value = "bar";

But you are right in that I probably don't need this because I am just tracking the objects that are being edited not necessarily the fields of those objects

[qwsmit]

Even without this the problem still occurs due to the validate method checking if the object is a registered object or not. This means that it still try's to read from an uninitializedThis

[jensdietrich]

Did you mean getfield earlier ?

When we set a (non-static) field, we have this and the new field value (say: val) on the stack. We can access this without getfield by duplicating. Isnt this all we need ? We basically have to check whether we can add an edge this -> val to the heap.

[qwsmit]

Did you mean getfield earlier ?

Yes, sorry

When we set a (non-static) field, we have this and the new field value (say: val) on the stack. We can access this without getfield by duplicating. Isnt this all we need ? We basically have to check whether we can add an edge this -> val to the heap.

Correct, the problem comes with finding what capability an object has (unsafe, local, isolated, or immutable). This is currently done with calls like localThreadMap.get(objectref) or immutableObjectsList.contains(objectref) these functions break when performed on a uninitializedThis. This is the same problem as this. I don't know how to deal with uninitializedThiss

[jensdietrich]

What is the signal (exception or error) we get from immutableObjectsList.contains(objectref) when the object has not been initialised ?

[qwsmit]

One of the problems that happens before that is that uninitializedThis isn't an java/lang/Object

Exception in thread "main" java.lang.VerifyError: Bad type on operand stack
Exception Details:
  Location:
    org/junit/platform/launcher/core/LauncherConfigurationParameters$ParameterProvider$3.<init>(Ljava/util/Properties;)V @3: invokestatic
  Reason:
    Type uninitializedThis (current frame, stack[2]) is not assignable to 'java/lang/Object'
  Current Frame:
    bci: @3
    flags: { flagThisUninit }
    locals: { uninitializedThis, 'java/util/Properties' }
    stack: { uninitializedThis, 'java/util/Properties', uninitializedThis, 'java/util/Properties' }
  Bytecode:
    0000000: 2a2b 5cb8 0063 b500 012a b700 07b1     

	at org.junit.platform.launcher.core.LauncherConfigurationParameters$ParameterProvider.propertiesFile(LauncherConfigurationParameters.java:186)
	at org.junit.platform.launcher.core.LauncherConfigurationParameters$Builder.build(LauncherConfigurationParameters.java:129)
	at org.junit.platform.launcher.core.LauncherFactory.loadAndFilterTestExecutionListeners(LauncherFactory.java:186)
	at org.junit.platform.launcher.core.LauncherFactory.registerTestExecutionListeners(LauncherFactory.java:179)
	at org.junit.platform.launcher.core.LauncherFactory.createDefaultLauncher(LauncherFactory.java:137)
	at org.junit.platform.launcher.core.LauncherFactory.create(LauncherFactory.java:125)
	at org.junit.platform.launcher.core.LauncherFactory.create(LauncherFactory.java:109)
	at com.intellij.junit5.JUnit5IdeaTestRunner.createListeners(JUnit5IdeaTestRunner.java:34)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:232)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:55)

[jensdietrich]

I wonder whether ASM has an API to get this type.

We could also try to track the invokespecial invocations to the super constructor.

There is also a discussion here that might be helpful: https://stackoverflow.com/questions/69549215/type-is-not-assignable-to-uninitializedthis-when-inserting-stackmap-frame-usin

[qwsmit]

I wonder whether ASM has an API to get this type.

Since it isn't an object yet I can inject in something like

INSTANCEOF          // "java/lang/Object"

And do something if this is the case, but not quite sure what to do after this, I can just skip them for now but they should be validated.

We could also try to track the invokespecial invocations to the super constructor.

Yeah, that was what I was also thinking, this might be the way to do it, because I already have a first visitor for the annotations I could even store the location of the invocations to the super constructor. I would just need to find a way to store the values from the stack till the end

[jensdietrich]

Perhaps we can use a global thread local invocation counter which we increase when starting a constructor and decrease when exiting. This would work with super() calls, and we can always check this cheaply. Perhaps we chat about this Monday morning on zoom.

…

On Fri, 24 Jan 2025 at 12:53 PM, Quinten ***@***.***> wrote: I wonder whether ASM has an API to get this type. Since it isn't an object yet I can inject in something like INSTANCEOF // "java/lang/Object" And do something if this is the case, but not quite sure what to do after this, I can just skip them for now but they should be validated. We could also try to track the invokespecial invocations to the super constructor. Yeah, that was what I was also thinking, this might be the way to do it, because I already have a first visitor for the annotations I could even store the location of the invocations to the super constructor. I would just need to find a way to store the values from the stack till the end — Reply to this email directly, view it on GitHub <#5 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASSV5T27UM4EAYVHT66BO32MF6IRAVCNFSM6AAAAABU3CEP6GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJRGI2DSNBSHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[jensdietrich]

It would of course be easier to set a flag in visitMethod when visiting a constructor (<init>) and checking this flag in MethodVisitor::visitFieldInsn, but constructors can call helper methods which can set fields.

Even with the constructor invocation counter idea there is still a corner case we have to consider: <init> invocations can result in exceptions. We have to ensure that in this case the counter is decreased.

[qwsmit]

Working from today:

To avoid the issue in a constructor where the instrumentation validate code will be run before the super constructor is called we will have different behaviour for <init> and all other methods.

For all <init> methods the validation will be called at the end of the method when the return bytecode command is given. All the current fields will be found by using reflection.

For all non <init> methods the Validate method is injected before the PutField call same as before.