fix: iteration block hook injection [backport 2.21] (#13801)

Backport d7b62d5 from #13772 to 2.21.

We fix an issue with the instrumentation of the beginning of iteration
blocks (e.g. for loops) whereby instrumenting the end might cause
unexpected behaviour when the modified function is invoked. This is due
to the bytecode VM not liking any bytecode interposed before the
instruction that ends the iteration block (e.g. `END_FOR`). This also
solves the problem of double-calls of the instrumented hook when hitting
the beginning of such blocks.

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

---------

Co-authored-by: Gabriele N. Tornetta <[email protected]>
Co-authored-by: Tyler Finethy <[email protected]>

Author: 3 people

ddtrace/internal/injection.py

@@ -7,6 +7,7 @@
 from typing import Tuple  # noqa:F401
 
 from bytecode import Bytecode
+from bytecode import Instr
 
 from ddtrace.internal.assembly import Assembly
 
@@ -109,6 +110,12 @@ def _inject_hook(code: Bytecode, hook: HookType, lineno: int, arg: Any) -> None:
         raise InvalidLine("Line %d does not exist or is either blank or a comment" % lineno)
 
     for i in locs:
+        instr = code[i]
+        if isinstance(instr, Instr) and instr.name.startswith("END_"):
+            # This is the end of a block, e.g. a for loop. We have already
+            # instrumented the block on entry, so we skip instrumenting the
+            # end as well.
+            continue
         code[i:i] = INJECTION_ASSEMBLY.bind(dict(hook=hook, arg=arg), lineno=lineno)
 
 

releasenotes/notes/fix-iteration-block-injection-af354b7004554dfd.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    dynamic instrumentation: fixed an issue with the instrumentation of the
+    first line of an iteration block (e.g. for loops) that could have caused
+    undefined behavior.

tests/internal/test_injection.py

@@ -253,3 +253,18 @@ def test_finally():
     f()
 
     hook.assert_called_once_with(arg)
+
+
+def test_for_block():
+    def for_loop():
+        a = []
+        for i in range(10):
+            a.append(i)
+        return a
+
+    hook, arg = mock.Mock(), mock.Mock()
+
+    with injected_hook(for_loop, hook, arg, line=for_loop.__code__.co_firstlineno + 2):
+        for_loop()
+
+    hook.assert_called_once_with(arg)