Fix duplicated locals with arguments (#7683)

When hoisting local variables remove the locals that could be have the
 same name than arguments.
This could happen resulting of the instrumentation by the tracer
that reshuffle the local variable table and then reassign dedicated
slots to those arguments

Author: jpbempel

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java

@@ -476,6 +476,7 @@ private Collection<LocalVariableNode> initAndHoistLocalVars(InsnList insnList) {
       }
       checkHoistableLocalVar(localVar, localVarsByName, localVarsBySlot, hoistableVarByName);
     }
+    removeDuplicatesFromArgs(hoistableVarByName, localVarsBySlotArray);
     // hoist vars
     List<LocalVariableNode> results = new ArrayList<>();
     for (Map.Entry<String, List<LocalVariableNode>> entry : hoistableVarByName.entrySet()) {
@@ -512,6 +513,19 @@ private Collection<LocalVariableNode> initAndHoistLocalVars(InsnList insnList) {
     return results;
   }
 
+  private void removeDuplicatesFromArgs(
+      Map<String, List<LocalVariableNode>> hoistableVarByName,
+      LocalVariableNode[] localVarsBySlotArray) {
+    for (int idx = 0; idx < argOffset; idx++) {
+      LocalVariableNode localVar = localVarsBySlotArray[idx];
+      if (localVar == null) {
+        continue;
+      }
+      // we are removing local variables that are arguments
+      hoistableVarByName.remove(localVar.name);
+    }
+  }
+
   private LabelNode findLatestLabel(InsnList instructions, Set<LabelNode> endLabels) {
     for (AbstractInsnNode insn = instructions.getLast(); insn != null; insn = insn.getPrevious()) {
       if (insn instanceof LabelNode && endLabels.contains(insn)) {
@@ -762,8 +776,8 @@ private void collectArguments(InsnList insnList, Snapshot.Kind kind) {
     int slot = isStatic ? 0 : 1;
     for (Type argType : argTypes) {
       String currentArgName = null;
-      if (slot < localVarsBySlot.length) {
-        LocalVariableNode localVarNode = localVarsBySlot[slot];
+      if (slot < localVarsBySlotArray.length) {
+        LocalVariableNode localVarNode = localVarsBySlotArray[slot];
         currentArgName = localVarNode != null ? localVarNode.name : null;
       }
       if (currentArgName == null) {

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/Instrumentor.java

@@ -45,7 +45,7 @@ public abstract class Instrumentor {
   protected final LabelNode methodEnterLabel;
   protected int localVarBaseOffset;
   protected int argOffset;
-  protected final LocalVariableNode[] localVarsBySlot;
+  protected final LocalVariableNode[] localVarsBySlotArray;
   protected LabelNode returnHandlerLabel;
   protected final List<CapturedContextInstrumentor.FinallyBlock> finallyBlocks = new ArrayList<>();
 
@@ -68,7 +68,7 @@ public Instrumentor(
     for (Type t : argTypes) {
       argOffset += t.getSize();
     }
-    localVarsBySlot = extractLocalVariables(argTypes);
+    localVarsBySlotArray = extractLocalVariables(argTypes);
   }
 
   public abstract InstrumentationResult.Status instrument();

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/MetricInstrumentor.java

@@ -697,8 +697,8 @@ private ASMHelper.Type tryRetrieveArgument(String head, InsnList insnList) {
       int slot = instrumentor.isStatic ? 0 : 1;
       for (Type argType : argTypes) {
         String currentArgName = null;
-        if (instrumentor.localVarsBySlot.length > 0) {
-          LocalVariableNode localVarNode = instrumentor.localVarsBySlot[slot];
+        if (instrumentor.localVarsBySlotArray.length > 0) {
+          LocalVariableNode localVarNode = instrumentor.localVarsBySlotArray[slot];
           currentArgName = localVarNode != null ? localVarNode.name : null;
         }
         if (currentArgName == null) {

dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/TracerDebuggerIntegrationTest.java

@@ -4,6 +4,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.datadog.debugger.agent.JsonSnapshotSerializer;
@@ -99,6 +100,8 @@ void testTracerSameMethod() throws Exception {
     Snapshot snapshot = request.getDebugger().getSnapshot();
     assertEquals(PROBE_ID.getId(), snapshot.getProbe().getId());
     assertEquals(42, snapshot.getCaptures().getEntry().getArguments().get("argInt").getValue());
+    // no locals captured
+    assertNull(snapshot.getCaptures().getEntry().getLocals());
     assertTrue(Pattern.matches("[0-9a-f]+", request.getTraceId()));
     assertTrue(Pattern.matches("\\d+", request.getSpanId()));
   }