From d80463c456f29329420dca1bd0fa3e665fa5d6e9 Mon Sep 17 00:00:00 2001 From: roterEmil Date: Thu, 1 Feb 2024 11:30:07 +0100 Subject: [PATCH 01/33] fixed prematurely read issue --- .../PrematurelyReadOfFinalField.java | 45 ++++++++++++++ .../ThisEscapesDuringConstruction.java | 23 +++++++ .../ValueReadBeforeAssignment.java | 25 ++++++++ .../sandbox/JSAllocationWriteFieldFromJS.java | 31 ++++++++++ .../JavaAcccessJSObject2EvalCalls.java | 59 ++++++++++++++++++ .../AbstractFieldAssignabilityAnalysis.scala | 21 ++++--- .../L2FieldAssignabilityAnalysis.scala | 62 +++++++++++++------ 7 files changed, 238 insertions(+), 28 deletions(-) create mode 100644 DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java create mode 100644 DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java create mode 100644 DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ValueReadBeforeAssignment.java create mode 100644 DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JSAllocationWriteFieldFromJS.java create mode 100644 DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JavaAcccessJSObject2EvalCalls.java diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java new file mode 100644 index 0000000000..7f00518e33 --- /dev/null +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java @@ -0,0 +1,45 @@ +/* BSD 2-Clause License - see OPAL/LICENSE for details. */ +package org.opalj.fpcf.fixtures.immutability.openworld.assignability.advanced_counter_examples; + +import org.opalj.fpcf.properties.immutability.field_assignability.AssignableField; + +/** + * The default value of the field x is assigned to another field n during construction and as + * a result seen with two different values. + */ +public class PrematurelyReadOfFinalField { + + @AssignableField("Field n is assigned with different values.") + static int n = 5; + + public static void main(String[] args) { + System.out.println("Value A.X before constructor:" + PrematurelyReadOfFinalField.n); + C c = new C(); + System.out.println("Value A.X after constructor:" + PrematurelyReadOfFinalField.n); + System.out.println("Value C.x after constructor:" + c.x ); + } + +} +class B { + + B() { + PrematurelyReadOfFinalField.n = ((C) this).x; + } + + void b(C c) { + PrematurelyReadOfFinalField.n = c.x; + } + +} + +class C extends B{ + + @AssignableField("Is seen with two different values during construction.") + public final int x; + + C() { + super(); + //this.b(this); + x = 3; + } +} diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java new file mode 100644 index 0000000000..dbc98c06bc --- /dev/null +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java @@ -0,0 +1,23 @@ +/* BSD 2-Clause License - see OPAL/LICENSE for details. */ +package org.opalj.fpcf.fixtures.immutability.openworld.assignability.advanced_counter_examples; + +import org.opalj.fpcf.properties.immutability.field_assignability.AssignableField; + +/** + * This test case simulates the fact that the this object escapes in the constructor before (final) fields + * are assigned. + */ +public class ThisEscapesDuringConstruction { + + @AssignableField("The this object escapes in the constructor before the field is assigned.") + final int n; + + public ThisEscapesDuringConstruction(){ + C2.m(this); + n = 7; + } +} + +class C2{ + public static void m(ThisEscapesDuringConstruction c){} +} diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ValueReadBeforeAssignment.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ValueReadBeforeAssignment.java new file mode 100644 index 0000000000..57d7c485f9 --- /dev/null +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ValueReadBeforeAssignment.java @@ -0,0 +1,25 @@ +/* BSD 2-Clause License - see OPAL/LICENSE for details. */ +package org.opalj.fpcf.fixtures.immutability.openworld.assignability.advanced_counter_examples; + +import org.opalj.fpcf.properties.immutability.field_assignability.AssignableField; + +/** + * The value of the field x is read with its default value (0) + * in the constructor before assignment and assigned to a public field. + * Thus, the value can be accessed from everywhere. + */ +public class ValueReadBeforeAssignment { + @AssignableField("Field value is read before assignment.") + private int x; + @AssignableField("Field y is public and not final.") + public int y; + + public ValueReadBeforeAssignment() { + y = x; + x = 42; + } + + public ValueReadBeforeAssignment foo() { + return new ValueReadBeforeAssignment(); + } +} diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JSAllocationWriteFieldFromJS.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JSAllocationWriteFieldFromJS.java new file mode 100644 index 0000000000..eb79b36727 --- /dev/null +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JSAllocationWriteFieldFromJS.java @@ -0,0 +1,31 @@ +package org.opalj.fpcf.fixtures.xl.js.sandbox; +/* +import org.opalj.fpcf.properties.pts.JavaMethodContextAllocSite; +import org.opalj.fpcf.properties.pts.PointsToSet; + +import javax.script.ScriptEngine; +import javax.script.ScriptEngineManager; +import javax.script.ScriptException; +*/ +/** + * javascript code modifies a Java instance field (instantiated from Javascript). + * + * + */ +/*public class JSAllocationWriteFieldFromJS { + + public static void main(String args[]) throws ScriptException, NoSuchMethodException { + ScriptEngineManager sem = new ScriptEngineManager(); + ScriptEngine se = sem.getEngineByName("JavaScript"); + Object myobject = new Object(); + System.out.println(myobject); + se.put("myobject", myobject); + se.eval("var javaTestClass = Java.type(\"org.opalj.fpcf.fixtures.xl.js.stateaccess.intraprocedural.unidirectional.JSAccessJava.JSAllocationWriteFieldFromJS\"); var instance = new javaTestClass(); instance.myfield = myobject"); + JSAllocationWriteFieldFromJS instance = (JSAllocationWriteFieldFromJS)se.get("instance"); + Object instancefield = instance.myfield; + System.out.println(instancefield); + + } + public Object myfield; +} +*/ \ No newline at end of file diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JavaAcccessJSObject2EvalCalls.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JavaAcccessJSObject2EvalCalls.java new file mode 100644 index 0000000000..20ddf7f52f --- /dev/null +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JavaAcccessJSObject2EvalCalls.java @@ -0,0 +1,59 @@ +package org.opalj.fpcf.fixtures.xl.js.sandbox; +/* +import org.opalj.fpcf.properties.pts.JavaMethodContextAllocSite; +import org.opalj.fpcf.properties.pts.PointsToSet; + +import javax.script.ScriptEngine; +import javax.script.ScriptEngineManager; +import javax.script.ScriptException; + +/** + * java modifies js state (setMember not yet supported) + * field of JS object is set using eval. in separate eval, field is read from same JS object + * (setMember: https://github.com/Sdk0815/logbook-kai/blob/6b6c62882de761c114c51c32425a885c55992137/src/main/java/logbook/internal/gui/BattleLogScriptController.java#L15) + * + *//* +public class JavaAcccessJSObject2EvalCalls { + @PointsToSet(variableDefinition = 38, + expectedJavaAllocSites = { + @JavaMethodContextAllocSite( + cf = JavaAcccessJSObject2EvalCalls.class, + methodName = "main", + methodDescriptor = "(java.lang.String[]): void", + allocSiteLinenumber = 35, + allocatedType = "org.opalj.fpcf.fixtures.xl.js.sandbox.O") + + } + ) + public static void main(String args[]) throws ScriptException, NoSuchMethodException { + JavaAcccessJSObject2EvalCalls instance = new JavaAcccessJSObject2EvalCalls(); + ScriptEngineManager sem = new ScriptEngineManager(); + ScriptEngine se = sem.getEngineByName("JavaScript"); + se.put("instance", instance); + se.eval("var n = {'a':'b'};"); + O n = new O(); // se.get("n"); + O myobject = new O(); + System.out.println(myobject); + setJSField(se, myobject, n); + Object getField = getJSField(se, n); + System.out.println(getField); + } + + private static Object getJSField(ScriptEngine se, Object n) throws ScriptException { + se.put("o2", n); + se.eval("var result = o2.field"); + Object getField = se.get("result"); + return getField; + } + + private static void setJSField(ScriptEngine se, Object fieldValue, Object jsObject) throws ScriptException { + se.put("o", fieldValue); + se.put("n2", jsObject); + se.eval("n2.field = o;"); + } + + +} +class O{ + Object field = new Object(); +}*/ \ No newline at end of file diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index 271c1bb826..e6c41fc948 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -128,21 +128,25 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { implicit val state: AnalysisState = createState(field) - if (field.isFinal) - return Result(field, NonAssignable); - else - state.fieldAssignability = EffectivelyNonAssignable + val fieldName = field.name + println(fieldName) + + state.fieldAssignability = + if (field.isFinal) + NonAssignable + else + EffectivelyNonAssignable - if (field.isPublic) + if (field.isPublic && !field.isFinal) return Result(field, Assignable); val thisType = field.classFile.thisType - if (field.isPublic) { + if (field.isPublic && !field.isFinal) { if (typeExtensibility(ObjectType.Object).isYesOrUnknown) { return Result(field, Assignable); } - } else if (field.isProtected) { + } else if (field.isProtected && !field.isFinal) { if (typeExtensibility(thisType).isYesOrUnknown) { return Result(field, Assignable); } @@ -150,7 +154,8 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { return Result(field, Assignable); } } - if (field.isPackagePrivate) { + + if (field.isPackagePrivate && !field.isFinal) { if (!closedPackages(thisType.packageName)) { return Result(field, Assignable); } diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 3f1209aa4b..8aa885f12b 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -92,18 +92,17 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) pc: PC, receiver: AccessReceiver )(implicit state: AnalysisState): Boolean = { + val field = state.field val method = definedMethod.definedMethod val stmts = taCode.stmts val receiverVar = receiver.map(uVarForDefSites(_, taCode.pcToIndex)) val index = taCode.pcToIndex(pc) - if (method.isInitializer) { - if (field.isStatic) { - method.isConstructor - } else { - receiverVar.isDefined && receiverVar.get.definedBy != SelfReferenceParameter - } + if (method.isInitializer && method.classFile == field.classFile) { + field.isStatic && method.isConstructor || + receiverVar.isDefined && receiverVar.get.definedBy != SelfReferenceParameter || + checkWriteDominance(definedMethod, taCode, receiverVar, index) } else { if (field.isStatic || receiverVar.isDefined && receiverVar.get.definedBy == SelfReferenceParameter) { // We consider lazy initialization if there is only single write @@ -152,24 +151,45 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) if (writesInMethod.distinctBy(_._2).size > 1) return true; // Field is written in multiple locations, thus must be assignable - // If we have no information about the receiver, we soundly return - if (receiverVar.isEmpty) + // If we have no information about the receiver, we soundly return true + // However, a static field has no receiver + if (receiverVar.isEmpty && !state.field.isStatic) return true; - val assignedValueObject = receiverVar.get - if (assignedValueObject.definedBy.exists(_ < 0)) + val assignedValueObject = + if (index > 0 && stmts(index).isPutStatic) { + stmts(index).asPutStatic.value.asVar + } else + receiverVar.get + + // When there are more than 1 definitionsite, we soundly return true + if (assignedValueObject.definedBy.size != 1) + return true; + + val definitionSite = assignedValueObject.definedBy.head + + if (definitionSite < -1 || + (definitionSite == -1 && !definedMethod.definedMethod.isConstructor) + ) return true; - val assignedValueObjectVar = stmts(assignedValueObject.definedBy.head).asAssignment.targetVar.asVar + val uses = if (definitionSite == -1) + taCode.params.thisParameter.useSites + else { + val assignedValueObjectVar = stmts(definitionSite).asAssignment.targetVar.asVar + if (assignedValueObjectVar != null) + assignedValueObjectVar.usedBy + else IntTrieSet.empty + } val fieldWriteInMethodIndex = taCode.pcToIndex(writesInMethod.head._2) - if (assignedValueObjectVar != null && !assignedValueObjectVar.usedBy.forall { index => + if (!uses.forall { index => val stmt = stmts(index) fieldWriteInMethodIndex == index || // The value is itself written to another object // IMPROVE: Can we use field access information to care about reflective accesses here? stmt.isPutField && stmt.asPutField.name != state.field.name || - stmt.isAssignment && stmt.asAssignment.targetVar == assignedValueObjectVar || + // stmt.isAssignment && stmt.asAssignment.targetVar == assignedValueObjectVar || stmt.isMethodCall && stmt.asMethodCall.name == "" || // CHECK do we really need the taCode here? dominates(fieldWriteInMethodIndex, index, taCode) @@ -256,15 +276,17 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) fieldReadAccessInformation.numIndirectAccesses - seenIndirectAccesses ).exists { readAccess => val method = contextProvider.contextFromId(readAccess._1).method - (writeAccess._1 eq method) && { - val taCode = state.tacDependees(method.asDefinedMethod).ub.tac.get - if (readAccess._3.isDefined && readAccess._3.get._2.forall(isFormalParameter)) { - false - } else { - !dominates(writeAccess._4, taCode.pcToIndex(readAccess._2), taCode) + method.definedMethod.classFile != state.field.classFile || + (writeAccess._1 eq method) && { + val taCode = state.tacDependees(method.asDefinedMethod).ub.tac.get + + if (readAccess._3.isDefined && readAccess._3.get._2.forall(isFormalParameter)) { + false + } else { + !dominates(writeAccess._4, taCode.pcToIndex(readAccess._2), taCode) + } } - } } } } From 624d6716af8f28091db7c5616eb166eaf06dd01a Mon Sep 17 00:00:00 2001 From: roterEmil Date: Tue, 30 Jul 2024 14:14:41 +0200 Subject: [PATCH 02/33] deleted sandbox files --- .../sandbox/JSAllocationWriteFieldFromJS.java | 31 ---------- .../JavaAcccessJSObject2EvalCalls.java | 59 ------------------- 2 files changed, 90 deletions(-) delete mode 100644 DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JSAllocationWriteFieldFromJS.java delete mode 100644 DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JavaAcccessJSObject2EvalCalls.java diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JSAllocationWriteFieldFromJS.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JSAllocationWriteFieldFromJS.java deleted file mode 100644 index eb79b36727..0000000000 --- a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JSAllocationWriteFieldFromJS.java +++ /dev/null @@ -1,31 +0,0 @@ -package org.opalj.fpcf.fixtures.xl.js.sandbox; -/* -import org.opalj.fpcf.properties.pts.JavaMethodContextAllocSite; -import org.opalj.fpcf.properties.pts.PointsToSet; - -import javax.script.ScriptEngine; -import javax.script.ScriptEngineManager; -import javax.script.ScriptException; -*/ -/** - * javascript code modifies a Java instance field (instantiated from Javascript). - * - * - */ -/*public class JSAllocationWriteFieldFromJS { - - public static void main(String args[]) throws ScriptException, NoSuchMethodException { - ScriptEngineManager sem = new ScriptEngineManager(); - ScriptEngine se = sem.getEngineByName("JavaScript"); - Object myobject = new Object(); - System.out.println(myobject); - se.put("myobject", myobject); - se.eval("var javaTestClass = Java.type(\"org.opalj.fpcf.fixtures.xl.js.stateaccess.intraprocedural.unidirectional.JSAccessJava.JSAllocationWriteFieldFromJS\"); var instance = new javaTestClass(); instance.myfield = myobject"); - JSAllocationWriteFieldFromJS instance = (JSAllocationWriteFieldFromJS)se.get("instance"); - Object instancefield = instance.myfield; - System.out.println(instancefield); - - } - public Object myfield; -} -*/ \ No newline at end of file diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JavaAcccessJSObject2EvalCalls.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JavaAcccessJSObject2EvalCalls.java deleted file mode 100644 index 20ddf7f52f..0000000000 --- a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/xl/js/sandbox/JavaAcccessJSObject2EvalCalls.java +++ /dev/null @@ -1,59 +0,0 @@ -package org.opalj.fpcf.fixtures.xl.js.sandbox; -/* -import org.opalj.fpcf.properties.pts.JavaMethodContextAllocSite; -import org.opalj.fpcf.properties.pts.PointsToSet; - -import javax.script.ScriptEngine; -import javax.script.ScriptEngineManager; -import javax.script.ScriptException; - -/** - * java modifies js state (setMember not yet supported) - * field of JS object is set using eval. in separate eval, field is read from same JS object - * (setMember: https://github.com/Sdk0815/logbook-kai/blob/6b6c62882de761c114c51c32425a885c55992137/src/main/java/logbook/internal/gui/BattleLogScriptController.java#L15) - * - *//* -public class JavaAcccessJSObject2EvalCalls { - @PointsToSet(variableDefinition = 38, - expectedJavaAllocSites = { - @JavaMethodContextAllocSite( - cf = JavaAcccessJSObject2EvalCalls.class, - methodName = "main", - methodDescriptor = "(java.lang.String[]): void", - allocSiteLinenumber = 35, - allocatedType = "org.opalj.fpcf.fixtures.xl.js.sandbox.O") - - } - ) - public static void main(String args[]) throws ScriptException, NoSuchMethodException { - JavaAcccessJSObject2EvalCalls instance = new JavaAcccessJSObject2EvalCalls(); - ScriptEngineManager sem = new ScriptEngineManager(); - ScriptEngine se = sem.getEngineByName("JavaScript"); - se.put("instance", instance); - se.eval("var n = {'a':'b'};"); - O n = new O(); // se.get("n"); - O myobject = new O(); - System.out.println(myobject); - setJSField(se, myobject, n); - Object getField = getJSField(se, n); - System.out.println(getField); - } - - private static Object getJSField(ScriptEngine se, Object n) throws ScriptException { - se.put("o2", n); - se.eval("var result = o2.field"); - Object getField = se.get("result"); - return getField; - } - - private static void setJSField(ScriptEngine se, Object fieldValue, Object jsObject) throws ScriptException { - se.put("o", fieldValue); - se.put("n2", jsObject); - se.eval("n2.field = o;"); - } - - -} -class O{ - Object field = new Object(); -}*/ \ No newline at end of file From fef5c6d4cfeb738fbb2828b3107f98c6314f3520 Mon Sep 17 00:00:00 2001 From: roterEmil Date: Thu, 1 Aug 2024 10:24:33 +0200 Subject: [PATCH 03/33] removed debug code --- .../AbstractFieldAssignabilityAnalysis.scala | 3 --- 1 file changed, 3 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index e6c41fc948..7948dcf64d 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -128,9 +128,6 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { implicit val state: AnalysisState = createState(field) - val fieldName = field.name - println(fieldName) - state.fieldAssignability = if (field.isFinal) NonAssignable From 0d762436e4d76306ed217eeaedd61620768c4df1 Mon Sep 17 00:00:00 2001 From: roterEmil <23134918+roterEmil@users.noreply.github.com> Date: Fri, 20 Sep 2024 14:13:43 +0200 Subject: [PATCH 04/33] Update PrematurelyReadOfFinalField.java due to Dominiks review --- .../PrematurelyReadOfFinalField.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java index 7f00518e33..1ada8d1d0e 100644 --- a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java @@ -13,23 +13,14 @@ public class PrematurelyReadOfFinalField { static int n = 5; public static void main(String[] args) { - System.out.println("Value A.X before constructor:" + PrematurelyReadOfFinalField.n); C c = new C(); - System.out.println("Value A.X after constructor:" + PrematurelyReadOfFinalField.n); - System.out.println("Value C.x after constructor:" + c.x ); } } class B { - B() { PrematurelyReadOfFinalField.n = ((C) this).x; } - - void b(C c) { - PrematurelyReadOfFinalField.n = c.x; - } - } class C extends B{ @@ -39,7 +30,6 @@ class C extends B{ C() { super(); - //this.b(this); x = 3; } } From 253c4fc93cfc3eafa9fb0c6dfa8b29119f6e4eca Mon Sep 17 00:00:00 2001 From: roterEmil <23134918+roterEmil@users.noreply.github.com> Date: Fri, 20 Sep 2024 14:14:39 +0200 Subject: [PATCH 05/33] Update PrematurelyReadOfFinalField.java inserted line break --- .../advanced_counter_examples/PrematurelyReadOfFinalField.java | 1 + 1 file changed, 1 insertion(+) diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java index 1ada8d1d0e..3f7db26cb9 100644 --- a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java @@ -17,6 +17,7 @@ public static void main(String[] args) { } } + class B { B() { PrematurelyReadOfFinalField.n = ((C) this).x; From 2f1d9de7420f4efec0dd805a7bbb9589355d82c8 Mon Sep 17 00:00:00 2001 From: roterEmil Date: Wed, 2 Oct 2024 10:43:41 +0200 Subject: [PATCH 06/33] refactoring for more readability --- .../AbstractFieldAssignabilityAnalysis.scala | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index e6c41fc948..f8e5c9c0e4 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -137,29 +137,20 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { else EffectivelyNonAssignable - if (field.isPublic && !field.isFinal) - return Result(field, Assignable); - val thisType = field.classFile.thisType - - if (field.isPublic && !field.isFinal) { - if (typeExtensibility(ObjectType.Object).isYesOrUnknown) { - return Result(field, Assignable); - } - } else if (field.isProtected && !field.isFinal) { - if (typeExtensibility(thisType).isYesOrUnknown) { - return Result(field, Assignable); - } - if (!closedPackages(thisType.packageName)) { + if(!field.isFinal) { + if (field.isPublic) { + return Result(field, Assignable) + } else if (field.isProtected) { + if (typeExtensibility(thisType).isYesOrUnknown || !closedPackages(thisType.packageName)) { return Result(field, Assignable); } } - if (field.isPackagePrivate && !field.isFinal) { - if (!closedPackages(thisType.packageName)) { + if (field.isPackagePrivate && !closedPackages(thisType.packageName)) { return Result(field, Assignable); - } } + } val fwaiEP = propertyStore(declaredFields(field), FieldWriteAccessInformation.key) From 9cce192ee1eadc599fc165ab3c6b3ec356dfc1c7 Mon Sep 17 00:00:00 2001 From: roterEmil Date: Fri, 18 Oct 2024 14:23:27 +0200 Subject: [PATCH 07/33] fixed multiple assignments of final fields in different branches and assignments of array values --- .../L2FieldAssignabilityAnalysis.scala | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 41cbfb5600..9a717b6785 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -148,8 +148,12 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) val writes = state.fieldWriteAccessDependee.get.ub.accesses val writesInMethod = writes.filter { w => contextProvider.contextFromId(w._1).method eq definedMethod }.toSeq - if (writesInMethod.distinctBy(_._2).size > 1) - return true; // Field is written in multiple locations, thus must be assignable + if (writesInMethod.distinctBy(_._2).size > 1) { + // There can be multiple assignments in the constructor + // in different branches to final fields + if(!definedMethod.definedMethod.isConstructor) + return true + }; // Field is written in multiple locations, thus must be assignable // If we have no information about the receiver, we soundly return true // However, a static field has no receiver @@ -159,6 +163,8 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) val assignedValueObject = if (index > 0 && stmts(index).isPutStatic) { stmts(index).asPutStatic.value.asVar + } else if (index > 0 && stmts(index).isPutField && stmts(index).asPutField.value.asVar.value.isArrayValue.isYes) { + stmts(index).asPutField.value.asVar } else receiverVar.get @@ -192,7 +198,8 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) // stmt.isAssignment && stmt.asAssignment.targetVar == assignedValueObjectVar || stmt.isMethodCall && stmt.asMethodCall.name == "" || // CHECK do we really need the taCode here? - dominates(fieldWriteInMethodIndex, index, taCode) + !dominates(index, fieldWriteInMethodIndex, taCode) || stmt.isArrayStore //TODO check + } ) return true; From 4675ef71bd64a1fb372f91eae006546a99749c23 Mon Sep 17 00:00:00 2001 From: roterEmil Date: Fri, 18 Oct 2024 14:44:25 +0200 Subject: [PATCH 08/33] formatting --- .../AbstractFieldAssignabilityAnalysis.scala | 18 +++++++++--------- .../L2FieldAssignabilityAnalysis.scala | 8 +++++--- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index 1d1af34b9f..495fd85268 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -135,19 +135,19 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { EffectivelyNonAssignable val thisType = field.classFile.thisType - if(!field.isFinal) { - if (field.isPublic) { - return Result(field, Assignable) - } else if (field.isProtected) { - if (typeExtensibility(thisType).isYesOrUnknown || !closedPackages(thisType.packageName)) { - return Result(field, Assignable); + if (!field.isFinal) { + if (field.isPublic) { + return Result(field, Assignable) + } else if (field.isProtected) { + if (typeExtensibility(thisType).isYesOrUnknown || !closedPackages(thisType.packageName)) { + return Result(field, Assignable); + } } - } - if (field.isPackagePrivate && !closedPackages(thisType.packageName)) { + if (field.isPackagePrivate && !closedPackages(thisType.packageName)) { return Result(field, Assignable); + } } - } val fwaiEP = propertyStore(declaredFields(field), FieldWriteAccessInformation.key) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 9a717b6785..68b56700e5 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -151,7 +151,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) if (writesInMethod.distinctBy(_._2).size > 1) { // There can be multiple assignments in the constructor // in different branches to final fields - if(!definedMethod.definedMethod.isConstructor) + if (!definedMethod.definedMethod.isConstructor) return true }; // Field is written in multiple locations, thus must be assignable @@ -163,7 +163,9 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) val assignedValueObject = if (index > 0 && stmts(index).isPutStatic) { stmts(index).asPutStatic.value.asVar - } else if (index > 0 && stmts(index).isPutField && stmts(index).asPutField.value.asVar.value.isArrayValue.isYes) { + } else if ( + index > 0 && stmts(index).isPutField && stmts(index).asPutField.value.asVar.value.isArrayValue.isYes + ) { stmts(index).asPutField.value.asVar } else receiverVar.get @@ -198,7 +200,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) // stmt.isAssignment && stmt.asAssignment.targetVar == assignedValueObjectVar || stmt.isMethodCall && stmt.asMethodCall.name == "" || // CHECK do we really need the taCode here? - !dominates(index, fieldWriteInMethodIndex, taCode) || stmt.isArrayStore //TODO check + !dominates(index, fieldWriteInMethodIndex, taCode) || stmt.isArrayStore // TODO check } ) From f3ba6d7a333db6e7fb88593b4507e45a04987661 Mon Sep 17 00:00:00 2001 From: roterEmil Date: Tue, 22 Oct 2024 11:12:43 +0200 Subject: [PATCH 09/33] fixed grammar --- .../L2FieldAssignabilityAnalysis.scala | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 68b56700e5..aca6603e66 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -83,7 +83,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) /** * Analyzes field writes for a single method, returning false if the field may still be - * effectively final and true otherwise. + * effectively non assignable and true otherwise. */ def methodUpdatesField( definedMethod: DefinedMethod, @@ -105,11 +105,11 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) checkWriteDominance(definedMethod, taCode, receiverVar, index) } else { if (field.isStatic || receiverVar.isDefined && receiverVar.get.definedBy == SelfReferenceParameter) { - // We consider lazy initialization if there is only single write - // outside an initializer, so we can ignore synchronization + // We consider lazy initialization if there is only a single write + // outside an initializer, so we can ignore synchronization. state.fieldAssignability == LazilyInitialized || state.fieldAssignability == UnsafelyLazilyInitialized || - // A field written outside an initializer must be lazily initialized or it is assignable + // A field written outside an initializer must be lazily initialized, or it is assignable { if (considerLazyInitialization) { isAssignable(index, getDefaultValues(), method, taCode) @@ -118,14 +118,13 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } } else if (receiverVar.isDefined && !referenceHasNotEscaped(receiverVar.get, stmts, definedMethod, callers)) { // Here the clone pattern is determined among others - // // note that here we assume real three address code (flat hierarchy) // for instance fields it is okay if they are written in the // constructor (w.r.t. the currently initialized object!) // If the field that is written is not the one referred to by the - // self reference, it is not effectively final. + // self reference, it is not effectively non assignable. // However, a method (e.g. clone) may instantiate a new object and // write the field as long as that new object did not yet escape. @@ -149,11 +148,11 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) val writesInMethod = writes.filter { w => contextProvider.contextFromId(w._1).method eq definedMethod }.toSeq if (writesInMethod.distinctBy(_._2).size > 1) { - // There can be multiple assignments in the constructor - // in different branches to final fields + // There can be multiple assignments of final fields in the constructor + // in different branches if (!definedMethod.definedMethod.isConstructor) return true - }; // Field is written in multiple locations, thus must be assignable + }; // Otherwise: field is written in multiple locations, thus must be assignable // If we have no information about the receiver, we soundly return true // However, a static field has no receiver @@ -170,7 +169,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } else receiverVar.get - // When there are more than 1 definitionsite, we soundly return true + // If there is more than 1 definitionsite, we soundly return true if (assignedValueObject.definedBy.size != 1) return true; @@ -358,7 +357,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) val accessingMethod = contextProvider.contextFromId(w._1).method.definedMethod (accessingMethod ne method) && !accessingMethod.isInitializer }) || - writes.iterator.distinctBy(_._1).size < writes.size // More than one write per method was detected + writes.iterator.distinctBy(_._1).size < writes.size // More than one field write per method was detected false } From 5fa99d8ee7f784129dff424e12dd65f797344852 Mon Sep 17 00:00:00 2001 From: roterEmil Date: Tue, 22 Oct 2024 11:19:48 +0200 Subject: [PATCH 10/33] removed commented out code --- .../fieldassignability/L2FieldAssignabilityAnalysis.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index aca6603e66..6443c597b4 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -196,7 +196,6 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) fieldWriteInMethodIndex == index || // The value is itself written to another object // IMPROVE: Can we use field access information to care about reflective accesses here? stmt.isPutField && stmt.asPutField.name != state.field.name || - // stmt.isAssignment && stmt.asAssignment.targetVar == assignedValueObjectVar || stmt.isMethodCall && stmt.asMethodCall.name == "" || // CHECK do we really need the taCode here? !dominates(index, fieldWriteInMethodIndex, taCode) || stmt.isArrayStore // TODO check From 1fb4123e06b8fe45da6aa369fec90bb815c863ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Tue, 16 Sep 2025 18:23:07 +0200 Subject: [PATCH 11/33] Fix typos and improve style --- ...ld.java => PrematurelyReadFinalField.java} | 6 +-- .../ThisEscapesDuringConstruction.java | 4 +- .../opalj/fpcf/FieldAssignabilityTests.scala | 53 ++----------------- .../L2FieldAssignabilityAnalysis.scala | 52 +++++++++--------- 4 files changed, 36 insertions(+), 79 deletions(-) rename DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/{PrematurelyReadOfFinalField.java => PrematurelyReadFinalField.java} (86%) diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadFinalField.java similarity index 86% rename from DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java rename to DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadFinalField.java index 3f7db26cb9..bee90dcf0b 100644 --- a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadOfFinalField.java +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadFinalField.java @@ -7,7 +7,7 @@ * The default value of the field x is assigned to another field n during construction and as * a result seen with two different values. */ -public class PrematurelyReadOfFinalField { +public class PrematurelyReadFinalField { @AssignableField("Field n is assigned with different values.") static int n = 5; @@ -20,11 +20,11 @@ public static void main(String[] args) { class B { B() { - PrematurelyReadOfFinalField.n = ((C) this).x; + PrematurelyReadFinalField.n = ((C) this).x; } } -class C extends B{ +class C extends B { @AssignableField("Is seen with two different values during construction.") public final int x; diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java index dbc98c06bc..779906c895 100644 --- a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java @@ -4,7 +4,7 @@ import org.opalj.fpcf.properties.immutability.field_assignability.AssignableField; /** - * This test case simulates the fact that the this object escapes in the constructor before (final) fields + * This test case simulates the fact that the `this` object escapes in the constructor before (final) fields * are assigned. */ public class ThisEscapesDuringConstruction { @@ -18,6 +18,6 @@ public ThisEscapesDuringConstruction(){ } } -class C2{ +class C2 { public static void m(ThisEscapesDuringConstruction c){} } diff --git a/DEVELOPING_OPAL/validate/src/test/scala/org/opalj/fpcf/FieldAssignabilityTests.scala b/DEVELOPING_OPAL/validate/src/test/scala/org/opalj/fpcf/FieldAssignabilityTests.scala index b5a2f7afb3..c05e8cb7ba 100644 --- a/DEVELOPING_OPAL/validate/src/test/scala/org/opalj/fpcf/FieldAssignabilityTests.scala +++ b/DEVELOPING_OPAL/validate/src/test/scala/org/opalj/fpcf/FieldAssignabilityTests.scala @@ -16,12 +16,8 @@ import org.opalj.tac.fpcf.analyses.LazyFieldLocalityAnalysis import org.opalj.tac.fpcf.analyses.escape.LazyInterProceduralEscapeAnalysis import org.opalj.tac.fpcf.analyses.escape.LazyReturnValueFreshnessAnalysis import org.opalj.tac.fpcf.analyses.fieldaccess.EagerFieldAccessInformationAnalysis -import org.opalj.tac.fpcf.analyses.fieldassignability.EagerL0FieldAssignabilityAnalysis -import org.opalj.tac.fpcf.analyses.fieldassignability.EagerL1FieldAssignabilityAnalysis import org.opalj.tac.fpcf.analyses.fieldassignability.EagerL2FieldAssignabilityAnalysis -import org.opalj.tac.fpcf.analyses.purity.L2PurityAnalysis import org.opalj.tac.fpcf.analyses.purity.LazyL2PurityAnalysis -import org.opalj.tac.fpcf.analyses.purity.SystemOutLoggingAllExceptionRater /** * Tests the field assignability analysis @@ -49,49 +45,6 @@ class FieldAssignabilityTests extends PropertiesTest { } - describe("no analysis is scheduled") { - val as = executeAnalyses(Set.empty) - as.propertyStore.shutdown() - validateProperties(as, fieldsWithAnnotations(as.project), Set("FieldAssignability")) - } - L2PurityAnalysis.setRater(Some(SystemOutLoggingAllExceptionRater)) - - describe("the org.opalj.fpcf.analyses.L0FieldAssignability is executed") { - - val as = executeAnalyses( - Set( - EagerL0FieldAssignabilityAnalysis, - LazyStaticDataUsageAnalysis, - LazyL2PurityAnalysis, - LazyL0CompileTimeConstancyAnalysis, - LazyInterProceduralEscapeAnalysis, - LazyReturnValueFreshnessAnalysis, - LazyFieldLocalityAnalysis, - EagerFieldAccessInformationAnalysis - ) - ) - as.propertyStore.shutdown() - validateProperties(as, fieldsWithAnnotations(as.project), Set("FieldAssignability")) - } - - describe("the org.opalj.fpcf.analyses.L1FieldAssignability is executed") { - - val as = executeAnalyses( - Set( - EagerL1FieldAssignabilityAnalysis, - LazyStaticDataUsageAnalysis, - LazyL2PurityAnalysis, - LazyL0CompileTimeConstancyAnalysis, - LazyInterProceduralEscapeAnalysis, - LazyReturnValueFreshnessAnalysis, - LazyFieldLocalityAnalysis, - EagerFieldAccessInformationAnalysis - ) - ) - as.propertyStore.shutdown() - validateProperties(as, fieldsWithAnnotations(as.project), Set("FieldAssignability")) - } - describe("the org.opalj.fpcf.analyses.L2FieldAssignability is executed") { val as = executeAnalyses( @@ -107,6 +60,10 @@ class FieldAssignabilityTests extends PropertiesTest { ) ) as.propertyStore.shutdown() - validateProperties(as, fieldsWithAnnotations(as.project), Set("FieldAssignability")) + validateProperties( + as, + fieldsWithAnnotations(as.project).filter(t => t._1.toJava.contains("advanced_counter_examples")), + Set("FieldAssignability") + ) } } diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index c7c45a48a3..0e8478cdf1 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -83,7 +83,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) /** * Analyzes field writes for a single method, returning false if the field may still be - * effectively non assignable and true otherwise. + * effectively non-assignable and true otherwise. */ def methodUpdatesField( definedMethod: DefinedMethod, @@ -92,19 +92,17 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) pc: PC, receiver: AccessReceiver )(implicit state: AnalysisState): Boolean = { - val field = state.field val method = definedMethod.definedMethod - val stmts = taCode.stmts - val receiverVar = receiver.map(uVarForDefSites(_, taCode.pcToIndex)) + val receiverVarOpt = receiver.map(uVarForDefSites(_, taCode.pcToIndex)) val index = taCode.pcToIndex(pc) if (method.isInitializer && method.classFile == field.classFile) { field.isStatic && method.isConstructor || - receiverVar.isDefined && receiverVar.get.definedBy != SelfReferenceParameter || - checkWriteDominance(definedMethod, taCode, receiverVar, index) + receiverVarOpt.isDefined && receiverVarOpt.get.definedBy != SelfReferenceParameter || + checkWriteDominance(definedMethod, taCode, receiverVarOpt, index) } else { - if (field.isStatic || receiverVar.isDefined && receiverVar.get.definedBy == SelfReferenceParameter) { + if (field.isStatic || receiverVarOpt.isDefined && receiverVarOpt.get.definedBy == SelfReferenceParameter) { // We consider lazy initialization if there is only a single write // outside an initializer, so we can ignore synchronization. state.fieldAssignability == LazilyInitialized || @@ -116,7 +114,13 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } else true } - } else if (receiverVar.isDefined && !referenceHasNotEscaped(receiverVar.get, stmts, definedMethod, callers)) { + } else if (receiverVarOpt.isDefined && !referenceHasNotEscaped( + receiverVarOpt.get, + taCode.stmts, + definedMethod, + callers + ) + ) { // Here the clone pattern is determined among others // note that here we assume real three address code (flat hierarchy) @@ -124,15 +128,14 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) // constructor (w.r.t. the currently initialized object!) // If the field that is written is not the one referred to by the - // self reference, it is not effectively non assignable. + // self reference, it is not effectively non-assignable. // However, a method (e.g. clone) may instantiate a new object and // write the field as long as that new object did not yet escape. true } else { - checkWriteDominance(definedMethod, taCode, receiverVar, index) + checkWriteDominance(definedMethod, taCode, receiverVarOpt, index) } - } } @@ -148,14 +151,14 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) val writesInMethod = writes.filter { w => contextProvider.contextFromId(w._1).method eq definedMethod }.toSeq if (writesInMethod.distinctBy(_._2).size > 1) { - // There can be multiple assignments of final fields in the constructor - // in different branches + // There can be multiple assignments of final fields in the constructor in different branches, + // but otherwise if field is written in multiple locations it must be assignable. if (!definedMethod.definedMethod.isConstructor) - return true - }; // Otherwise: field is written in multiple locations, thus must be assignable + return true; + } - // If we have no information about the receiver, we soundly return true - // However, a static field has no receiver + // If we have no information about the receiver, we soundly return true. + // However, a static field has no receiver. if (receiverVar.isEmpty && !state.field.isStatic) return true; @@ -169,21 +172,19 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } else receiverVar.get - // If there is more than 1 definitionsite, we soundly return true + // If there is more than 1 definition site, we soundly return true if (assignedValueObject.definedBy.size != 1) return true; - val definitionSite = assignedValueObject.definedBy.head + val defSite = assignedValueObject.definedBy.head - if (definitionSite < -1 || - (definitionSite == -1 && !definedMethod.definedMethod.isConstructor) - ) + if (defSite < -1 || (defSite == -1 && !definedMethod.definedMethod.isConstructor)) return true; - val uses = if (definitionSite == -1) + val uses = if (defSite == -1) taCode.params.thisParameter.useSites else { - val assignedValueObjectVar = stmts(definitionSite).asAssignment.targetVar.asVar + val assignedValueObjectVar = stmts(defSite).asAssignment.targetVar.asVar if (assignedValueObjectVar != null) assignedValueObjectVar.usedBy else IntTrieSet.empty @@ -204,8 +205,6 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) ) return true; - val writeAccess = (definedMethod, taCode, receiverVar, index) - if (state.fieldReadAccessDependee.isEmpty) { state.fieldReadAccessDependee = Some(propertyStore(declaredFields(state.field), FieldReadAccessInformation.key)) @@ -213,6 +212,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) val fraiEP = state.fieldReadAccessDependee.get + val writeAccess = (definedMethod, taCode, receiverVar, index) if (fraiEP.hasUBP && fieldReadsNotDominated(fraiEP.ub, 0, 0, Seq(writeAccess))) return true; From 11da1b4d3e36e396f2ab6d65d52988fbf27521ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Tue, 16 Sep 2025 18:58:45 +0200 Subject: [PATCH 12/33] Revert minimizing tests --- .../opalj/fpcf/FieldAssignabilityTests.scala | 53 +++++++++++++++++-- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/DEVELOPING_OPAL/validate/src/test/scala/org/opalj/fpcf/FieldAssignabilityTests.scala b/DEVELOPING_OPAL/validate/src/test/scala/org/opalj/fpcf/FieldAssignabilityTests.scala index c05e8cb7ba..b5a2f7afb3 100644 --- a/DEVELOPING_OPAL/validate/src/test/scala/org/opalj/fpcf/FieldAssignabilityTests.scala +++ b/DEVELOPING_OPAL/validate/src/test/scala/org/opalj/fpcf/FieldAssignabilityTests.scala @@ -16,8 +16,12 @@ import org.opalj.tac.fpcf.analyses.LazyFieldLocalityAnalysis import org.opalj.tac.fpcf.analyses.escape.LazyInterProceduralEscapeAnalysis import org.opalj.tac.fpcf.analyses.escape.LazyReturnValueFreshnessAnalysis import org.opalj.tac.fpcf.analyses.fieldaccess.EagerFieldAccessInformationAnalysis +import org.opalj.tac.fpcf.analyses.fieldassignability.EagerL0FieldAssignabilityAnalysis +import org.opalj.tac.fpcf.analyses.fieldassignability.EagerL1FieldAssignabilityAnalysis import org.opalj.tac.fpcf.analyses.fieldassignability.EagerL2FieldAssignabilityAnalysis +import org.opalj.tac.fpcf.analyses.purity.L2PurityAnalysis import org.opalj.tac.fpcf.analyses.purity.LazyL2PurityAnalysis +import org.opalj.tac.fpcf.analyses.purity.SystemOutLoggingAllExceptionRater /** * Tests the field assignability analysis @@ -45,6 +49,49 @@ class FieldAssignabilityTests extends PropertiesTest { } + describe("no analysis is scheduled") { + val as = executeAnalyses(Set.empty) + as.propertyStore.shutdown() + validateProperties(as, fieldsWithAnnotations(as.project), Set("FieldAssignability")) + } + L2PurityAnalysis.setRater(Some(SystemOutLoggingAllExceptionRater)) + + describe("the org.opalj.fpcf.analyses.L0FieldAssignability is executed") { + + val as = executeAnalyses( + Set( + EagerL0FieldAssignabilityAnalysis, + LazyStaticDataUsageAnalysis, + LazyL2PurityAnalysis, + LazyL0CompileTimeConstancyAnalysis, + LazyInterProceduralEscapeAnalysis, + LazyReturnValueFreshnessAnalysis, + LazyFieldLocalityAnalysis, + EagerFieldAccessInformationAnalysis + ) + ) + as.propertyStore.shutdown() + validateProperties(as, fieldsWithAnnotations(as.project), Set("FieldAssignability")) + } + + describe("the org.opalj.fpcf.analyses.L1FieldAssignability is executed") { + + val as = executeAnalyses( + Set( + EagerL1FieldAssignabilityAnalysis, + LazyStaticDataUsageAnalysis, + LazyL2PurityAnalysis, + LazyL0CompileTimeConstancyAnalysis, + LazyInterProceduralEscapeAnalysis, + LazyReturnValueFreshnessAnalysis, + LazyFieldLocalityAnalysis, + EagerFieldAccessInformationAnalysis + ) + ) + as.propertyStore.shutdown() + validateProperties(as, fieldsWithAnnotations(as.project), Set("FieldAssignability")) + } + describe("the org.opalj.fpcf.analyses.L2FieldAssignability is executed") { val as = executeAnalyses( @@ -60,10 +107,6 @@ class FieldAssignabilityTests extends PropertiesTest { ) ) as.propertyStore.shutdown() - validateProperties( - as, - fieldsWithAnnotations(as.project).filter(t => t._1.toJava.contains("advanced_counter_examples")), - Set("FieldAssignability") - ) + validateProperties(as, fieldsWithAnnotations(as.project), Set("FieldAssignability")) } } From 8cbab1f2415fe58290ea7de1d0b2e15515c92d15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Wed, 17 Sep 2025 11:11:27 +0200 Subject: [PATCH 13/33] Improve clarity on assignability analysis --- .../L2FieldAssignabilityAnalysis.scala | 41 ++++++------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 0e8478cdf1..208adccf6f 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -94,23 +94,27 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) )(implicit state: AnalysisState): Boolean = { val field = state.field val method = definedMethod.definedMethod + val writeIndex = taCode.pcToIndex(pc) val receiverVarOpt = receiver.map(uVarForDefSites(_, taCode.pcToIndex)) - val index = taCode.pcToIndex(pc) - if (method.isInitializer && method.classFile == field.classFile) { - field.isStatic && method.isConstructor || + if (field.isStatic && method.isConstructor) { + // A static field updated in an arbitrary constructor may be updated with (at least) the first call. + // Thus, we may see its initial value or the updated value, making the field assignable. + true + } else if (method.isInitializer && method.classFile == field.classFile) { receiverVarOpt.isDefined && receiverVarOpt.get.definedBy != SelfReferenceParameter || - checkWriteDominance(definedMethod, taCode, receiverVarOpt, index) + checkWriteDominance(definedMethod, taCode, receiverVarOpt, writeIndex) } else { if (field.isStatic || receiverVarOpt.isDefined && receiverVarOpt.get.definedBy == SelfReferenceParameter) { - // We consider lazy initialization if there is only a single write - // outside an initializer, so we can ignore synchronization. + // We consider lazy initialization if there is only a single write outside an initializer, so we can + // ignore synchronization. state.fieldAssignability == LazilyInitialized || state.fieldAssignability == UnsafelyLazilyInitialized || - // A field written outside an initializer must be lazily initialized, or it is assignable { if (considerLazyInitialization) { - isAssignable(index, getDefaultValues(), method, taCode) + // A field written outside an initializer must be lazily initialized or it is assignable. + state.fieldAssignability = determineLazyInitialization(writeIndex, getDefaultValues(), method, taCode) + state.fieldAssignability eq Assignable } else true } @@ -134,7 +138,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) // write the field as long as that new object did not yet escape. true } else { - checkWriteDominance(definedMethod, taCode, receiverVarOpt, index) + checkWriteDominance(definedMethod, taCode, receiverVarOpt, writeIndex) } } } @@ -172,12 +176,10 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } else receiverVar.get - // If there is more than 1 definition site, we soundly return true if (assignedValueObject.definedBy.size != 1) return true; val defSite = assignedValueObject.definedBy.head - if (defSite < -1 || (defSite == -1 && !definedMethod.definedMethod.isConstructor)) return true; @@ -331,23 +333,6 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) bbPotentiallyDominator == bbPotentiallyDominated && potentiallyDominatorIndex < potentiallyDominatedIndex } - // lazy initialization: - - /** - * Handles the lazy initialization determination for a field write in a given method - * @author Tobias Roth - * @return true if no lazy initialization was recognized - */ - def isAssignable( - writeIndex: Int, - defaultValues: Set[Any], - method: Method, - taCode: TACode[TACMethodParameter, V] - )(implicit state: AnalysisState): Boolean = { - state.fieldAssignability = determineLazyInitialization(writeIndex, defaultValues, method, taCode) - state.fieldAssignability eq Assignable - } - def hasMultipleNonConstructorWrites(method: Method)(implicit state: AnalysisState): Boolean = { val writes = state.fieldWriteAccessDependee.get.ub.accesses.toSeq From 4ed186bc5c0f6636a9a53331d926ce65c082a6b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Wed, 17 Sep 2025 11:23:39 +0200 Subject: [PATCH 14/33] Improve test case style --- .../advanced_counter_examples/PrematurelyReadFinalField.java | 1 - .../ThisEscapesDuringConstruction.java | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadFinalField.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadFinalField.java index bee90dcf0b..b94859d7e5 100644 --- a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadFinalField.java +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/PrematurelyReadFinalField.java @@ -15,7 +15,6 @@ public class PrematurelyReadFinalField { public static void main(String[] args) { C c = new C(); } - } class B { diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java index 779906c895..b6d2759f50 100644 --- a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/advanced_counter_examples/ThisEscapesDuringConstruction.java @@ -12,12 +12,12 @@ public class ThisEscapesDuringConstruction { @AssignableField("The this object escapes in the constructor before the field is assigned.") final int n; - public ThisEscapesDuringConstruction(){ + public ThisEscapesDuringConstruction() { C2.m(this); n = 7; } } class C2 { - public static void m(ThisEscapesDuringConstruction c){} + public static void m(ThisEscapesDuringConstruction c) {} } From adde0869ee49e38827c5fa303dccda7ef52e19bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Wed, 17 Sep 2025 11:29:18 +0200 Subject: [PATCH 15/33] Improve clarity --- .../L2FieldAssignabilityAnalysis.scala | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 82bcc6d463..69080f99d3 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -135,7 +135,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) definedMethod: DefinedMethod, taCode: TACode[TACMethodParameter, V], receiverVar: Option[V], - index: Int + writeIndex: Int )(implicit state: State): Boolean = { val stmts = taCode.stmts @@ -155,12 +155,12 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) return true; val assignedValueObject = - if (index > 0 && stmts(index).isPutStatic) { - stmts(index).asPutStatic.value.asVar + if (writeIndex > 0 && stmts(writeIndex).isPutStatic) { + stmts(writeIndex).asPutStatic.value.asVar } else if ( - index > 0 && stmts(index).isPutField && stmts(index).asPutField.value.asVar.value.isArrayValue.isYes + writeIndex > 0 && stmts(writeIndex).isPutField && stmts(writeIndex).asPutField.value.asVar.value.isArrayValue.isYes ) { - stmts(index).asPutField.value.asVar + stmts(writeIndex).asPutField.value.asVar } else receiverVar.get @@ -202,7 +202,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) val fraiEP = state.fieldReadAccessDependee.get - val writeAccess = (definedMethod, taCode, receiverVar, index) + val writeAccess = (definedMethod, taCode, receiverVar, writeIndex) if (fraiEP.hasUBP && fieldReadsNotDominated(fraiEP.ub, 0, 0, Seq(writeAccess))) return true; From 13bb641383bc70af52430f16716fdcca017ea799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Wed, 17 Sep 2025 11:49:40 +0200 Subject: [PATCH 16/33] Fix formatting --- .../L2FieldAssignabilityAnalysis.scala | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 69080f99d3..6f8b9de12d 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -108,11 +108,18 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) if (field.isStatic || receiverVarOpt.isDefined && receiverVarOpt.get.definedBy == SelfReferenceParameter) { // A field written outside an initializer must be lazily initialized or it is assignable if (considerLazyInitialization) { - state.fieldAssignability = determineLazyInitialization(writeIndex, getDefaultValues(), method, taCode) + state.fieldAssignability = + determineLazyInitialization(writeIndex, getDefaultValues(), method, taCode) state.fieldAssignability eq Assignable } else true - } else if (receiverVarOpt.isDefined && !referenceHasNotEscaped(receiverVarOpt.get, taCode.stmts, definedMethod, callers)) { + } else if (receiverVarOpt.isDefined && !referenceHasNotEscaped( + receiverVarOpt.get, + taCode.stmts, + definedMethod, + callers + ) + ) { // Here the clone pattern is determined among others // note that here we assume real three address code (flat hierarchy) @@ -149,16 +156,16 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) return true; } - // If we have no information about the receiver, we soundly return true. - // However, a static field has no receiver. + // If we have no information about the receiver, but we should have it, soundly return true. if (receiverVar.isEmpty && !state.field.isStatic) return true; val assignedValueObject = if (writeIndex > 0 && stmts(writeIndex).isPutStatic) { stmts(writeIndex).asPutStatic.value.asVar - } else if ( - writeIndex > 0 && stmts(writeIndex).isPutField && stmts(writeIndex).asPutField.value.asVar.value.isArrayValue.isYes + } else if (writeIndex > 0 && stmts( + writeIndex + ).isPutField && stmts(writeIndex).asPutField.value.asVar.value.isArrayValue.isYes ) { stmts(writeIndex).asPutField.value.asVar } else From f62d0345896e3448b98814e1ce44551148658b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Wed, 17 Sep 2025 12:05:26 +0200 Subject: [PATCH 17/33] Add missing semicolon --- .../fieldassignability/AbstractFieldAssignabilityAnalysis.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index 3b719360b6..4b0289c5e9 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -137,7 +137,7 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { val thisType = field.classFile.thisType if (!field.isFinal) { if (field.isPublic) { - return Result(field, Assignable) + return Result(field, Assignable); } else if (field.isProtected) { if (typeExtensibility(thisType).isYesOrUnknown || !closedPackages(thisType.packageName)) { return Result(field, Assignable); From 13c4f87a0e4b712b9b607959a1416e0bcbf8f96f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Wed, 24 Sep 2025 16:35:59 +0200 Subject: [PATCH 18/33] Encapsulate assignability analysis --- .../L2FieldAssignabilityAnalysis.scala | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 6f8b9de12d..05420dc788 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -76,7 +76,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) extends AbstractFieldAssignabilityAnalysis with FPCFAnalysis { - val considerLazyInitialization: Boolean = + private val considerLazyInitialization: Boolean = project.config.getBoolean( "org.opalj.fpcf.analyses.L2FieldAssignabilityAnalysis.considerLazyInitialization" ) @@ -315,7 +315,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * Determines whether the basic block of a given index dominates the basic block of the other index or is executed * before the other index in the case of both indexes belonging to the same basic block. */ - def dominates( + private def dominates( potentiallyDominatorIndex: Int, potentiallyDominatedIndex: Int, taCode: TACode[TACMethodParameter, V] @@ -327,7 +327,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) bbPotentiallyDominator == bbPotentiallyDominated && potentiallyDominatorIndex < potentiallyDominatedIndex } - def hasMultipleNonConstructorWrites(method: Method)(implicit state: AnalysisState): Boolean = { + private def hasMultipleNonConstructorWrites(method: Method)(implicit state: AnalysisState): Boolean = { val writes = state.fieldWriteAccessDependee.get.ub.accesses.toSeq // prevents writes outside the method and the constructor @@ -342,7 +342,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * Determines the kind of lazy initialization of a given field in the given method through a given field write. * @author Tobias Roth */ - def determineLazyInitialization( + private def determineLazyInitialization( writeIndex: Int, defaultValues: Set[Any], method: Method, @@ -442,7 +442,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) Assignable } - def doFieldReadsEscape( + private def doFieldReadsEscape( reads: Seq[(Int, PC, AccessReceiver, AccessParameter)], method: Method, guardIndex: Int, @@ -515,7 +515,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * The throw statements: (the pc, the definitionSites, the bb of the throw statement) * @author Tobias Roth */ - def findCatchesAndThrows( + private def findCatchesAndThrows( tacCode: TACode[TACMethodParameter, V] ): (List[(Int, IntTrieSet)], List[(Int, IntTrieSet)]) = { var caughtExceptions: List[(Int, IntTrieSet)] = List.empty @@ -545,7 +545,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * @return the index of the monitor enter and exit * @author Tobias Roth */ - def findMonitors( + private def findMonitors( fieldWrite: Int, tacCode: TACode[TACMethodParameter, V] )(implicit state: State): (Option[Int], Option[Int]) = { @@ -638,7 +638,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * first statement executed if the field does not have its default value and the index of the * field read used for the guard and the index of the field-read. */ - def findGuards( + private def findGuards( fieldWrite: Int, defaultValues: Set[Any], taCode: TACode[TACMethodParameter, V] @@ -736,7 +736,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) /** * Returns all predecessor BasicBlocks of a CFGNode. */ - def getPredecessors(node: CFGNode, visited: Set[CFGNode]): List[BasicBlock] = { + private def getPredecessors(node: CFGNode, visited: Set[CFGNode]): List[BasicBlock] = { def getPredecessorsInternal(node: CFGNode, visited: Set[CFGNode]): Iterator[BasicBlock] = { node.predecessors.iterator.flatMap { currentNode => if (currentNode.isBasicBlock) { @@ -754,7 +754,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) /** * Determines whether a node is a transitive predecessor of another node. */ - def isTransitivePredecessor(possiblePredecessor: CFGNode, node: CFGNode): Boolean = { + private def isTransitivePredecessor(possiblePredecessor: CFGNode, node: CFGNode): Boolean = { val visited: mutable.Set[CFGNode] = mutable.Set.empty @@ -774,7 +774,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) /** * Returns all successors BasicBlocks of a CFGNode */ - def getSuccessors(node: CFGNode, visited: Set[CFGNode]): List[BasicBlock] = { + private def getSuccessors(node: CFGNode, visited: Set[CFGNode]): List[BasicBlock] = { def getSuccessorsInternal(node: CFGNode, visited: Set[CFGNode]): Iterator[BasicBlock] = { node.successors.iterator flatMap { currentNode => if (currentNode.isBasicBlock) @@ -790,7 +790,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * Checks if an expression is a field read of the currently analyzed field. * For instance fields, the read must be on the `this` reference. */ - def isReadOfCurrentField( + private def isReadOfCurrentField( expr: Expr[V], tacCode: TACode[TACMethodParameter, V], index: Int @@ -845,7 +845,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * Determines if an if-Statement is actually a guard for the current field, i.e. it compares * the current field to the default value. */ - def isGuard( + private def isGuard( ifStmt: If[V], defaultValues: Set[Any], code: Array[Stmt[V]], @@ -944,7 +944,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) /** * Checks that the returned value is definitely read from the field. */ - def isFieldValueReturned( + private def isFieldValueReturned( write: FieldWriteAccessStmt[V], writeIndex: Int, readIndex: Int, @@ -1036,11 +1036,7 @@ object LazyL2FieldAssignabilityAnalysis extends L2FieldAssignabilityAnalysisSche override def derivesLazily: Some[PropertyBounds] = Some(derivedProperty) - override final def register( - p: SomeProject, - ps: PropertyStore, - unused: Null - ): FPCFAnalysis = { + override final def register(p: SomeProject, ps: PropertyStore, unused: Null): FPCFAnalysis = { val analysis = new L2FieldAssignabilityAnalysis(p) ps.registerLazyPropertyComputation( FieldAssignability.key, From df793f7e45dad71ae41f16bfe6cfb446c14fa082 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Wed, 24 Sep 2025 17:18:50 +0200 Subject: [PATCH 19/33] Simplify mental overhead in assignability --- .../AbstractFieldAssignabilityAnalysis.scala | 10 +++++----- .../L1FieldAssignabilityAnalysis.scala | 2 +- .../L2FieldAssignabilityAnalysis.scala | 8 ++------ 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index 4b0289c5e9..19c38a2591 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -205,19 +205,19 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { /** * Checks whether the object reference of a PutField does not escape (except for being returned). */ - def referenceHasNotEscaped( + def referenceHasEscaped( ref: V, stmts: Array[Stmt[V]], method: DefinedMethod, callers: Callers )(implicit state: AnalysisState): Boolean = { ref.definedBy.forall { defSite => - if (defSite < 0) false // Must be locally created + if (defSite < 0) true // Must be locally created else { val definition = stmts(defSite).asAssignment // Must either be null or freshly allocated - if (definition.expr.isNullExpr) true - else if (!definition.expr.isNew) false + if (definition.expr.isNullExpr) false + else if (!definition.expr.isNew) true else { var hasEscaped = false callers.forNewCalleeContexts(null, method) { context => @@ -225,7 +225,7 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { val escapeProperty = propertyStore(entity, EscapeProperty.key) hasEscaped ||= handleEscapeProperty(escapeProperty) } - !hasEscaped + hasEscaped } } } diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala index 1d0ed2eb20..61d16da8ce 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala @@ -64,7 +64,7 @@ class L1FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) // write the field as long as that new object did not yet escape. (!method.isConstructor || objRef.definedBy != SelfReferenceParameter) && - !referenceHasNotEscaped(objRef, stmts, definedMethod, callers) + referenceHasEscaped(objRef, stmts, definedMethod, callers) } else { !method.isStaticInitializer } diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 05420dc788..bce8b58bfe 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -113,12 +113,8 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) state.fieldAssignability eq Assignable } else true - } else if (receiverVarOpt.isDefined && !referenceHasNotEscaped( - receiverVarOpt.get, - taCode.stmts, - definedMethod, - callers - ) + } else if (receiverVarOpt.isDefined && + referenceHasEscaped(receiverVarOpt.get, taCode.stmts, definedMethod, callers) ) { // Here the clone pattern is determined among others // note that here we assume real three address code (flat hierarchy) From 9ce7d9ebf97d566b11afecbe9152d0836de17803 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Thu, 2 Oct 2025 13:55:24 +0100 Subject: [PATCH 20/33] Move state up --- .../L2FieldAssignabilityAnalysis.scala | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index bce8b58bfe..2c871c4cf0 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -81,6 +81,23 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) "org.opalj.fpcf.analyses.L2FieldAssignabilityAnalysis.considerLazyInitialization" ) + case class State( + field: Field + ) extends AbstractFieldAssignabilityAnalysisState { + var checkLazyInit: Option[(Method, Int, Int, TACode[TACMethodParameter, V])] = None + var openWrites = List.empty[(DefinedMethod, TACode[TACMethodParameter, V], Option[V], PC)] + + var fieldReadAccessDependee: Option[EOptionP[DeclaredField, FieldReadAccessInformation]] = None + + override def hasDependees: Boolean = fieldReadAccessDependee.exists(_.isRefinable) || super.hasDependees + + override def dependees: Set[SomeEOptionP] = super.dependees ++ fieldReadAccessDependee.filter(_.isRefinable) + } + + type AnalysisState = State + + override def createState(field: Field): AnalysisState = State(field) + /** * Analyzes field writes for a single method, returning false if the field may still be * effectively non-assignable and true otherwise. @@ -290,23 +307,6 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } } - case class State( - field: Field - ) extends AbstractFieldAssignabilityAnalysisState { - var checkLazyInit: Option[(Method, Int, Int, TACode[TACMethodParameter, V])] = None - var openWrites = List.empty[(DefinedMethod, TACode[TACMethodParameter, V], Option[V], PC)] - - var fieldReadAccessDependee: Option[EOptionP[DeclaredField, FieldReadAccessInformation]] = None - - override def hasDependees: Boolean = fieldReadAccessDependee.exists(_.isRefinable) || super.hasDependees - - override def dependees: Set[SomeEOptionP] = super.dependees ++ fieldReadAccessDependee.filter(_.isRefinable) - } - - type AnalysisState = State - - override def createState(field: Field): AnalysisState = State(field) - /** * Determines whether the basic block of a given index dominates the basic block of the other index or is executed * before the other index in the case of both indexes belonging to the same basic block. From ae0fc47a9f72e51c04859adaaa54bc3eaedb176a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Thu, 2 Oct 2025 17:23:16 +0100 Subject: [PATCH 21/33] Large refactor of assignability analysis --- .../AbstractFieldAssignabilityAnalysis.scala | 253 +++++++----------- .../L1FieldAssignabilityAnalysis.scala | 49 ++-- .../L2FieldAssignabilityAnalysis.scala | 248 +++++++---------- 3 files changed, 221 insertions(+), 329 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index 19c38a2591..52af6057c0 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -37,7 +37,6 @@ import org.opalj.br.fpcf.properties.EscapeInCallee import org.opalj.br.fpcf.properties.EscapeProperty import org.opalj.br.fpcf.properties.EscapeViaReturn import org.opalj.br.fpcf.properties.NoEscape -import org.opalj.br.fpcf.properties.cg.Callers import org.opalj.br.fpcf.properties.fieldaccess.AccessReceiver import org.opalj.br.fpcf.properties.fieldaccess.FieldWriteAccessInformation import org.opalj.br.fpcf.properties.immutability.Assignable @@ -59,8 +58,6 @@ import org.opalj.fpcf.SomeInterimEP import org.opalj.fpcf.UBP import org.opalj.tac.DUVar import org.opalj.tac.Stmt -import org.opalj.tac.TACMethodParameter -import org.opalj.tac.TACode import org.opalj.tac.common.DefinitionSite import org.opalj.tac.common.DefinitionSitesKey import org.opalj.tac.fpcf.properties.TACAI @@ -72,22 +69,19 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { val field: Field var fieldAssignability: FieldAssignability = NonAssignable - var fieldAccesses: Map[DefinedMethod, Set[(PC, AccessReceiver)]] = Map.empty + var fieldAccesses: Map[DefinedMethod, Map[Int, Set[(PC, AccessReceiver)]]] = Map.empty var escapeDependees: Set[EOptionP[(Context, DefinitionSite), EscapeProperty]] = Set.empty var fieldWriteAccessDependee: Option[EOptionP[DeclaredField, FieldWriteAccessInformation]] = None var tacDependees: Map[DefinedMethod, EOptionP[Method, TACAI]] = Map.empty - var callerDependees: Map[DefinedMethod, EOptionP[DefinedMethod, Callers]] = Map.empty.withDefault { dm => - propertyStore(dm, Callers.key) - } def hasDependees: Boolean = { escapeDependees.nonEmpty || fieldWriteAccessDependee.exists(_.isRefinable) || - tacDependees.valuesIterator.exists(_.isRefinable) || callerDependees.valuesIterator.exists(_.isRefinable) + tacDependees.valuesIterator.exists(_.isRefinable) } def dependees: Set[SomeEOptionP] = { escapeDependees ++ fieldWriteAccessDependee.filter(_.isRefinable) ++ - callerDependees.valuesIterator.filter(_.isRefinable) ++ tacDependees.valuesIterator.filter(_.isRefinable) + tacDependees.valuesIterator.filter(_.isRefinable) } } @@ -114,26 +108,9 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { def createState(field: Field): AnalysisState - /** - * Analyzes the field's assignability. - * - * This analysis is only ''soundy'' if the class file does not contain native methods and reflections. - * Fields can be manipulated by any given native method. - * Because the analysis cannot be aware of any given native method, - * they are not considered as well as reflections. - */ - private[analyses] def determineFieldAssignability( - field: Field - ): ProperPropertyComputationResult = { - + private[analyses] def determineFieldAssignability(field: Field): ProperPropertyComputationResult = { implicit val state: AnalysisState = createState(field) - state.fieldAssignability = - if (field.isFinal) - NonAssignable - else - EffectivelyNonAssignable - val thisType = field.classFile.thisType if (!field.isFinal) { if (field.isPublic) { @@ -149,26 +126,106 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { } } - val fwaiEP = propertyStore(declaredFields(field), FieldWriteAccessInformation.key) + state.fieldAssignability = + if (field.isFinal) NonAssignable + else EffectivelyNonAssignable + + handleWriteAccessInformation(propertyStore(declaredFields(field), FieldWriteAccessInformation.key)) + } + + protected def handleWriteAccessInformation( + newEP: EOptionP[DeclaredField, FieldWriteAccessInformation] + )(implicit state: AnalysisState): ProperPropertyComputationResult = { + if (newEP.hasUBP) { + val newFai = newEP.ub + val (seenDirectAccesses, seenIndirectAccesses) = state.fieldWriteAccessDependee match { + case Some(UBP(fai)) => (fai.numDirectAccesses, fai.numIndirectAccesses) + case _ => (0, 0) + } + state.fieldWriteAccessDependee = Some(newEP) + + newFai.getNewestAccesses( + newFai.numDirectAccesses - seenDirectAccesses, + newFai.numIndirectAccesses - seenIndirectAccesses + ) foreach { case (contextID, pc, receiver, _) => + val method = contextProvider.contextFromId(contextID).method.asDefinedMethod + val access = (pc, receiver) + state.fieldAccesses = state.fieldAccesses.updatedWith(method) { + case None => Some(Map((contextID, Set(access)))) + case Some(map) => Some(map + (contextID -> (map.getOrElse(contextID, Set.empty) + access))) + } + } + + state.fieldAccesses.foreachEntry { (method, contextIDToAccesses) => + val tacEP = state.tacDependees.get(method) match { + case Some(tacEP) => tacEP + case None => + val tacEP = propertyStore(method.definedMethod, TACAI.key) + state.tacDependees += method -> tacEP + tacEP + } - if (handleFieldWriteAccessInformation(fwaiEP)) - return Result(field, Assignable); + if (tacEP.hasUBP) { + contextIDToAccesses.foreachEntry { (contextID, accesses) => + val context = contextProvider.contextFromId(contextID) + if (state.fieldAssignability == Assignable || + doesMethodUpdateFieldInContext(context, method, tacEP.ub.tac.get, accesses)) + state.fieldAssignability = Assignable + } + } + } + } else { + state.fieldWriteAccessDependee = Some(newEP) + } createResult() } /** - * Analyzes field writes for a single method, returning false if the field may still be - * effectively final and true otherwise. + * Returns true when the method in the given context definitely updates the field in a way that forces it to be + * assignable, and false when it does not, or we are not yet sure. + * + * @note Callers must pass ALL write accesses of this method in this context discovered so far. */ - def methodUpdatesField( - method: DefinedMethod, - taCode: TACode[TACMethodParameter, V], - callers: Callers, - pc: PC, - receiver: AccessReceiver + protected def doesMethodUpdateFieldInContext( + context: Context, + definedMethod: DefinedMethod, + taCode: TACode[TACMethodParameter, V], + writeAccesses: Iterable[(PC, AccessReceiver)] )(implicit state: AnalysisState): Boolean + def createResult()(implicit state: AnalysisState): ProperPropertyComputationResult = { + if (state.hasDependees && (state.fieldAssignability ne Assignable)) + InterimResult(state.field, lb = Assignable, ub = state.fieldAssignability, state.dependees, continuation) + else + Result(state.field, state.fieldAssignability) + } + + def continuation(eps: SomeEPS)(implicit state: AnalysisState): ProperPropertyComputationResult = { + eps.pk match { + case FieldWriteAccessInformation.key => + handleWriteAccessInformation(eps.asInstanceOf[EOptionP[DeclaredField, FieldWriteAccessInformation]]) + + case EscapeProperty.key => + val newEP = eps.asInstanceOf[EOptionP[(Context, DefinitionSite), EscapeProperty]] + state.escapeDependees = state.escapeDependees.filter(_.e != newEP.e) + if (handleEscapeProperty(newEP)) + Result(state.field, Assignable) + else + createResult() + + case TACAI.key => + val newEP = eps.asInstanceOf[EOptionP[Method, TACAI]] + val method = declaredMethods(newEP.e) + val accessesByContext = state.fieldAccesses(method) + state.tacDependees += method -> newEP + if (accessesByContext.exists(kv => doesMethodUpdateFieldInContext(contextProvider.contextFromId(kv._1), method, newEP.ub.tac.get, kv._2))) + Result(state.field, Assignable) + else + createResult() + } + } + /** * Handles the influence of an escape property on the field immutability. * @@ -209,7 +266,7 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { ref: V, stmts: Array[Stmt[V]], method: DefinedMethod, - callers: Callers + context: Context )(implicit state: AnalysisState): Boolean = { ref.definedBy.forall { defSite => if (defSite < 0) true // Must be locally created @@ -219,123 +276,14 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { if (definition.expr.isNullExpr) false else if (!definition.expr.isNew) true else { - var hasEscaped = false - callers.forNewCalleeContexts(null, method) { context => - val entity = (context, definitionSites(method.definedMethod, definition.pc)) - val escapeProperty = propertyStore(entity, EscapeProperty.key) - hasEscaped ||= handleEscapeProperty(escapeProperty) - } - hasEscaped + val entity = (context, definitionSites(method.definedMethod, definition.pc)) + val escapeProperty = propertyStore(entity, EscapeProperty.key) + handleEscapeProperty(escapeProperty) } } } } - protected[this] def handleFieldWriteAccessInformation( - newEP: EOptionP[DeclaredField, FieldWriteAccessInformation] - )(implicit state: AnalysisState): Boolean = { - val assignable = if (newEP.hasUBP) { - val newFai = newEP.ub - val (seenDirectAccesses, seenIndirectAccesses) = state.fieldWriteAccessDependee match { - case Some(UBP(fai)) => (fai.numDirectAccesses, fai.numIndirectAccesses) - case _ => (0, 0) - } - state.fieldWriteAccessDependee = Some(newEP) - - newFai.getNewestAccesses( - newFai.numDirectAccesses - seenDirectAccesses, - newFai.numIndirectAccesses - seenIndirectAccesses - ) exists { case (contextID, pc, receiver, _) => - val method = contextProvider.contextFromId(contextID).method.asDefinedMethod - state.fieldAccesses += method -> (state.fieldAccesses.getOrElse(method, Set.empty) + - ((pc, receiver))) - - val tacEP = state.tacDependees.get(method) match { - case Some(tacEP) => tacEP - case None => - val tacEP = propertyStore(method.definedMethod, TACAI.key) - state.tacDependees += method -> tacEP - tacEP - } - - val callersEP = state.callerDependees.get(method) match { - case Some(callersEP) => callersEP - case None => - val callersEP = propertyStore(method, Callers.key) - state.callerDependees += method -> callersEP - callersEP - } - - if (tacEP.hasUBP && callersEP.hasUBP) - methodUpdatesField(method, tacEP.ub.tac.get, callersEP.ub, pc, receiver) - else - false - } - } else { - state.fieldWriteAccessDependee = Some(newEP) - false - } - - assignable - } - - /** - * Continuation function handling updates to the FieldPrematurelyRead property or to the purity - * property of the method that initializes a (potentially) lazy initialized field. - */ - def c(eps: SomeEPS)(implicit state: AnalysisState): ProperPropertyComputationResult = { - val isNonFinal = eps.pk match { - case EscapeProperty.key => - val newEP = eps.asInstanceOf[EOptionP[(Context, DefinitionSite), EscapeProperty]] - state.escapeDependees = state.escapeDependees.filter(_.e != newEP.e) - handleEscapeProperty(newEP) - case TACAI.key => - val newEP = eps.asInstanceOf[EOptionP[Method, TACAI]] - val method = declaredMethods(newEP.e) - val accesses = state.fieldAccesses.get(method) - state.tacDependees += method -> newEP - val callersProperty = state.callerDependees(method) - if (callersProperty.hasUBP && accesses.isDefined) - accesses.get.exists(access => - methodUpdatesField(method, newEP.ub.tac.get, callersProperty.ub, access._1, access._2) - ) - else false - case Callers.key => - val newEP = eps.asInstanceOf[EOptionP[DefinedMethod, Callers]] - val method = newEP.e - val accesses = state.fieldAccesses.get(method) - state.callerDependees += newEP.e -> newEP - val tacProperty = state.tacDependees(method) - if (tacProperty.hasUBP && tacProperty.ub.tac.isDefined && accesses.isDefined) - accesses.get.exists(access => - methodUpdatesField(method, tacProperty.ub.tac.get, newEP.ub, access._1, access._2) - ) - else false - case FieldWriteAccessInformation.key => - val newEP = eps.asInstanceOf[EOptionP[DeclaredField, FieldWriteAccessInformation]] - handleFieldWriteAccessInformation(newEP) - } - - if (isNonFinal) - Result(state.field, Assignable) - else - createResult() - } - - def createResult()(implicit state: AnalysisState): ProperPropertyComputationResult = { - if (state.hasDependees && (state.fieldAssignability ne Assignable)) { - InterimResult( - state.field, - Assignable, - state.fieldAssignability, - state.dependees, - c - ) - } else { - Result(state.field, state.fieldAssignability) - } - } - /** * Returns the initialization value of a given type. */ @@ -361,8 +309,7 @@ trait AbstractFieldAssignabilityAnalysisScheduler extends FPCFAnalysisScheduler override def uses: Set[PropertyBounds] = PropertyBounds.ubs( TACAI, EscapeProperty, - FieldWriteAccessInformation, - Callers + FieldWriteAccessInformation ) override def requiredProjectInformation: ProjectInformationKeys = Seq( diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala index 61d16da8ce..6f954ee4f3 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala @@ -12,7 +12,7 @@ import org.opalj.br.analyses.SomeProject import org.opalj.br.fpcf.BasicFPCFEagerAnalysisScheduler import org.opalj.br.fpcf.BasicFPCFLazyAnalysisScheduler import org.opalj.br.fpcf.FPCFAnalysis -import org.opalj.br.fpcf.properties.cg.Callers +import org.opalj.br.fpcf.properties.Context import org.opalj.br.fpcf.properties.fieldaccess.AccessReceiver import org.opalj.br.fpcf.properties.immutability.FieldAssignability import org.opalj.fpcf.PropertyBounds @@ -37,36 +37,37 @@ class L1FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) override def createState(field: Field): AnalysisState = State(field) /** - * Analyzes field writes for a single method, returning false if the field may still be - * effectively final and true otherwise. + * Returns true when the method in the given context definitely updates the field in a way that forces it to be + * assignable, and false when it does not, or we are not yet sure. + * + * @note Callers must pass ALL write accesses of this method in this context discovered so far. */ - def methodUpdatesField( + override def doesMethodUpdateFieldInContext( + context: Context, definedMethod: DefinedMethod, - taCode: TACode[TACMethodParameter, V], - callers: Callers, - pc: PC, - receiver: AccessReceiver + taCode: TACode[TACMethodParameter, V], + writeAccesses: Iterable[(PC, AccessReceiver)], )(implicit state: AnalysisState): Boolean = { - val stmts = taCode.stmts val method = definedMethod.definedMethod + writeAccesses.exists { access => + val receiverVarOpt = access._2.map(uVarForDefSites(_, taCode.pcToIndex)) + if (receiverVarOpt.isDefined) { + val receiverVar = receiverVarOpt.get + // note that here we assume real three address code (flat hierarchy) - if (receiver.isDefined) { - val objRef = uVarForDefSites(receiver.get, taCode.pcToIndex).asVar - // note that here we assume real three address code (flat hierarchy) + // for instance fields it is okay if they are written in the + // constructor (w.r.t. the currently initialized object!) - // for instance fields it is okay if they are written in the - // constructor (w.r.t. the currently initialized object!) + // If the field that is written is not the one referred to by the + // self reference, it is not effectively final. - // If the field that is written is not the one referred to by the - // self reference, it is not effectively final. - - // However, a method (e.g. clone) may instantiate a new object and - // write the field as long as that new object did not yet escape. - (!method.isConstructor || - objRef.definedBy != SelfReferenceParameter) && - referenceHasEscaped(objRef, stmts, definedMethod, callers) - } else { - !method.isStaticInitializer + // However, a method (e.g. clone) may instantiate a new object and + // write the field as long as that new object did not yet escape. + (!method.isConstructor || receiverVar.definedBy != SelfReferenceParameter) && + referenceHasEscaped(receiverVar, taCode.stmts, definedMethod, context) + } else { + !method.isStaticInitializer + } } } } diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 2c871c4cf0..f97d02b324 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -11,7 +11,6 @@ import scala.collection.mutable import org.opalj.RelationalOperators.EQ import org.opalj.RelationalOperators.NE -import org.opalj.ai.isFormalParameter import org.opalj.br.ClassType import org.opalj.br.DeclaredField import org.opalj.br.DefinedMethod @@ -26,7 +25,7 @@ import org.opalj.br.cfg.CFGNode import org.opalj.br.fpcf.BasicFPCFEagerAnalysisScheduler import org.opalj.br.fpcf.BasicFPCFLazyAnalysisScheduler import org.opalj.br.fpcf.FPCFAnalysis -import org.opalj.br.fpcf.properties.cg.Callers +import org.opalj.br.fpcf.properties.Context import org.opalj.br.fpcf.properties.fieldaccess.AccessParameter import org.opalj.br.fpcf.properties.fieldaccess.AccessReceiver import org.opalj.br.fpcf.properties.fieldaccess.FieldReadAccessInformation @@ -66,11 +65,16 @@ import org.opalj.tac.fpcf.analyses.cg.uVarForDefSites /** * Determines the assignability of a field. * + * This analysis relies on the field access information provided by the framework, and can thus only detect field + * accesses provided by the framework. If one needs to include reflective or native accesses, the respective modules + * for the field access information need to be included in the analysis. + * * @note Requires that the 3-address code's expressions are not deeply nested. * @author Tobias Roth * @author Dominik Helm * @author Florian Kübler * @author Michael Eichberg + * @author Maximilian Rüsch */ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) extends AbstractFieldAssignabilityAnalysis @@ -85,7 +89,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) field: Field ) extends AbstractFieldAssignabilityAnalysisState { var checkLazyInit: Option[(Method, Int, Int, TACode[TACMethodParameter, V])] = None - var openWrites = List.empty[(DefinedMethod, TACode[TACMethodParameter, V], Option[V], PC)] + var writesToCheckForReadDominance = List.empty[(DefinedMethod, PC)] var fieldReadAccessDependee: Option[EOptionP[DeclaredField, FieldReadAccessInformation]] = None @@ -95,143 +99,112 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } type AnalysisState = State - override def createState(field: Field): AnalysisState = State(field) + override protected def handleWriteAccessInformation( + newEP: EOptionP[DeclaredField, FieldWriteAccessInformation] + )(implicit state: State): ProperPropertyComputationResult = { + if (state.checkLazyInit.isDefined && hasMultipleNonConstructorWrites(state.checkLazyInit.get._1)) { + state.fieldAssignability = Assignable + createResult() + } else { + super.handleWriteAccessInformation(newEP) + } + } + /** - * Analyzes field writes for a single method, returning false if the field may still be - * effectively non-assignable and true otherwise. + * Returns true when the method in the given context definitely updates the field in a way that forces it to be + * assignable, and false when it does not, or we are not yet sure. + * + * @note Callers must pass ALL write accesses of this method in this context discovered so far. */ - def methodUpdatesField( + override def doesMethodUpdateFieldInContext( + context: Context, definedMethod: DefinedMethod, - taCode: TACode[TACMethodParameter, V], - callers: Callers, - pc: PC, - receiver: AccessReceiver + taCode: TACode[TACMethodParameter, V], + writeAccesses: Iterable[(PC, AccessReceiver)] )(implicit state: AnalysisState): Boolean = { + assert(writeAccesses.nonEmpty) + val field = state.field val method = definedMethod.definedMethod - val writeIndex = taCode.pcToIndex(pc) - val receiverVarOpt = receiver.map(uVarForDefSites(_, taCode.pcToIndex)) + + if (writeAccesses.size > 1) { + // There can be multiple assignments of final fields in the constructor in different branches, + // but otherwise if a field is written in multiple locations it must be assignable. + //if (!method.isConstructor) TODO handle multi-branches with domination + return true; + } if (field.isStatic && method.isConstructor) { // A static field updated in an arbitrary constructor may be updated with (at least) the first call. // Thus, we may see its initial value or the updated value, making the field assignable. - true - } else if (method.isInitializer && method.classFile == field.classFile) { - receiverVarOpt.isDefined && receiverVarOpt.get.definedBy != SelfReferenceParameter || - checkWriteDominance(definedMethod, taCode, receiverVarOpt, writeIndex) - } else { - if (field.isStatic || receiverVarOpt.isDefined && receiverVarOpt.get.definedBy == SelfReferenceParameter) { - // A field written outside an initializer must be lazily initialized or it is assignable - if (considerLazyInitialization) { - state.fieldAssignability = - determineLazyInitialization(writeIndex, getDefaultValues(), method, taCode) - state.fieldAssignability eq Assignable - } else - true - } else if (receiverVarOpt.isDefined && - referenceHasEscaped(receiverVarOpt.get, taCode.stmts, definedMethod, callers) - ) { - // Here the clone pattern is determined among others - // note that here we assume real three address code (flat hierarchy) - - // for instance fields it is okay if they are written in the - // constructor (w.r.t. the currently initialized object!) - - // If the field that is written is not the one referred to by the - // self reference, it is not effectively non-assignable. - - // However, a method (e.g. clone) may instantiate a new object and - // write the field as long as that new object did not yet escape. - true - } else { - checkWriteDominance(definedMethod, taCode, receiverVarOpt, writeIndex) - } + return true; } - } - - private def checkWriteDominance( - definedMethod: DefinedMethod, - taCode: TACode[TACMethodParameter, V], - receiverVar: Option[V], - writeIndex: Int - )(implicit state: State): Boolean = { - val stmts = taCode.stmts - val writes = state.fieldWriteAccessDependee.get.ub.accesses - val writesInMethod = writes.filter { w => contextProvider.contextFromId(w._1).method eq definedMethod }.toSeq - - if (writesInMethod.distinctBy(_._2).size > 1) { - // There can be multiple assignments of final fields in the constructor in different branches, - // but otherwise if field is written in multiple locations it must be assignable. - if (!definedMethod.definedMethod.isConstructor) - return true; - } + val (pc, persistentReceiverOpt) = writeAccesses.head + val writeIndex = taCode.pcToIndex(pc) + val receiverVarOpt = persistentReceiverOpt.map(uVarForDefSites(_, taCode.pcToIndex)) // If we have no information about the receiver, but we should have it, soundly return true. - if (receiverVar.isEmpty && !state.field.isStatic) + if (state.field.isNotStatic && receiverVarOpt.isEmpty) return true; - val assignedValueObject = - if (writeIndex > 0 && stmts(writeIndex).isPutStatic) { - stmts(writeIndex).asPutStatic.value.asVar - } else if (writeIndex > 0 && stmts( - writeIndex - ).isPutField && stmts(writeIndex).asPutField.value.asVar.value.isArrayValue.isYes - ) { - stmts(writeIndex).asPutField.value.asVar - } else - receiverVar.get - - if (assignedValueObject.definedBy.size != 1) - return true; - - val defSite = assignedValueObject.definedBy.head - if (defSite < -1 || (defSite == -1 && !definedMethod.definedMethod.isConstructor)) + if (state.field.isNotStatic && + method.isInitializer && method.classFile == field.classFile && + receiverVarOpt.get.definedBy != SelfReferenceParameter) { + // An instance field that is modified in an initializer of a different class must be assignable return true; + } - val uses = if (defSite == -1) - taCode.params.thisParameter.useSites - else { - val assignedValueObjectVar = stmts(defSite).asAssignment.targetVar.asVar - if (assignedValueObjectVar != null) - assignedValueObjectVar.usedBy - else IntTrieSet.empty + if (field.isStatic || receiverVarOpt.isDefined && receiverVarOpt.get.definedBy == SelfReferenceParameter) { + // A field written outside an initializer must be lazily initialized or it is assignable + if (considerLazyInitialization) { + state.fieldAssignability = determineLazyInitialization(writeIndex, getDefaultValues(), method, taCode) + return state.fieldAssignability eq Assignable; + } else + return true; } - val fieldWriteInMethodIndex = taCode.pcToIndex(writesInMethod.head._2) - if (!uses.forall { index => - val stmt = stmts(index) + if (receiverVarOpt.isDefined && referenceHasEscaped(receiverVarOpt.get, taCode.stmts, definedMethod, context)) { + // Arbitrary methods may instantiate new objects and write instance fields, as long as the new object did + // not yet escape. This effectively determines usage of the `clone` pattern. If the reference has escaped, + // we need to assume that someone observed the old field value before modification, thus soundly return. + return true; + } - fieldWriteInMethodIndex == index || // The value is itself written to another object - // IMPROVE: Can we use field access information to care about reflective accesses here? - stmt.isPutField && stmt.asPutField.name != state.field.name || - stmt.isMethodCall && stmt.asMethodCall.name == "" || - // CHECK do we really need the taCode here? - !dominates(index, fieldWriteInMethodIndex, taCode) || stmt.isArrayStore // TODO check + //if (method.isInitializer && method.classFile == field.classFile) { + // checkWriteDominance(definedMethod, taCode, receiverVarOpt, writeIndex) + //} - } - ) - return true; + !doesWriteDominateAllReads(definedMethod, taCode, receiverVarOpt, writeIndex) + } + /** + * @return Whether the given write dominates all reads. Soundly returns true if this cannot be determined yet. + */ + private def doesWriteDominateAllReads( + definedMethod: DefinedMethod, + taCode: TACode[TACMethodParameter, V], + receiverVar: Option[V], + writeIndex: Int + )(implicit state: State): Boolean = { if (state.fieldReadAccessDependee.isEmpty) { state.fieldReadAccessDependee = Some(propertyStore(declaredFields(state.field), FieldReadAccessInformation.key)) } val fraiEP = state.fieldReadAccessDependee.get + val writeAccess = (definedMethod, writeIndex) + if (fraiEP.hasUBP && fieldReadsNotDominated(fraiEP.ub.accesses, Seq(writeAccess))) + return false; - val writeAccess = (definedMethod, taCode, receiverVar, writeIndex) - if (fraiEP.hasUBP && fieldReadsNotDominated(fraiEP.ub, 0, 0, Seq(writeAccess))) - return true; - - state.openWrites ::= writeAccess + state.writesToCheckForReadDominance ::= writeAccess - false + true } - override def c(eps: SomeEPS)(implicit state: State): ProperPropertyComputationResult = { + override def continuation(eps: SomeEPS)(implicit state: State): ProperPropertyComputationResult = { eps.pk match { case FieldReadAccessInformation.key => val newEP = eps.asInstanceOf[EOptionP[DeclaredField, FieldReadAccessInformation]] @@ -240,23 +213,14 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) case Some(UBP(fai)) => (fai.numDirectAccesses, fai.numIndirectAccesses) case _ => (0, 0) } + val newestReads = reads.getNewestAccesses(seenDirectAccesses, seenIndirectAccesses) - if (fieldReadsNotDominated(reads, seenDirectAccesses, seenIndirectAccesses, state.openWrites)) + if (fieldReadsNotDominated(newestReads, state.writesToCheckForReadDominance)) return Result(state.field, Assignable); if (state.checkLazyInit.isDefined) { val (method, guardIndex, writeIndex, taCode) = state.checkLazyInit.get - if (doFieldReadsEscape( - reads.getNewestAccesses( - reads.numDirectAccesses - seenDirectAccesses, - reads.numIndirectAccesses - seenIndirectAccesses - ).toSeq, - method, - guardIndex, - writeIndex, - taCode - ) - ) + if (doFieldReadsEscape(newestReads.toSeq, method, guardIndex, writeIndex, taCode)) return Result(state.field, Assignable); } @@ -264,46 +228,27 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) createResult() case _ => - super.c(eps) + super.continuation(eps) } } - override protected[this] def handleFieldWriteAccessInformation( - newEP: EOptionP[DeclaredField, FieldWriteAccessInformation] - )(implicit state: State): Boolean = { - val openWrites = state.openWrites - state.openWrites = List.empty - - state.checkLazyInit.isDefined && hasMultipleNonConstructorWrites(state.checkLazyInit.get._1) || - super.handleFieldWriteAccessInformation(newEP) || - openWrites.exists { case (method, tac, receiver, index) => - checkWriteDominance(method, tac, receiver, index) - } - } - + /** + * @return Whether there is at least one given read such that there is at least one given write in the same method + * that does not dominate the read. + */ private def fieldReadsNotDominated( - fieldReadAccessInformation: FieldReadAccessInformation, - seenDirectAccesses: Int, - seenIndirectAccesses: Int, - writes: Seq[(DefinedMethod, TACode[TACMethodParameter, V], Option[V], Int)] + reads: IterableOnce[(Int, PC, AccessReceiver, AccessParameter)], + writes: Seq[(DefinedMethod, Int)] )(implicit state: State): Boolean = { - writes.exists { case (writeMethod, _, _, writeIndex) => - fieldReadAccessInformation.getNewestAccesses( - fieldReadAccessInformation.numDirectAccesses - seenDirectAccesses, - fieldReadAccessInformation.numIndirectAccesses - seenIndirectAccesses - ).exists { case (readContextID, readPC, readReceiver, _) => - val method = contextProvider.contextFromId(readContextID).method - method.definedMethod.classFile != state.field.classFile || - (writeMethod eq method) && { - val taCode = state.tacDependees(method.asDefinedMethod).ub.tac.get - - if (readReceiver.isDefined && readReceiver.get._2.forall(isFormalParameter)) { - false - } else { - !dominates(writeIndex, taCode.pcToIndex(readPC), taCode) - } + reads.iterator.exists { + case (readContextID, readPC, readReceiver, _) => + val readMethod = contextProvider.contextFromId(readContextID).method + writes.exists { case (writeMethod, writeIndex) => + (readMethod eq writeMethod) && { + val taCode = state.tacDependees(readMethod.asDefinedMethod).ub.tac.get + !dominates(writeIndex, taCode.pcToIndex(readPC), taCode) } - } + } } } @@ -999,7 +944,6 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } } } - } trait L2FieldAssignabilityAnalysisScheduler extends AbstractFieldAssignabilityAnalysisScheduler { From 9126fed89e1ece8180f406e80f710068325f943d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Fri, 3 Oct 2025 11:47:53 +0100 Subject: [PATCH 22/33] Shuffle around logic --- .../AbstractFieldAssignabilityAnalysis.scala | 4 +- .../L2FieldAssignabilityAnalysis.scala | 56 +++++++++---------- 2 files changed, 27 insertions(+), 33 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index 52af6057c0..5a45859466 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -227,7 +227,7 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { } /** - * Handles the influence of an escape property on the field immutability. + * Handles the influence of an escape property on the field assignability. * * @return true if the object - on which a field write occurred - escapes, false otherwise. * @note (Re-)Adds dependees as necessary. @@ -260,7 +260,7 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { } /** - * Checks whether the object reference of a PutField does not escape (except for being returned). + * Checks whether the object reference of a field access does not escape (except for being returned). */ def referenceHasEscaped( ref: V, diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index f97d02b324..e314318778 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -129,19 +129,17 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) val field = state.field val method = definedMethod.definedMethod - if (writeAccesses.size > 1) { - // There can be multiple assignments of final fields in the constructor in different branches, - // but otherwise if a field is written in multiple locations it must be assignable. - //if (!method.isConstructor) TODO handle multi-branches with domination - return true; - } - if (field.isStatic && method.isConstructor) { // A static field updated in an arbitrary constructor may be updated with (at least) the first call. // Thus, we may see its initial value or the updated value, making the field assignable. return true; } + if (writeAccesses.size > 1) { + // TODO handle multi-branches with domination and making sure its the same variable (this is difficult across multiple variables, may need to fall back soundly) + return true; + } + val (pc, persistentReceiverOpt) = writeAccesses.head val writeIndex = taCode.pcToIndex(pc) val receiverVarOpt = persistentReceiverOpt.map(uVarForDefSites(_, taCode.pcToIndex)) @@ -150,33 +148,29 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) if (state.field.isNotStatic && receiverVarOpt.isEmpty) return true; - if (state.field.isNotStatic && - method.isInitializer && method.classFile == field.classFile && - receiverVarOpt.get.definedBy != SelfReferenceParameter) { - // An instance field that is modified in an initializer of a different class must be assignable - return true; - } - - if (field.isStatic || receiverVarOpt.isDefined && receiverVarOpt.get.definedBy == SelfReferenceParameter) { - // A field written outside an initializer must be lazily initialized or it is assignable - if (considerLazyInitialization) { - state.fieldAssignability = determineLazyInitialization(writeIndex, getDefaultValues(), method, taCode) - return state.fieldAssignability eq Assignable; - } else + if (method.isInitializer && method.classFile == field.classFile) { + if (state.field.isNotStatic && receiverVarOpt.get.definedBy != SelfReferenceParameter) { + // An instance field that is modified in an initializer of a different class must be assignable return true; - } + } + } else { + if (field.isStatic || receiverVarOpt.isDefined && receiverVarOpt.get.definedBy == SelfReferenceParameter) { + // A field written outside an initializer must be lazily initialized or it is assignable + if (considerLazyInitialization) { + state.fieldAssignability = determineLazyInitialization(writeIndex, getDefaultValues(), method, taCode) + return state.fieldAssignability eq Assignable; + } else + return true; + } - if (receiverVarOpt.isDefined && referenceHasEscaped(receiverVarOpt.get, taCode.stmts, definedMethod, context)) { - // Arbitrary methods may instantiate new objects and write instance fields, as long as the new object did - // not yet escape. This effectively determines usage of the `clone` pattern. If the reference has escaped, - // we need to assume that someone observed the old field value before modification, thus soundly return. - return true; + if (receiverVarOpt.isDefined && referenceHasEscaped(receiverVarOpt.get, taCode.stmts, definedMethod, context)) { + // Arbitrary methods may instantiate new objects and write instance fields, as long as the new object did + // not yet escape. This effectively determines usage of the `clone` pattern. If the reference has escaped, + // we need to assume that someone observed the old field value before modification, thus soundly return. + return true; + } } - //if (method.isInitializer && method.classFile == field.classFile) { - // checkWriteDominance(definedMethod, taCode, receiverVarOpt, writeIndex) - //} - !doesWriteDominateAllReads(definedMethod, taCode, receiverVarOpt, writeIndex) } @@ -245,7 +239,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) val readMethod = contextProvider.contextFromId(readContextID).method writes.exists { case (writeMethod, writeIndex) => (readMethod eq writeMethod) && { - val taCode = state.tacDependees(readMethod.asDefinedMethod).ub.tac.get + val taCode = state.tacDependees(readMethod.asDefinedMethod).ub.tac.get // TODO check that the receiver is the same (what about static fields?) !dominates(writeIndex, taCode.pcToIndex(readPC), taCode) } } From 79a8ad306f4929520595c7907a0570b98b1ec7b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Fri, 3 Oct 2025 11:48:00 +0100 Subject: [PATCH 23/33] Fix typos --- .../assignability/clone_function/SimpleClonePattern.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/clone_function/SimpleClonePattern.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/clone_function/SimpleClonePattern.java index cf68de1262..b14f671315 100644 --- a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/clone_function/SimpleClonePattern.java +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/assignability/clone_function/SimpleClonePattern.java @@ -15,7 +15,7 @@ public final class SimpleClonePattern { @TransitivelyImmutableField("Field is effectively non assignable and has a primitive type") - @EffectivelyNonAssignableField("Field is only assigned ones due to the clone function pattern") + @EffectivelyNonAssignableField("Field is only assigned once due to the clone function pattern") private int i; public SimpleClonePattern clone(){ @@ -28,7 +28,7 @@ public SimpleClonePattern clone(){ class CloneNonAssignableWithNewObject { @TransitivelyImmutableField("field is effectively non assignable and assigned with a transitively immutable object") - @EffectivelyNonAssignableField("field is only assigned ones due to the clone function pattern") + @EffectivelyNonAssignableField("field is only assigned once due to the clone function pattern") private Integer integer; public CloneNonAssignableWithNewObject clone(){ @@ -41,7 +41,7 @@ public CloneNonAssignableWithNewObject clone(){ class EscapesAfterAssignment { @TransitivelyImmutableField("field is effectively non assignable and assigned with a transitively immutable object") - @EffectivelyNonAssignableField("field is only assigned ones due to the clone function pattern") + @EffectivelyNonAssignableField("field is only assigned once due to the clone function pattern") private Integer integer; private Integer integerCopy; @@ -77,7 +77,7 @@ public MultipleFieldsAssignedInCloneFunction clone(){ class ConstructorWithParameter { @TransitivelyImmutableField("field is effectively non assignable and has a transitively immutable type") - @EffectivelyNonAssignableField("field is only assigned ones due to the clone function pattern") + @EffectivelyNonAssignableField("field is only assigned once due to the clone function pattern") private Integer integer; public ConstructorWithParameter(Integer integer){ From 518a1ce2ab1a4eb0d8f3095f6323369c77bb8628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Fri, 3 Oct 2025 12:26:13 +0100 Subject: [PATCH 24/33] Fix logic in meeting field assignability --- .../fpcf/properties/immutability/FieldAssignability.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/OPAL/br/src/main/scala/org/opalj/br/fpcf/properties/immutability/FieldAssignability.scala b/OPAL/br/src/main/scala/org/opalj/br/fpcf/properties/immutability/FieldAssignability.scala index 53f61713a3..f942abfec4 100644 --- a/OPAL/br/src/main/scala/org/opalj/br/fpcf/properties/immutability/FieldAssignability.scala +++ b/OPAL/br/src/main/scala/org/opalj/br/fpcf/properties/immutability/FieldAssignability.scala @@ -34,6 +34,8 @@ sealed trait FieldAssignability extends OrderedProperty with FieldAssignabilityP final def key: PropertyKey[FieldAssignability] = FieldAssignability.key def isImmutable = false + + def meet(other: FieldAssignability): FieldAssignability } object FieldAssignability extends FieldAssignabilityPropertyMetaInformation { @@ -73,9 +75,9 @@ case object EffectivelyNonAssignable extends NonAssignableField { def meet(other: FieldAssignability): FieldAssignability = if (other == NonAssignable) { - other - } else { this + } else { + other } } From bc49ab53fbc137d68fb830c2f9391dc61f02f89f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Fri, 3 Oct 2025 12:49:30 +0100 Subject: [PATCH 25/33] More shuffling --- .../DifferentLazyInitializedFieldTypes.java | 4 +- .../AbstractFieldAssignabilityAnalysis.scala | 34 ++++++----- .../L1FieldAssignabilityAnalysis.scala | 52 +++++++++++----- .../L2FieldAssignabilityAnalysis.scala | 59 ++++++++++++------- 4 files changed, 96 insertions(+), 53 deletions(-) diff --git a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/lazyinitialization/objects/DifferentLazyInitializedFieldTypes.java b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/lazyinitialization/objects/DifferentLazyInitializedFieldTypes.java index c3aa38cbfb..fa2f24d728 100644 --- a/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/lazyinitialization/objects/DifferentLazyInitializedFieldTypes.java +++ b/DEVELOPING_OPAL/validate/src/test/java/org/opalj/fpcf/fixtures/immutability/openworld/lazyinitialization/objects/DifferentLazyInitializedFieldTypes.java @@ -16,8 +16,8 @@ */ public class DifferentLazyInitializedFieldTypes { - @TransitivelyImmutableField("Lazy initialized field with primitive type.") - @LazilyInitializedField("field is thread safely lazy initialized") + @TransitivelyImmutableField("Lazy initialized field with primitive type.") + @LazilyInitializedField("field is thread safely lazy initialized") private int inTheGetterLazyInitializedIntField; public synchronized int getInTheGetterLazyInitializedIntField(){ diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index 5a45859466..4574c3573b 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -156,7 +156,7 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { } } - state.fieldAccesses.foreachEntry { (method, contextIDToAccesses) => + state.fieldAccesses.foreachEntry { (method, accessesByContext) => val tacEP = state.tacDependees.get(method) match { case Some(tacEP) => tacEP case None => @@ -166,11 +166,13 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { } if (tacEP.hasUBP) { - contextIDToAccesses.foreachEntry { (contextID, accesses) => + accessesByContext.foreachEntry { (contextID, accesses) => val context = contextProvider.contextFromId(contextID) - if (state.fieldAssignability == Assignable || - doesMethodUpdateFieldInContext(context, method, tacEP.ub.tac.get, accesses)) - state.fieldAssignability = Assignable + if (state.fieldAssignability != Assignable) { + state.fieldAssignability = state.fieldAssignability.meet { + determineAssignabilityFromWritesInContext(context, method, tacEP.ub.tac.get, accesses) + } + } } } } @@ -187,12 +189,12 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { * * @note Callers must pass ALL write accesses of this method in this context discovered so far. */ - protected def doesMethodUpdateFieldInContext( + protected def determineAssignabilityFromWritesInContext( context: Context, definedMethod: DefinedMethod, taCode: TACode[TACMethodParameter, V], writeAccesses: Iterable[(PC, AccessReceiver)] - )(implicit state: AnalysisState): Boolean + )(implicit state: AnalysisState): FieldAssignability def createResult()(implicit state: AnalysisState): ProperPropertyComputationResult = { if (state.hasDependees && (state.fieldAssignability ne Assignable)) @@ -210,19 +212,23 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { val newEP = eps.asInstanceOf[EOptionP[(Context, DefinitionSite), EscapeProperty]] state.escapeDependees = state.escapeDependees.filter(_.e != newEP.e) if (handleEscapeProperty(newEP)) - Result(state.field, Assignable) - else - createResult() + state.fieldAssignability = Assignable + createResult() case TACAI.key => val newEP = eps.asInstanceOf[EOptionP[Method, TACAI]] val method = declaredMethods(newEP.e) val accessesByContext = state.fieldAccesses(method) state.tacDependees += method -> newEP - if (accessesByContext.exists(kv => doesMethodUpdateFieldInContext(contextProvider.contextFromId(kv._1), method, newEP.ub.tac.get, kv._2))) - Result(state.field, Assignable) - else - createResult() + accessesByContext.foreachEntry((contextId, accesses) => { + val context = contextProvider.contextFromId(contextId) + if (state.fieldAssignability != Assignable) { + state.fieldAssignability = state.fieldAssignability.meet { + determineAssignabilityFromWritesInContext(context, method, newEP.ub.tac.get, accesses) + } + } + }) + createResult() } } diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala index 6f954ee4f3..e03a135647 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala @@ -14,7 +14,9 @@ import org.opalj.br.fpcf.BasicFPCFLazyAnalysisScheduler import org.opalj.br.fpcf.FPCFAnalysis import org.opalj.br.fpcf.properties.Context import org.opalj.br.fpcf.properties.fieldaccess.AccessReceiver +import org.opalj.br.fpcf.properties.immutability.Assignable import org.opalj.br.fpcf.properties.immutability.FieldAssignability +import org.opalj.br.fpcf.properties.immutability.NonAssignable import org.opalj.fpcf.PropertyBounds import org.opalj.fpcf.PropertyStore import org.opalj.tac.fpcf.analyses.cg.uVarForDefSites @@ -42,33 +44,51 @@ class L1FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * * @note Callers must pass ALL write accesses of this method in this context discovered so far. */ - override def doesMethodUpdateFieldInContext( + override def determineAssignabilityFromWritesInContext( context: Context, definedMethod: DefinedMethod, taCode: TACode[TACMethodParameter, V], writeAccesses: Iterable[(PC, AccessReceiver)], - )(implicit state: AnalysisState): Boolean = { + )(implicit state: AnalysisState): FieldAssignability = { + assert(writeAccesses.nonEmpty) + + val field = state.field val method = definedMethod.definedMethod - writeAccesses.exists { access => - val receiverVarOpt = access._2.map(uVarForDefSites(_, taCode.pcToIndex)) - if (receiverVarOpt.isDefined) { - val receiverVar = receiverVarOpt.get - // note that here we assume real three address code (flat hierarchy) - // for instance fields it is okay if they are written in the - // constructor (w.r.t. the currently initialized object!) + if (field.isStatic && method.isConstructor) { + // A static field updated in an arbitrary constructor may be updated with (at least) the first call. + // Thus, we may see its initial value or the updated value, making the field assignable. + return Assignable; + } - // If the field that is written is not the one referred to by the - // self reference, it is not effectively final. + if (writeAccesses.size > 1) { + // Multi-branch access detection is not available on this level. + return Assignable; + } - // However, a method (e.g. clone) may instantiate a new object and - // write the field as long as that new object did not yet escape. - (!method.isConstructor || receiverVar.definedBy != SelfReferenceParameter) && - referenceHasEscaped(receiverVar, taCode.stmts, definedMethod, context) + val assignabilities: Iterable[FieldAssignability] = writeAccesses.map { access => + val receiverVarOpt = access._2.map(uVarForDefSites(_, taCode.pcToIndex)) + if (receiverVarOpt.isDefined) { + val receiverVar = receiverVarOpt.get + if (method.isConstructor && receiverVar.definedBy == SelfReferenceParameter) { + // for instance fields it is okay if they are written in the + // constructor (w.r.t. the currently initialized object!) + NonAssignable + } else if (!referenceHasEscaped(receiverVar, taCode.stmts, definedMethod, context)) { + // A method (e.g. clone) may instantiate a new object and write the field as long as that new object + // did not yet escape. + NonAssignable + } else { + Assignable + } + } else if (!method.isStaticInitializer) { + Assignable } else { - !method.isStaticInitializer + NonAssignable } } + + assignabilities.reduce(_.meet(_)) } } diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index e314318778..a8220aaf83 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -33,6 +33,7 @@ import org.opalj.br.fpcf.properties.fieldaccess.FieldWriteAccessInformation import org.opalj.br.fpcf.properties.immutability.Assignable import org.opalj.br.fpcf.properties.immutability.FieldAssignability import org.opalj.br.fpcf.properties.immutability.LazilyInitialized +import org.opalj.br.fpcf.properties.immutability.NonAssignable import org.opalj.br.fpcf.properties.immutability.UnsafelyLazilyInitialized import org.opalj.collection.immutable.IntTrieSet import org.opalj.fpcf.EOptionP @@ -118,60 +119,76 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * * @note Callers must pass ALL write accesses of this method in this context discovered so far. */ - override def doesMethodUpdateFieldInContext( - context: Context, + override def determineAssignabilityFromWritesInContext( + context: Context, definedMethod: DefinedMethod, - taCode: TACode[TACMethodParameter, V], + taCode: TACode[TACMethodParameter, V], writeAccesses: Iterable[(PC, AccessReceiver)] - )(implicit state: AnalysisState): Boolean = { + )(implicit state: AnalysisState): FieldAssignability = { assert(writeAccesses.nonEmpty) val field = state.field - val method = definedMethod.definedMethod - - if (field.isStatic && method.isConstructor) { + if (field.isStatic && definedMethod.definedMethod.isConstructor) { // A static field updated in an arbitrary constructor may be updated with (at least) the first call. // Thus, we may see its initial value or the updated value, making the field assignable. - return true; + return Assignable; } if (writeAccesses.size > 1) { // TODO handle multi-branches with domination and making sure its the same variable (this is difficult across multiple variables, may need to fall back soundly) - return true; + return Assignable; } - val (pc, persistentReceiverOpt) = writeAccesses.head + val assignabilities = writeAccesses.map { access => { + determineAssignabilityFromAccess(context, definedMethod, taCode)(access._1, access._2) + }} + assignabilities.reduce(_.meet(_)) + } + + private def determineAssignabilityFromAccess( + context: Context, + definedMethod: DefinedMethod, + taCode: TACode[TACMethodParameter, V] + )( + pc: PC, + persistentReceiverOpt: AccessReceiver + )(implicit state: State): FieldAssignability = { + val field = state.field + val method = definedMethod.definedMethod val writeIndex = taCode.pcToIndex(pc) val receiverVarOpt = persistentReceiverOpt.map(uVarForDefSites(_, taCode.pcToIndex)) // If we have no information about the receiver, but we should have it, soundly return true. if (state.field.isNotStatic && receiverVarOpt.isEmpty) - return true; + return Assignable; if (method.isInitializer && method.classFile == field.classFile) { if (state.field.isNotStatic && receiverVarOpt.get.definedBy != SelfReferenceParameter) { // An instance field that is modified in an initializer of a different class must be assignable - return true; + return Assignable; } } else { - if (field.isStatic || receiverVarOpt.isDefined && receiverVarOpt.get.definedBy == SelfReferenceParameter) { + if (field.isStatic || + receiverVarOpt.isDefined && receiverVarOpt.get.definedBy == SelfReferenceParameter) { // A field written outside an initializer must be lazily initialized or it is assignable if (considerLazyInitialization) { - state.fieldAssignability = determineLazyInitialization(writeIndex, getDefaultValues(), method, taCode) - return state.fieldAssignability eq Assignable; + return determineLazyInitialization(writeIndex, getDefaultValues(), method, taCode); } else - return true; - } - - if (receiverVarOpt.isDefined && referenceHasEscaped(receiverVarOpt.get, taCode.stmts, definedMethod, context)) { + return Assignable; + } else if (receiverVarOpt.isDefined && + referenceHasEscaped(receiverVarOpt.get, taCode.stmts, definedMethod, context)) { // Arbitrary methods may instantiate new objects and write instance fields, as long as the new object did // not yet escape. This effectively determines usage of the `clone` pattern. If the reference has escaped, // we need to assume that someone observed the old field value before modification, thus soundly return. - return true; + return Assignable; } } - !doesWriteDominateAllReads(definedMethod, taCode, receiverVarOpt, writeIndex) + if (!doesWriteDominateAllReads(definedMethod, taCode, receiverVarOpt, writeIndex)) { + Assignable + } else { + NonAssignable + } } /** From c5c2f5acd67102759ffb43ddea2ca858dc177ca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Fri, 3 Oct 2025 13:15:33 +0100 Subject: [PATCH 26/33] Fix receiver var of field domination --- .../L2FieldAssignabilityAnalysis.scala | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index a8220aaf83..29d71efa81 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -90,7 +90,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) field: Field ) extends AbstractFieldAssignabilityAnalysisState { var checkLazyInit: Option[(Method, Int, Int, TACode[TACMethodParameter, V])] = None - var writesToCheckForReadDominance = List.empty[(DefinedMethod, PC)] + var writesToCheckForReadDominance = List.empty[(DefinedMethod, PC, Option[V])] var fieldReadAccessDependee: Option[EOptionP[DeclaredField, FieldReadAccessInformation]] = None @@ -206,11 +206,12 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } val fraiEP = state.fieldReadAccessDependee.get - val writeAccess = (definedMethod, writeIndex) + val writeAccess = (definedMethod, writeIndex, receiverVar) if (fraiEP.hasUBP && fieldReadsNotDominated(fraiEP.ub.accesses, Seq(writeAccess))) return false; - state.writesToCheckForReadDominance ::= writeAccess + if (fraiEP.isRefinable) + state.writesToCheckForReadDominance ::= writeAccess true } @@ -249,15 +250,26 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) */ private def fieldReadsNotDominated( reads: IterableOnce[(Int, PC, AccessReceiver, AccessParameter)], - writes: Seq[(DefinedMethod, Int)] + writes: Seq[(DefinedMethod, Int, Option[V])] )(implicit state: State): Boolean = { reads.iterator.exists { case (readContextID, readPC, readReceiver, _) => val readMethod = contextProvider.contextFromId(readContextID).method - writes.exists { case (writeMethod, writeIndex) => + writes.exists { case (writeMethod, writeIndex, writeReceiverVarOpt) => (readMethod eq writeMethod) && { - val taCode = state.tacDependees(readMethod.asDefinedMethod).ub.tac.get // TODO check that the receiver is the same (what about static fields?) - !dominates(writeIndex, taCode.pcToIndex(readPC), taCode) + val taCode = state.tacDependees(readMethod.asDefinedMethod).ub.tac.get + val readReceiverVarOpt = readReceiver.map(uVarForDefSites(_, taCode.pcToIndex)) + // A field read needs to be dominated if the field is static ... + if (state.field.isStatic || + // ... OR we can infer no information about the read receiver ... + readReceiverVarOpt.isEmpty || + // ... OR the read receiver is the write receiver, i.e. they have overlapping def sites + // IMPROVE this does not consider obfuscating reassignments, consider using PDUWebs + readReceiverVarOpt.get.definedBy.intersect(writeReceiverVarOpt.get.definedBy).nonEmpty + ) { + !dominates(writeIndex, taCode.pcToIndex(readPC), taCode) + } else + false } } } From 891e92ad26c5df135d0033eea09dad5d8cb33c92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Fri, 3 Oct 2025 13:53:29 +0100 Subject: [PATCH 27/33] Handle domination of multiple branches --- .../L2FieldAssignabilityAnalysis.scala | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 29d71efa81..ed521e0cb6 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -128,14 +128,21 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) assert(writeAccesses.nonEmpty) val field = state.field - if (field.isStatic && definedMethod.definedMethod.isConstructor) { + val method = definedMethod.definedMethod + if (field.isStatic && method.isConstructor) { // A static field updated in an arbitrary constructor may be updated with (at least) the first call. // Thus, we may see its initial value or the updated value, making the field assignable. return Assignable; } - if (writeAccesses.size > 1) { - // TODO handle multi-branches with domination and making sure its the same variable (this is difficult across multiple variables, may need to fall back soundly) + if (writeAccesses.exists { access1 => writeAccesses.exists { access2 => + access1._1 != access2._1 && + dominates(taCode.pcToIndex(access1._1), taCode.pcToIndex(access2._1), taCode) + }}) { + // When one write is detected to dominate another, the field is definitively assigned multiple times + // and cannot be effectively non-assignable, even in initializers. + // IMPROVE reduce this to modifications on the same instance and consider cases where not every path + // contains multiple writes to be more sound. return Assignable; } From d2770360115524cff4ec4e52eaa59a360ef0d028 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Fri, 3 Oct 2025 15:47:08 +0100 Subject: [PATCH 28/33] Cleanup --- .../fieldassignability/L2FieldAssignabilityAnalysis.scala | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index ed521e0cb6..9078e36fbf 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -139,8 +139,8 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) access1._1 != access2._1 && dominates(taCode.pcToIndex(access1._1), taCode.pcToIndex(access2._1), taCode) }}) { - // When one write is detected to dominate another, the field is definitively assigned multiple times - // and cannot be effectively non-assignable, even in initializers. + // When one write is detected to dominate another within the same method, the field is definitively assigned + // multiple times and cannot be effectively non-assignable, even in initializers. // IMPROVE reduce this to modifications on the same instance and consider cases where not every path // contains multiple writes to be more sound. return Assignable; @@ -191,7 +191,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } } - if (!doesWriteDominateAllReads(definedMethod, taCode, receiverVarOpt, writeIndex)) { + if (!doesWriteDominateAllReads(definedMethod, receiverVarOpt, writeIndex)) { Assignable } else { NonAssignable @@ -203,7 +203,6 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) */ private def doesWriteDominateAllReads( definedMethod: DefinedMethod, - taCode: TACode[TACMethodParameter, V], receiverVar: Option[V], writeIndex: Int )(implicit state: State): Boolean = { From 3873fe0506606804fd2902557140d47bf1b98e25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Fri, 3 Oct 2025 16:33:00 +0100 Subject: [PATCH 29/33] Analyze suspicious usages again --- .../L2FieldAssignabilityAnalysis.scala | 51 +++++++++++++++++-- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 9078e36fbf..2e8d69b08b 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -147,7 +147,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } val assignabilities = writeAccesses.map { access => { - determineAssignabilityFromAccess(context, definedMethod, taCode)(access._1, access._2) + determineAssignabilityFromAccess(context, definedMethod, taCode)(taCode.pcToIndex(access._1), access._2) }} assignabilities.reduce(_.meet(_)) } @@ -157,12 +157,11 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) definedMethod: DefinedMethod, taCode: TACode[TACMethodParameter, V] )( - pc: PC, + writeIndex: PC, persistentReceiverOpt: AccessReceiver )(implicit state: State): FieldAssignability = { val field = state.field val method = definedMethod.definedMethod - val writeIndex = taCode.pcToIndex(pc) val receiverVarOpt = persistentReceiverOpt.map(uVarForDefSites(_, taCode.pcToIndex)) // If we have no information about the receiver, but we should have it, soundly return true. @@ -191,13 +190,57 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } } - if (!doesWriteDominateAllReads(definedMethod, receiverVarOpt, writeIndex)) { + if ((state.field.isNotStatic && + isInstanceUsedSuspiciously(definedMethod, taCode, writeIndex, receiverVarOpt.get)) || + !doesWriteDominateAllReads(definedMethod, receiverVarOpt, writeIndex) + ) { Assignable } else { NonAssignable } } + /** + * @return Whether the given instance on which a field is written is used in a statement that cannot be identified + * as preserving non-assignability, in which case we can soundly return. + * + * IMPROVE handle static fields and obfuscated variable uses + */ + private def isInstanceUsedSuspiciously( + definedMethod: DefinedMethod, + taCode: TACode[TACMethodParameter, V], + writeIndex: Int, + receiverVar: V, + )(implicit state: State): Boolean = { + val stmts = taCode.stmts + if (receiverVar.definedBy.size != 1) + return true; + + val defSite = receiverVar.definedBy.head + if (defSite < -1 || (defSite == -1 && !definedMethod.definedMethod.isConstructor)) + return true; + + val uses = if (defSite == -1) + taCode.params.thisParameter.useSites + else + stmts(defSite).asAssignment.targetVar.asVar.usedBy + + // Analyze usages of the current variable for suspicious behaviour + uses.exists { index => + val stmt = stmts(index) + + // We ignore the given write ... + writeIndex != index && + // ... and ignore fresh initializations of the object, as they cannot read fields ... + !(stmt.isMethodCall && stmt.asMethodCall.name == "") && + // ... and ignore easily recognizable assignments of other fields (read dominance is checked later) + // IMPROVE: Use field access information to incorporate reflective accesses + !(stmt.isPutField && stmt.asPutField.name != state.field.name) && + // ... and ignore the case in which the statement is dominated by the given write anyway + !dominates(writeIndex, index, taCode) + } + } + /** * @return Whether the given write dominates all reads. Soundly returns true if this cannot be determined yet. */ From 05e77cf502b6ef49222a81df72f9fb6aa1cc4ca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Fri, 3 Oct 2025 16:45:06 +0100 Subject: [PATCH 30/33] Refactor field access structure --- .../AbstractFieldAssignabilityAnalysis.scala | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index 4574c3573b..ef1715f3ab 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -69,11 +69,14 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { val field: Field var fieldAssignability: FieldAssignability = NonAssignable - var fieldAccesses: Map[DefinedMethod, Map[Int, Set[(PC, AccessReceiver)]]] = Map.empty + var fieldAccesses: Map[Context, Set[(PC, AccessReceiver)]] = Map.empty var escapeDependees: Set[EOptionP[(Context, DefinitionSite), EscapeProperty]] = Set.empty var fieldWriteAccessDependee: Option[EOptionP[DeclaredField, FieldWriteAccessInformation]] = None var tacDependees: Map[DefinedMethod, EOptionP[Method, TACAI]] = Map.empty + def forEachFieldAccess(definedMethod: DefinedMethod)(f: (Context, Set[(PC, AccessReceiver)]) => Unit): Unit = + fieldAccesses.iterator.filter(_._1.method eq definedMethod).foreach(kv => f(kv._1, kv._2)) + def hasDependees: Boolean = { escapeDependees.nonEmpty || fieldWriteAccessDependee.exists(_.isRefinable) || tacDependees.valuesIterator.exists(_.isRefinable) @@ -148,15 +151,16 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { newFai.numDirectAccesses - seenDirectAccesses, newFai.numIndirectAccesses - seenIndirectAccesses ) foreach { case (contextID, pc, receiver, _) => - val method = contextProvider.contextFromId(contextID).method.asDefinedMethod + val context = contextProvider.contextFromId(contextID) val access = (pc, receiver) - state.fieldAccesses = state.fieldAccesses.updatedWith(method) { - case None => Some(Map((contextID, Set(access)))) - case Some(map) => Some(map + (contextID -> (map.getOrElse(contextID, Set.empty) + access))) + state.fieldAccesses = state.fieldAccesses.updatedWith(context) { + case None => Some(Set(access)) + case Some(accesses) => Some(accesses + access) } } - state.fieldAccesses.foreachEntry { (method, accessesByContext) => + state.fieldAccesses.foreachEntry { (context, accesses) => + val method = context.method.asDefinedMethod val tacEP = state.tacDependees.get(method) match { case Some(tacEP) => tacEP case None => @@ -165,14 +169,9 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { tacEP } - if (tacEP.hasUBP) { - accessesByContext.foreachEntry { (contextID, accesses) => - val context = contextProvider.contextFromId(contextID) - if (state.fieldAssignability != Assignable) { - state.fieldAssignability = state.fieldAssignability.meet { - determineAssignabilityFromWritesInContext(context, method, tacEP.ub.tac.get, accesses) - } - } + if (tacEP.hasUBP && state.fieldAssignability != Assignable) { + state.fieldAssignability = state.fieldAssignability.meet { + determineAssignabilityFromWritesInContext(context, method, tacEP.ub.tac.get, accesses) } } } @@ -218,16 +217,14 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { case TACAI.key => val newEP = eps.asInstanceOf[EOptionP[Method, TACAI]] val method = declaredMethods(newEP.e) - val accessesByContext = state.fieldAccesses(method) state.tacDependees += method -> newEP - accessesByContext.foreachEntry((contextId, accesses) => { - val context = contextProvider.contextFromId(contextId) + state.forEachFieldAccess(method) { (context, accesses) => if (state.fieldAssignability != Assignable) { state.fieldAssignability = state.fieldAssignability.meet { determineAssignabilityFromWritesInContext(context, method, newEP.ub.tac.get, accesses) } } - }) + } createResult() } } From 22a1e180ef65e653855e1054683209963bad2be6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Fri, 3 Oct 2025 17:43:47 +0100 Subject: [PATCH 31/33] Refine suspicious uses --- .../L2FieldAssignabilityAnalysis.scala | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 2e8d69b08b..6f73a72962 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -191,7 +191,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } if ((state.field.isNotStatic && - isInstanceUsedSuspiciously(definedMethod, taCode, writeIndex, receiverVarOpt.get)) || + isInstanceUsedSuspiciously(context, definedMethod, taCode, writeIndex, receiverVarOpt.get)) || !doesWriteDominateAllReads(definedMethod, receiverVarOpt, writeIndex) ) { Assignable @@ -207,6 +207,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * IMPROVE handle static fields and obfuscated variable uses */ private def isInstanceUsedSuspiciously( + context: Context, definedMethod: DefinedMethod, taCode: TACode[TACMethodParameter, V], writeIndex: Int, @@ -225,17 +226,23 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) else stmts(defSite).asAssignment.targetVar.asVar.usedBy - // Analyze usages of the current variable for suspicious behaviour + // Analyze uses of the current variable for suspicious usage while excluding known safe cases + val writeIndices = state.fieldAccesses(context).map(access => taCode.pcToIndex(access._1)) uses.exists { index => val stmt = stmts(index) - // We ignore the given write ... - writeIndex != index && + // We ignore any writes (possibly disruptive reads are checked for dominance later) + !writeIndices.contains(index) && // ... and ignore fresh initializations of the object, as they cannot read fields ... - !(stmt.isMethodCall && stmt.asMethodCall.name == "") && - // ... and ignore easily recognizable assignments of other fields (read dominance is checked later) + !(stmt.isMethodCall + && stmt.asMethodCall.name == "" + && (stmt.asMethodCall.declaringClass eq ClassType.Object)) && + // ... and ignore easily recognizable assignments of other fields (again, reads are checked later) ... // IMPROVE: Use field access information to incorporate reflective accesses !(stmt.isPutField && stmt.asPutField.name != state.field.name) && + // ... and ignore easily recognizable field reads of arbitrary fields on the current instance ... + stmt.forallSubExpressions(expr => !expr.isGetField || + !expr.asGetField.objRef.asVar.definedBy.contains(defSite)) && // ... and ignore the case in which the statement is dominated by the given write anyway !dominates(writeIndex, index, taCode) } From a985a4bfea002553e30f72b4014f40adc3c15429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Sat, 4 Oct 2025 10:15:07 +0100 Subject: [PATCH 32/33] Enable proper updateability of field access --- .../AbstractFieldAssignabilityAnalysis.scala | 57 ++++++++++++------- .../L1FieldAssignabilityAnalysis.scala | 51 +++++++---------- .../L2FieldAssignabilityAnalysis.scala | 49 +++++----------- 3 files changed, 69 insertions(+), 88 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index ef1715f3ab..2cd508daa5 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -147,10 +147,11 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { } state.fieldWriteAccessDependee = Some(newEP) + // Register all field accesses in the state first to enable cross access comparisons newFai.getNewestAccesses( newFai.numDirectAccesses - seenDirectAccesses, newFai.numIndirectAccesses - seenIndirectAccesses - ) foreach { case (contextID, pc, receiver, _) => + ) foreach { case (contextID, pc, receiver, _) => val context = contextProvider.contextFromId(contextID) val access = (pc, receiver) state.fieldAccesses = state.fieldAccesses.updatedWith(context) { @@ -159,19 +160,26 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { } } - state.fieldAccesses.foreachEntry { (context, accesses) => - val method = context.method.asDefinedMethod - val tacEP = state.tacDependees.get(method) match { - case Some(tacEP) => tacEP - case None => - val tacEP = propertyStore(method.definedMethod, TACAI.key) - state.tacDependees += method -> tacEP - tacEP - } + // Then determine assignability impact per access individually + newFai.getNewestAccesses( + newFai.numDirectAccesses - seenDirectAccesses, + newFai.numIndirectAccesses - seenIndirectAccesses + ) foreach { case (contextID, pc, receiver, _) => + if (state.fieldAssignability != Assignable) { + val context = contextProvider.contextFromId(contextID) + val method = context.method.asDefinedMethod + val tacEP = state.tacDependees.get(method) match { + case Some(tacEP) => tacEP + case None => + val tacEP = propertyStore(method.definedMethod, TACAI.key) + state.tacDependees += method -> tacEP + tacEP + } - if (tacEP.hasUBP && state.fieldAssignability != Assignable) { - state.fieldAssignability = state.fieldAssignability.meet { - determineAssignabilityFromWritesInContext(context, method, tacEP.ub.tac.get, accesses) + if (tacEP.hasUBP) { + state.fieldAssignability = state.fieldAssignability.meet { + determineAssignabilityFromWriteInContext(context, method, tacEP.ub.tac.get, pc, receiver) + } } } } @@ -183,16 +191,18 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { } /** - * Returns true when the method in the given context definitely updates the field in a way that forces it to be - * assignable, and false when it does not, or we are not yet sure. + * @return The assignability inferrable from the relationship of the given write to other writes / reads in the + * same context. * - * @note Callers must pass ALL write accesses of this method in this context discovered so far. + * @note Callers must register ALL writes discovered so far in the state before calling this method. + * TODO think about if this is really necessary */ - protected def determineAssignabilityFromWritesInContext( + protected def determineAssignabilityFromWriteInContext( context: Context, definedMethod: DefinedMethod, taCode: TACode[TACMethodParameter, V], - writeAccesses: Iterable[(PC, AccessReceiver)] + writePC: PC, + receiver: AccessReceiver )(implicit state: AnalysisState): FieldAssignability def createResult()(implicit state: AnalysisState): ProperPropertyComputationResult = { @@ -218,11 +228,14 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { val newEP = eps.asInstanceOf[EOptionP[Method, TACAI]] val method = declaredMethods(newEP.e) state.tacDependees += method -> newEP + // Renew field assignability analysis for all field accesses state.forEachFieldAccess(method) { (context, accesses) => - if (state.fieldAssignability != Assignable) { - state.fieldAssignability = state.fieldAssignability.meet { - determineAssignabilityFromWritesInContext(context, method, newEP.ub.tac.get, accesses) - } + val taCode = newEP.ub.tac.get + accesses.foreach { case (pc, receiver) => + if (state.fieldAssignability != Assignable) + state.fieldAssignability = state.fieldAssignability.meet { + determineAssignabilityFromWriteInContext(context, method, taCode, pc, receiver) + } } } createResult() diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala index e03a135647..ac29c716a7 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala @@ -38,20 +38,13 @@ class L1FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) type AnalysisState = State override def createState(field: Field): AnalysisState = State(field) - /** - * Returns true when the method in the given context definitely updates the field in a way that forces it to be - * assignable, and false when it does not, or we are not yet sure. - * - * @note Callers must pass ALL write accesses of this method in this context discovered so far. - */ - override def determineAssignabilityFromWritesInContext( + override def determineAssignabilityFromWriteInContext( context: Context, definedMethod: DefinedMethod, taCode: TACode[TACMethodParameter, V], - writeAccesses: Iterable[(PC, AccessReceiver)], + writePC: PC, + receiver: AccessReceiver )(implicit state: AnalysisState): FieldAssignability = { - assert(writeAccesses.nonEmpty) - val field = state.field val method = definedMethod.definedMethod @@ -61,34 +54,30 @@ class L1FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) return Assignable; } - if (writeAccesses.size > 1) { + if (state.fieldAccesses(context).size > 1) { // Multi-branch access detection is not available on this level. return Assignable; } - val assignabilities: Iterable[FieldAssignability] = writeAccesses.map { access => - val receiverVarOpt = access._2.map(uVarForDefSites(_, taCode.pcToIndex)) - if (receiverVarOpt.isDefined) { - val receiverVar = receiverVarOpt.get - if (method.isConstructor && receiverVar.definedBy == SelfReferenceParameter) { - // for instance fields it is okay if they are written in the - // constructor (w.r.t. the currently initialized object!) - NonAssignable - } else if (!referenceHasEscaped(receiverVar, taCode.stmts, definedMethod, context)) { - // A method (e.g. clone) may instantiate a new object and write the field as long as that new object - // did not yet escape. - NonAssignable - } else { - Assignable - } - } else if (!method.isStaticInitializer) { - Assignable - } else { + val receiverVarOpt = receiver.map(uVarForDefSites(_, taCode.pcToIndex)) + if (receiverVarOpt.isDefined) { + val receiverVar = receiverVarOpt.get + if (method.isConstructor && receiverVar.definedBy == SelfReferenceParameter) { + // for instance fields it is okay if they are written in the + // constructor (w.r.t. the currently initialized object!) + NonAssignable + } else if (!referenceHasEscaped(receiverVar, taCode.stmts, definedMethod, context)) { + // A method (e.g. clone) may instantiate a new object and write the field as long as that new object + // did not yet escape. NonAssignable + } else { + Assignable } + } else if (!method.isStaticInitializer) { + Assignable + } else { + NonAssignable } - - assignabilities.reduce(_.meet(_)) } } diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 6f73a72962..5e33e13339 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -113,32 +113,28 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } } - /** - * Returns true when the method in the given context definitely updates the field in a way that forces it to be - * assignable, and false when it does not, or we are not yet sure. - * - * @note Callers must pass ALL write accesses of this method in this context discovered so far. - */ - override def determineAssignabilityFromWritesInContext( + override def determineAssignabilityFromWriteInContext( context: Context, definedMethod: DefinedMethod, taCode: TACode[TACMethodParameter, V], - writeAccesses: Iterable[(PC, AccessReceiver)] + writePC: PC, + receiver: AccessReceiver )(implicit state: AnalysisState): FieldAssignability = { - assert(writeAccesses.nonEmpty) - val field = state.field val method = definedMethod.definedMethod + val writeIndex = taCode.pcToIndex(writePC) if (field.isStatic && method.isConstructor) { // A static field updated in an arbitrary constructor may be updated with (at least) the first call. // Thus, we may see its initial value or the updated value, making the field assignable. return Assignable; } - if (writeAccesses.exists { access1 => writeAccesses.exists { access2 => - access1._1 != access2._1 && - dominates(taCode.pcToIndex(access1._1), taCode.pcToIndex(access2._1), taCode) - }}) { + if (state.fieldAccesses(context).exists { case(otherWritePC, _) => + writePC != otherWritePC && ( + dominates(writeIndex, taCode.pcToIndex(otherWritePC), taCode) || + dominates(taCode.pcToIndex(otherWritePC), writeIndex, taCode) + ) + }) { // When one write is detected to dominate another within the same method, the field is definitively assigned // multiple times and cannot be effectively non-assignable, even in initializers. // IMPROVE reduce this to modifications on the same instance and consider cases where not every path @@ -146,30 +142,13 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) return Assignable; } - val assignabilities = writeAccesses.map { access => { - determineAssignabilityFromAccess(context, definedMethod, taCode)(taCode.pcToIndex(access._1), access._2) - }} - assignabilities.reduce(_.meet(_)) - } - - private def determineAssignabilityFromAccess( - context: Context, - definedMethod: DefinedMethod, - taCode: TACode[TACMethodParameter, V] - )( - writeIndex: PC, - persistentReceiverOpt: AccessReceiver - )(implicit state: State): FieldAssignability = { - val field = state.field - val method = definedMethod.definedMethod - val receiverVarOpt = persistentReceiverOpt.map(uVarForDefSites(_, taCode.pcToIndex)) - + val receiverVarOpt = receiver.map(uVarForDefSites(_, taCode.pcToIndex)) // If we have no information about the receiver, but we should have it, soundly return true. - if (state.field.isNotStatic && receiverVarOpt.isEmpty) + if (field.isNotStatic && receiverVarOpt.isEmpty) return Assignable; if (method.isInitializer && method.classFile == field.classFile) { - if (state.field.isNotStatic && receiverVarOpt.get.definedBy != SelfReferenceParameter) { + if (field.isNotStatic && receiverVarOpt.get.definedBy != SelfReferenceParameter) { // An instance field that is modified in an initializer of a different class must be assignable return Assignable; } @@ -190,7 +169,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } } - if ((state.field.isNotStatic && + if ((field.isNotStatic && isInstanceUsedSuspiciously(context, definedMethod, taCode, writeIndex, receiverVarOpt.get)) || !doesWriteDominateAllReads(definedMethod, receiverVarOpt, writeIndex) ) { From c644d97ce1df2c68a070d2312dfa0482894c575a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20R=C3=BCsch?= Date: Sat, 4 Oct 2025 10:16:07 +0100 Subject: [PATCH 33/33] Format code --- .../AbstractFieldAssignabilityAnalysis.scala | 10 ++--- .../L1FieldAssignabilityAnalysis.scala | 8 ++-- .../L2FieldAssignabilityAnalysis.scala | 39 +++++++++++-------- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala index 2cd508daa5..85cc080e8a 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/AbstractFieldAssignabilityAnalysis.scala @@ -155,7 +155,7 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { val context = contextProvider.contextFromId(contextID) val access = (pc, receiver) state.fieldAccesses = state.fieldAccesses.updatedWith(context) { - case None => Some(Set(access)) + case None => Some(Set(access)) case Some(accesses) => Some(accesses + access) } } @@ -198,11 +198,11 @@ trait AbstractFieldAssignabilityAnalysis extends FPCFAnalysis { * TODO think about if this is really necessary */ protected def determineAssignabilityFromWriteInContext( - context: Context, + context: Context, definedMethod: DefinedMethod, - taCode: TACode[TACMethodParameter, V], - writePC: PC, - receiver: AccessReceiver + taCode: TACode[TACMethodParameter, V], + writePC: PC, + receiver: AccessReceiver )(implicit state: AnalysisState): FieldAssignability def createResult()(implicit state: AnalysisState): ProperPropertyComputationResult = { diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala index ac29c716a7..b6127ab62e 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L1FieldAssignabilityAnalysis.scala @@ -39,11 +39,11 @@ class L1FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) override def createState(field: Field): AnalysisState = State(field) override def determineAssignabilityFromWriteInContext( - context: Context, + context: Context, definedMethod: DefinedMethod, - taCode: TACode[TACMethodParameter, V], - writePC: PC, - receiver: AccessReceiver + taCode: TACode[TACMethodParameter, V], + writePC: PC, + receiver: AccessReceiver )(implicit state: AnalysisState): FieldAssignability = { val field = state.field val method = definedMethod.definedMethod diff --git a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala index 5e33e13339..1c2a9b719a 100644 --- a/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala +++ b/OPAL/tac/src/main/scala/org/opalj/tac/fpcf/analyses/fieldassignability/L2FieldAssignabilityAnalysis.scala @@ -117,8 +117,8 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) context: Context, definedMethod: DefinedMethod, taCode: TACode[TACMethodParameter, V], - writePC: PC, - receiver: AccessReceiver + writePC: PC, + receiver: AccessReceiver )(implicit state: AnalysisState): FieldAssignability = { val field = state.field val method = definedMethod.definedMethod @@ -129,12 +129,13 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) return Assignable; } - if (state.fieldAccesses(context).exists { case(otherWritePC, _) => - writePC != otherWritePC && ( - dominates(writeIndex, taCode.pcToIndex(otherWritePC), taCode) || - dominates(taCode.pcToIndex(otherWritePC), writeIndex, taCode) - ) - }) { + if (state.fieldAccesses(context).exists { case (otherWritePC, _) => + writePC != otherWritePC && ( + dominates(writeIndex, taCode.pcToIndex(otherWritePC), taCode) || + dominates(taCode.pcToIndex(otherWritePC), writeIndex, taCode) + ) + } + ) { // When one write is detected to dominate another within the same method, the field is definitively assigned // multiple times and cannot be effectively non-assignable, even in initializers. // IMPROVE reduce this to modifications on the same instance and consider cases where not every path @@ -154,14 +155,16 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } } else { if (field.isStatic || - receiverVarOpt.isDefined && receiverVarOpt.get.definedBy == SelfReferenceParameter) { + receiverVarOpt.isDefined && receiverVarOpt.get.definedBy == SelfReferenceParameter + ) { // A field written outside an initializer must be lazily initialized or it is assignable if (considerLazyInitialization) { return determineLazyInitialization(writeIndex, getDefaultValues(), method, taCode); } else return Assignable; } else if (receiverVarOpt.isDefined && - referenceHasEscaped(receiverVarOpt.get, taCode.stmts, definedMethod, context)) { + referenceHasEscaped(receiverVarOpt.get, taCode.stmts, definedMethod, context) + ) { // Arbitrary methods may instantiate new objects and write instance fields, as long as the new object did // not yet escape. This effectively determines usage of the `clone` pattern. If the reference has escaped, // we need to assume that someone observed the old field value before modification, thus soundly return. @@ -170,7 +173,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) } if ((field.isNotStatic && - isInstanceUsedSuspiciously(context, definedMethod, taCode, writeIndex, receiverVarOpt.get)) || + isInstanceUsedSuspiciously(context, definedMethod, taCode, writeIndex, receiverVarOpt.get)) || !doesWriteDominateAllReads(definedMethod, receiverVarOpt, writeIndex) ) { Assignable @@ -186,11 +189,11 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * IMPROVE handle static fields and obfuscated variable uses */ private def isInstanceUsedSuspiciously( - context: Context, + context: Context, definedMethod: DefinedMethod, taCode: TACode[TACMethodParameter, V], - writeIndex: Int, - receiverVar: V, + writeIndex: Int, + receiverVar: V )(implicit state: State): Boolean = { val stmts = taCode.stmts if (receiverVar.definedBy.size != 1) @@ -220,8 +223,10 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) // IMPROVE: Use field access information to incorporate reflective accesses !(stmt.isPutField && stmt.asPutField.name != state.field.name) && // ... and ignore easily recognizable field reads of arbitrary fields on the current instance ... - stmt.forallSubExpressions(expr => !expr.isGetField || - !expr.asGetField.objRef.asVar.definedBy.contains(defSite)) && + stmt.forallSubExpressions(expr => + !expr.isGetField || + !expr.asGetField.objRef.asVar.definedBy.contains(defSite) + ) && // ... and ignore the case in which the statement is dominated by the given write anyway !dominates(writeIndex, index, taCode) } @@ -284,7 +289,7 @@ class L2FieldAssignabilityAnalysis private[analyses] (val project: SomeProject) * that does not dominate the read. */ private def fieldReadsNotDominated( - reads: IterableOnce[(Int, PC, AccessReceiver, AccessParameter)], + reads: IterableOnce[(Int, PC, AccessReceiver, AccessParameter)], writes: Seq[(DefinedMethod, Int, Option[V])] )(implicit state: State): Boolean = { reads.iterator.exists {