feat: consider variable types at extraction by szokeasaurusrex · Pull Request #2078 · getsentry/symbolicator

szokeasaurusrex · 2026-10-02T12:49:58Z

WIP change to consider variable types when extracting the variables

Resolves [variables extraction] Format numbers/bools/etc. properly into the schema #2066
Resolves SYMBOLI-78

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

Dav1dde · 2026-10-02T13:08:28Z

+        let (start_truncation_notice, end_truncation_notice) = match value.truncated {
+            Some((count, Endian::Little)) => (Some(truncation_notice(count)), None),
+            Some((count, Endian::Big)) => (None, Some(truncation_notice(count))),
+            None => (None, None),


I don't think that makes sense, even with different endianness we're still just formatting a plain slice of memory from small to large.

What exactly is confusing?

We are considering endianness when reading this memory slice, as we don't use the general method for reading slices that does not consider the endianness. So, where to place the truncation notice does depend on endianness in this case.

It's a slice of bytes, where the notice is doesn't depend on the endianness - it's just raw bytes. Seems like you already dealt with this though.

linear-code · 2026-10-05T08:54:25Z

SYMBOLI-78

szokeasaurusrex · 2026-10-09T12:10:30Z

+        // TODO: Is it correct to extract both signed and unsigned chars as u8 in this way?
+        (Primitive(SignedChar | UnsignedChar), 1) => {
+            get_value!(u8, |v| format_variable!("'{}'", v.escape_ascii()))
+        }


@Dav1dde is this correct?

This looks good to me.

cursor

Cursor Bugbot has reviewed your changes and found 3 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit b6bc016. Configure here.}

cursor · 2026-10-13T13:02:28Z

-                .find_map(|loc| resolve_value(cache, registers, memory, loc, variable.ty()));
+            let value = variable.locations().find_map(|loc| {
+                resolve_variable_value(cache, registers, memory, loc, variable.ty())
+            })?;


Missing values omit variables

Medium Severity

do_extract now drops a variable when every location fails to produce a value. The name, type, and kind were already resolved, and the previous code still emitted the variable with an empty value. Those names disappear whenever memory is missing or the type is unsupported.

^{Reviewed by Cursor Bugbot for commit b6bc016. Configure here.}

I guess we can still emit such variables without a value. @Dav1dde what do you think?

Yeah I think we should, to give the user at least some confidence that what we're doing is correct, we just failed to extract the value.

Can definitely do that in a follow-up/separate ticket.

cursor · 2026-10-13T13:02:29Z

+        // Floating-point types. Reading f128 from memory is currently not supported, so we don't
+        // handle it for now.
+        (Primitive(Float), 4) => get_value!(f32),
+        (Primitive(Float), 8) => get_value!(f64),


Float JSON conversion misrepresents values

Medium Severity

Float reads go through VariableValue::value, which converts with Into<serde_json::Value>. NaN and infinities become JSON null, and f32 is widened to f64, so many finite 32-bit values pick up extra digits when serialized.

^{Reviewed by Cursor Bugbot for commit b6bc016. Configure here.}

I guess it would make sense to support infinity and NaN

I am not sure about whether we should care about 32-bit values picking up additional digits when serialized. Apparently enabling arbitrary_precision may fix this; otherwise we might need custom serialization logic. Should we enable arbitrary_precision?

Let's not enable it, we can track this is a separate issue we can come back to later if we think that's an issue with real data.

If we fix this on this end, we'd also need to make sure the frontend deals with it properly (e.g. keeps the same precision), which I doubt it does.

@Dav1dde

Create a separate module for extracting variables, per @Dav1dde's suggestion: #2078 (comment).

Dav1dde · 2026-10-20T07:55:21Z

+    }
+}
+
+impl FromStr for PointerAddress {


Is this used, I can't find a usage 🤔

If the deserialization part is unused, I would keep it out of the module for now and focus purely on serialization.

Dav1dde · 2026-10-20T07:56:38Z

-                .find_map(|loc| resolve_value(cache, registers, memory, loc, variable.ty()));
+            let value = variable.locations().find_map(|loc| {
+                resolve_variable_value(cache, registers, memory, loc, variable.ty())
+            })?;


Yeah I think we should, to give the user at least some confidence that what we're doing is correct, we just failed to extract the value.

Can definitely do that in a follow-up/separate ticket.

Dav1dde · 2026-10-20T07:58:11Z

+    // Actual beginning of the function is here, everything above are helper type and macro defs.
+


Suggested change

// Actual beginning of the function is here, everything above are helper type and macro defs.

Dav1dde · 2026-10-20T07:59:26Z

+        // Floating-point types. Reading f128 from memory is currently not supported, so we don't
+        // handle it for now.
+        (Primitive(Float), 4) => get_value!(f32),
+        (Primitive(Float), 8) => get_value!(f64),


Let's not enable it, we can track this is a separate issue we can come back to later if we think that's an issue with real data.

If we fix this on this end, we'd also need to make sure the frontend deals with it properly (e.g. keeps the same precision), which I doubt it does.

Dav1dde · 2026-10-20T07:59:49Z

+        // TODO: Is it correct to extract both signed and unsigned chars as u8 in this way?
+        (Primitive(SignedChar | UnsignedChar), 1) => {
+            get_value!(u8, |v| format_variable!("'{}'", v.escape_ascii()))
+        }


This looks good to me.

Dav1dde · 2026-10-20T08:01:50Z

 }
+
+#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+pub(crate) enum PointerAddress {


Suggested change

pub(crate) enum PointerAddress {

pub enum PointerAddress {

szokeasaurusrex added this pull request to stack #2079 October 2, 2026 12:50

szokeasaurusrex changed the title ~~feat: consider variable values at extraction~~ feat: consider variable types at extraction Oct 2, 2026

Dav1dde reviewed Oct 2, 2026

View reviewed changes

szokeasaurusrex removed this pull request from stack #2079 October 8, 2026 09:03

szokeasaurusrex mentioned this pull request Oct 8, 2026

ref(native): Dedicated submodule for variables extraction #2086

Open

szokeasaurusrex force-pushed the szokeasaurusrex/schema-float-int branch from 89d5a70 to 819d4af Compare October 8, 2026 09:03

szokeasaurusrex changed the base branch from szokeasaurusrex/variables-extraction-schema to szokeasaurusrex/separate-variables-module October 8, 2026 09:03

szokeasaurusrex added this pull request to stack #2087 October 8, 2026 09:03

szokeasaurusrex force-pushed the szokeasaurusrex/schema-float-int branch from 819d4af to 41ab4cd Compare October 8, 2026 09:31

szokeasaurusrex requested a review from Dav1dde October 9, 2026 12:09

szokeasaurusrex commented Oct 9, 2026

View reviewed changes

szokeasaurusrex marked this pull request as ready for review October 13, 2026 12:55

szokeasaurusrex requested a review from a team as a code owner October 13, 2026 12:55

cursor Bot reviewed Oct 13, 2026

View reviewed changes

szokeasaurusrex marked this pull request as draft October 13, 2026 13:17

szokeasaurusrex force-pushed the szokeasaurusrex/schema-float-int branch from b6bc016 to 7466db0 Compare October 13, 2026 14:23

sentry-warden Bot reviewed Oct 13, 2026

View reviewed changes

Comment thread crates/symbolicator-native/src/symbolication/native_variables.rs

szokeasaurusrex mentioned this pull request Oct 13, 2026

Fix ineffective depth guard in native variable extraction #2094

Open

szokeasaurusrex added 4 commits October 13, 2026 17:14

feat: consider variable values at extraction

8a584c1

code review feedback

2adec90

reformat imports

6bb4bd2

ref: Simplify unknown bytes extraction

9b26271

szokeasaurusrex added a commit that referenced this pull request Oct 13, 2026

ref(native): Dedicated submodule for variables extraction

8948879

Create a separate module for extracting variables, per @Dav1dde's suggestion: #2078 (comment).

szokeasaurusrex force-pushed the szokeasaurusrex/schema-float-int branch from 7466db0 to 9b26271 Compare October 13, 2026 15:15

szokeasaurusrex removed this pull request from stack #2087 October 13, 2026 15:17

szokeasaurusrex added this pull request to stack #2096 October 13, 2026 15:19

szokeasaurusrex marked this pull request as ready for review October 13, 2026 15:57

Dav1dde approved these changes Oct 20, 2026

View reviewed changes

		// Actual beginning of the function is here, everything above are helper type and macro defs.

Conversation

szokeasaurusrex commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linear-code Bot commented Oct 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot Oct 13, 2026

Choose a reason for hiding this comment

Missing values omit variables

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot Oct 13, 2026

Choose a reason for hiding this comment

Float JSON conversion misrepresents values

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

szokeasaurusrex commented Oct 2, 2026 •

edited

Loading