feat: Implement instruction_addr_adjustment (#42533)

Implements handling of `instruction_addr_adjustment` and attaching
appropriate per-frame attributes (`adjust_instruction_addr`) when sending stack traces to
symbolicator.

It implements the Sentry and Profiling side of [RFC
43](getsentry/rfcs#43)

The Symbolicator side is implemented in
getsentry/symbolicator#948.

---------

Co-authored-by: Sebastian Zivota <[email protected]>

Author: Swatinem

src/sentry/lang/native/processing.py

@@ -302,15 +302,24 @@ def _handles_frame(data, frame):
     return is_native_platform(platform) and frame.get("instruction_addr") is not None
 
 
-def get_frames_for_symbolication(frames, data, modules):
+def get_frames_for_symbolication(
+    frames,
+    data,
+    modules,
+    adjustment=None,
+):
     modules_by_debug_id = None
     rv = []
+    adjustment = adjustment or "auto"
 
     for frame in reversed(frames):
         if not _handles_frame(data, frame):
             continue
         s_frame = dict(frame)
 
+        if adjustment == "none":
+            s_frame["adjust_instruction_addr"] = False
+
         # validate and expand addressing modes.  If we can't validate and
         # expand it, we keep None which is absolute.  That's not great but
         # at least won't do damage.
@@ -342,6 +351,13 @@ def get_frames_for_symbolication(frames, data, modules):
             s_frame["addr_mode"] = sanitized_addr_mode
         rv.append(s_frame)
 
+    if len(rv) > 0:
+        first_frame = rv[0]
+        if adjustment == "all":
+            first_frame["adjust_instruction_addr"] = True
+        elif adjustment == "all_but_first":
+            first_frame["adjust_instruction_addr"] = False
+
     return rv
 
 
@@ -362,7 +378,10 @@ def process_payload(data):
         {
             "registers": sinfo.stacktrace.get("registers") or {},
             "frames": get_frames_for_symbolication(
-                sinfo.stacktrace.get("frames") or (), data, modules
+                sinfo.stacktrace.get("frames") or (),
+                data,
+                modules,
+                sinfo.stacktrace.get("instruction_addr_adjustment"),
             ),
         }
         for sinfo in stacktrace_infos

src/sentry/profiles/task.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+from copy import deepcopy
 from datetime import datetime
 from time import sleep, time
 from typing import Any, List, Mapping, MutableMapping, Optional, Tuple
@@ -72,6 +73,8 @@ def process_profile_task(
                 )
                 return
 
+            # WARNING(loewenheim): This function call may mutate `profile`'s frame list!
+            # See comments in the function for why this happens.
             raw_modules, raw_stacktraces = _prepare_frames_from_profile(profile)
             modules, stacktraces = _symbolicate(
                 project=project,
@@ -221,18 +224,41 @@ def _normalize(profile: Profile, organization: Organization) -> None:
 def _prepare_frames_from_profile(profile: Profile) -> Tuple[List[Any], List[Any]]:
     modules = profile["debug_meta"]["images"]
 
+    # NOTE: the usage of `adjust_instruction_addr` assumes that all
+    # the profilers on all the platforms are walking stacks right from a
+    # suspended threads cpu context
+
     # in the sample format, we have a frames key containing all the frames
     if "version" in profile:
-        stacktraces = [{"registers": {}, "frames": profile["profile"]["frames"]}]
+        frames = profile["profile"]["frames"]
+
+        for stack in profile["profile"]["stacks"]:
+            if len(stack) > 0:
+                # Make a deep copy of the leaf frame with adjust_instruction_addr = False
+                # and append it to the list. This ensures correct behavior
+                # if the leaf frame also shows up in the middle of another stack.
+                first_frame_idx = stack[0]
+                frame = deepcopy(frames[first_frame_idx])
+                frame["adjust_instruction_addr"] = False
+                frames.append(frame)
+                stack[0] = len(frames) - 1
+
+        stacktraces = [{"registers": {}, "frames": frames}]
     # in the original format, we need to gather frames from all samples
     else:
-        stacktraces = [
-            {
-                "registers": {},
-                "frames": s["frames"],
-            }
-            for s in profile["sampled_profile"]["samples"]
-        ]
+        stacktraces = []
+        for s in profile["sampled_profile"]["samples"]:
+            frames = s["frames"]
+
+            if len(frames) > 0:
+                frames[0]["adjust_instruction_addr"] = False
+
+            stacktraces.append(
+                {
+                    "registers": {},
+                    "frames": frames,
+                }
+            )
     return (modules, stacktraces)
 
 

tests/sentry/lang/native/test_processing.py

@@ -178,6 +178,54 @@ def test_filter_frames():
     assert len(filtered_frames) == 0
 
 
+def test_instruction_addr_adjustment_auto():
+    frames = [
+        {"instruction_addr": "0xdeadbeef", "platform": "native"},
+        {"instruction_addr": "0xbeefdead", "platform": "native"},
+    ]
+
+    processed_frames = get_frames_for_symbolication(frames, None, None, None)
+
+    assert "adjust_instruction_addr" not in processed_frames[0].keys()
+    assert "adjust_instruction_addr" not in processed_frames[1].keys()
+
+
+def test_instruction_addr_adjustment_all():
+    frames = [
+        {"instruction_addr": "0xdeadbeef", "platform": "native"},
+        {"instruction_addr": "0xbeefdead", "platform": "native"},
+    ]
+
+    processed_frames = get_frames_for_symbolication(frames, None, None, "all")
+
+    assert processed_frames[0]["adjust_instruction_addr"]
+    assert "adjust_instruction_addr" not in processed_frames[1].keys()
+
+
+def test_instruction_addr_adjustment_all_but_first():
+    frames = [
+        {"instruction_addr": "0xdeadbeef", "platform": "native"},
+        {"instruction_addr": "0xbeefdead", "platform": "native"},
+    ]
+
+    processed_frames = get_frames_for_symbolication(frames, None, None, "all_but_first")
+
+    assert not processed_frames[0]["adjust_instruction_addr"]
+    assert "adjust_instruction_addr" not in processed_frames[1].keys()
+
+
+def test_instruction_addr_adjustment_none():
+    frames = [
+        {"instruction_addr": "0xdeadbeef", "platform": "native"},
+        {"instruction_addr": "0xbeefdead", "platform": "native"},
+    ]
+
+    processed_frames = get_frames_for_symbolication(frames, None, None, "none")
+
+    assert not processed_frames[0]["adjust_instruction_addr"]
+    assert not processed_frames[1]["adjust_instruction_addr"]
+
+
 @pytest.mark.django_db
 @mock.patch("sentry.lang.native.processing.Symbolicator")
 def test_il2cpp_symbolication(mock_symbolicator, default_project):

tests/sentry/profiles/consumers/test_process.py

@@ -1,3 +1,4 @@
+from copy import copy
 from datetime import datetime
 from unittest.mock import Mock, patch
 
@@ -6,6 +7,7 @@
 from arroyo.types import BrokerValue, Message, Partition, Topic
 
 from sentry.profiles.consumers.process.factory import ProcessProfileStrategyFactory
+from sentry.profiles.task import _prepare_frames_from_profile
 from sentry.testutils.cases import TestCase
 from sentry.utils import json
 
@@ -56,3 +58,51 @@ def test_basic_profile_to_celery(self, process_profile_task):
         )
 
         process_profile_task.assert_called_with(profile=profile)
+
+
+def test_adjust_instruction_addr_sample_format():
+    original_frames = [
+        {"instruction_addr": "0xdeadbeef"},
+        {"instruction_addr": "0xbeefdead"},
+        {"instruction_addr": "0xfeedface"},
+    ]
+    profile = {
+        "version": "1",
+        "profile": {
+            "frames": copy(original_frames),
+            "stacks": [[1, 0], [0, 1, 2]],
+        },
+        "debug_meta": {"images": []},
+    }
+
+    _, stacktraces = _prepare_frames_from_profile(profile)
+    assert profile["profile"]["stacks"] == [[3, 0], [4, 1, 2]]
+    frames = stacktraces[0]["frames"]
+
+    for i in range(3):
+        assert frames[i] == original_frames[i]
+
+    assert frames[3] == {"instruction_addr": "0xbeefdead", "adjust_instruction_addr": False}
+    assert frames[4] == {"instruction_addr": "0xdeadbeef", "adjust_instruction_addr": False}
+
+
+def test_adjust_instruction_addr_original_format():
+    profile = {
+        "sampled_profile": {
+            "samples": [
+                {
+                    "frames": [
+                        {"instruction_addr": "0xdeadbeef", "platform": "native"},
+                        {"instruction_addr": "0xbeefdead", "platform": "native"},
+                    ],
+                }
+            ]
+        },
+        "debug_meta": {"images": []},
+    }
+
+    _, stacktraces = _prepare_frames_from_profile(profile)
+    frames = stacktraces[0]["frames"]
+
+    assert not frames[0]["adjust_instruction_addr"]
+    assert "adjust_instruction_addr" not in frames[1]