Refactor JvmTransferRelation to handle increments separately from other arithmetic instructions

Summary:
closes T19485

The old workflow for `IINC` despite being correct was confusing and inconvenient to support. Now the JVM transfer relation should compute the incremented abstract state in a separate `computeIncrement` method.

Author: DIvanov503

base/src/main/java/proguard/analysis/cpa/jvm/domain/reference/JvmReferenceTransferRelation.java

@@ -19,7 +19,6 @@
 package proguard.analysis.cpa.jvm.domain.reference;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import proguard.analysis.datastructure.callgraph.Call;
 import proguard.classfile.instruction.Instruction;
@@ -63,11 +62,6 @@ public SetAbstractState<Reference> getAbstractDefault()
     @Override
     protected List<SetAbstractState<Reference>> applyInstruction(Instruction instruction, List<SetAbstractState<Reference>> operands, int resultCount)
     {
-        // the increment doesn't affect the reference
-        if (instruction.opcode == Instruction.OP_IINC)
-        {
-            return Collections.singletonList(operands.get(0));
-        }
         List<SetAbstractState<Reference>> answer = new ArrayList<>(resultCount);
         for (int i = 0; i < resultCount; i++)
         {
@@ -76,6 +70,13 @@ protected List<SetAbstractState<Reference>> applyInstruction(Instruction instruc
         return answer;
     }
 
+    @Override
+    protected SetAbstractState<Reference> computeIncrement(SetAbstractState<Reference> state, int value)
+    {
+        // references cannot be incremented
+        return getAbstractDefault();
+    }
+
     @Override
     public void invokeMethod(JvmAbstractState<SetAbstractState<Reference>> state, Call call, List<SetAbstractState<Reference>> operands)
     {

base/src/main/java/proguard/analysis/cpa/jvm/transfer/JvmTransferRelation.java

@@ -124,6 +124,14 @@ protected List<StateT> applyInstruction(Instruction instruction, List<StateT> op
         return answer;
     }
 
+    /**
+     * Returns the abstract state of the incremented input {@code state} by {@code value}. The default implementation computes the join.
+     */
+    protected StateT computeIncrement(StateT state, int value)
+    {
+        return state.join(getAbstractIntegerConstant(value));
+    }
+
     /**
      * Returns an abstract representation of a byte constant {@code b}.
      */
@@ -459,31 +467,25 @@ public void visitVariableInstruction(Clazz clazz, Method method, CodeAttribute c
         {
             if (variableInstruction.opcode == Instruction.OP_IINC)
             {
-                List<StateT> operands = new ArrayList<>(2);
-                operands.add(abstractState.getVariableOrDefault(variableInstruction.variableIndex, getAbstractDefault()));
-                operands.add(getAbstractIntegerConstant(variableInstruction.constant));
-                List<StateT> res = applyInstruction(variableInstruction, operands, 1);
-                if (res.size() != 1)
-                {
-                    throw new IllegalStateException("applyInstruction should return a list of size 1");
-                }
-                abstractState.setVariable(variableInstruction.variableIndex, res.get(0), getAbstractDefault());
+                abstractState.setVariable(variableInstruction.variableIndex,
+                                          computeIncrement(abstractState.getVariableOrDefault(variableInstruction.variableIndex, getAbstractDefault()), variableInstruction.constant),
+                                          getAbstractDefault());
+                return;
             }
-            else if (variableInstruction.isLoad())
+            if (variableInstruction.isLoad())
             {
                 if (variableInstruction.isCategory2())
                 {
                     abstractState.push(abstractState.getVariableOrDefault(variableInstruction.variableIndex + 1, getAbstractDefault()));
                 }
                 abstractState.push(abstractState.getVariableOrDefault(variableInstruction.variableIndex, getAbstractDefault()));
+                return;
             }
-            else if (variableInstruction.isStore())
+            // process store instruction
+            abstractState.setVariable(variableInstruction.variableIndex, abstractState.pop(), getAbstractDefault());
+            if (variableInstruction.isCategory2())
             {
-                abstractState.setVariable(variableInstruction.variableIndex, abstractState.pop(), getAbstractDefault());
-                if (variableInstruction.isCategory2())
-                {
-                    abstractState.setVariable(variableInstruction.variableIndex + 1, abstractState.pop(), getAbstractDefault());
-                }
+                abstractState.setVariable(variableInstruction.variableIndex + 1, abstractState.pop(), getAbstractDefault());
             }
         }
 

base/src/test/kotlin/proguard/analysis/cpa/JvmTransferRelationTest.kt

@@ -736,8 +736,7 @@ class JvmTransferRelationTest : FreeSpec({
                         InstructionExpression(
                             VariableInstruction(Instruction.OP_IINC, 0, 1),
                             listOf(
-                                ExpressionAbstractState(setOf(ValueExpression(ParticularIntegerValue(2)))),
-                                ExpressionAbstractState(setOf(ValueExpression(ParticularIntegerValue(1))))
+                                ExpressionAbstractState(setOf(ValueExpression(ParticularIntegerValue(2))))
                             )
                         )
                     )

base/src/testFixtures/kotlin/proguard/testutils/cpa/ExpressionTransferRelation.kt

@@ -22,6 +22,7 @@ import proguard.analysis.cpa.jvm.state.JvmAbstractState
 import proguard.analysis.cpa.jvm.transfer.JvmTransferRelation
 import proguard.analysis.datastructure.callgraph.Call
 import proguard.classfile.instruction.Instruction
+import proguard.classfile.instruction.VariableInstruction
 import proguard.classfile.util.ClassUtil
 import proguard.evaluation.value.ParticularDoubleValue
 import proguard.evaluation.value.ParticularFloatValue
@@ -61,6 +62,17 @@ class ExpressionTransferRelation : JvmTransferRelation<ExpressionAbstractState>(
             )
     }
 
+    override fun computeIncrement(state: ExpressionAbstractState, value: Int): ExpressionAbstractState {
+        return ExpressionAbstractState(
+            setOf(
+                InstructionExpression(
+                    VariableInstruction(Instruction.OP_IINC, 0, value),
+                    listOf(state)
+                )
+            )
+        )
+    }
+
     override fun getAbstractDoubleConstant(d: Double): MutableList<ExpressionAbstractState> {
         return mutableListOf(
             abstractDefault,

docs/md/releasenotes.md

@@ -1,3 +1,8 @@
+## Version 9.0.8
+
+### API changes
+- `JvmTransferRelation` has been refactored to model `IINC` in a separate `computeIncrement` method.
+
 ## Version 9.0.7
 
 ### Improved