Skip to content

fix: crashtracker runtime stack infinite loop#3845

Merged
bwoebi merged 3 commits intomasterfrom
gyuheon0h/advance-frame-unmapped-opcode
Apr 28, 2026
Merged

fix: crashtracker runtime stack infinite loop#3845
bwoebi merged 3 commits intomasterfrom
gyuheon0h/advance-frame-unmapped-opcode

Conversation

@gyuheon0h
Copy link
Copy Markdown
Contributor

@gyuheon0h gyuheon0h commented Apr 28, 2026

Description

There was a crash report with the runtime stack unwinding infinite looping: https://app.datadoghq.com/logs?query=-%40org_id%3A603589%20%40error.is_crash%3Atrue%20%40org_id%3A1000626888&agg_m=count&agg_m_source=base&agg_t=count&clustering_pattern_field_path=message&cols=host%2Cservice%2C%40org_id&event=AwAAAZ3U6zFHIybQJAAAABhBWjNVNnpGSEFBQVF1X283ZzRmcEh3QUEAAAAkMDE5ZGQ0ZWUtNTMzNC00Mzk4LTliNzktOGU5YTQ5NjZhZDhjAANM0A&fromUser=true&index=instrumentation-telemetry-data&link_source=monitor_notif&messageDisplay=inline&refresh_mode=sliding&storage=flex_tier&stream_sort=desc&viz=stream&from_ts=1747059580431&to_ts=1747145980431&live=true

Here is what the raw stack looks like

    [
           {
              "column": 4294967295,
              "file": "some-repeated-file-name.php",
              "function": "someRepeatedFunc",
              "line": 157,
              "type_name": "<corrupted scope>"
            },
            {
              "column": 4294967295,
              "file": "some-repeated-file-name.php",
              "function": "someRepeatedFunc",
              "line": 157,
              "type_name": "<corrupted scope>"
            },
            {
              "column": 4294967295,
              "file": "some-repeated-file-name.php",
              "function": "someRepeatedFunc",
              "line": 157,
              "type_name": "<corrupted scope>"
            },
            ....
            {
              "column": 4294967295,
              "file": "some-repeated-file-name.php",
              "function": "someRepeatedFunc",
              "line": 157,
              "type_name": "<corrupted scope>"
            },
            {
              "column": 4294967295,
              "file": "some-repeated-file-name.php",
              "function": "someRepeatedFunc",
              "line": 157,
              "type_name": "<corrupted scope>"
            }
          ]

Couple of things here.

  1. type_name is `'
    • this means that this line:
frame.type_name = func->common.scope ? zai_is_mapped(&func->common.scope->name, sizeof(zend_string *)) ? dd_validate_zstr(func->common.scope->name) : DDOG_CHARSLICE_C("<corrupted scope>") : DDOG_CHARSLICE_C("");

triggered

  1. The column number is 4294967295. This is unrealistic. The reason this is so is because its -1 in uint32. 2^32-1 = 4294967295

Then, this points to this section of code

if (!zai_is_mapped(opline, sizeof(zend_op))) {
                frame.column = -1;
                EMIT(&frame);
                continue;
            }

being the culprit. frame.column is set to -1, and the frame does not proceed, since we don't perform call = call->prev_execute_data;

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@gyuheon0h gyuheon0h changed the title Advance frame or else we will just infinite loop fix: crashtracker runtime stack infinite loop and negative number unit32 wrapping Apr 28, 2026
@gyuheon0h gyuheon0h marked this pull request as ready for review April 28, 2026 18:15
@gyuheon0h gyuheon0h requested a review from a team as a code owner April 28, 2026 18:15
frame.column = -1;
frame.column = 0;
EMIT(&frame);
call = call->prev_execute_data;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is valid, yeah.

Copy link
Copy Markdown
Collaborator

@morrisonlevi morrisonlevi Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In light of the other investigation... we cannot be certain call-> is valid xD

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is checked in line 57.

@bwoebi
Copy link
Copy Markdown
Collaborator

bwoebi commented Apr 28, 2026

@gyuheon0h I was intentionally using the negative numbers as markers.

Unless there was a problem with the fact that they wrap?

@gyuheon0h
Copy link
Copy Markdown
Contributor Author

gyuheon0h commented Apr 28, 2026

@gyuheon0h I was intentionally using the negative numbers as markers.

Unless there was a problem with the fact that they wrap?

@bwoebi Not particularly, but theyre noisy IMO. Libdatadog also says to send 0 for unknown values, but up to you 🤷

@bwoebi
Copy link
Copy Markdown
Collaborator

bwoebi commented Apr 28, 2026

Well, PHP already uses 0 itself when unknown. This is to distinguish.

And sure, they're noisy, so you actually see "there's corruption"- that's good thing, right?

Could you please revert that part? Thanks :-)

@datadog-datadog-prod-us1-2
Copy link
Copy Markdown

datadog-datadog-prod-us1-2 Bot commented Apr 28, 2026

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 60.68% (-0.01%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 8e49afb | Docs | Datadog PR Page | Give us feedback!

@gyuheon0h gyuheon0h changed the title fix: crashtracker runtime stack infinite loop and negative number unit32 wrapping fix: crashtracker runtime stack infinite loop Apr 28, 2026
@gyuheon0h
Copy link
Copy Markdown
Contributor Author

Well, PHP already uses 0 itself when unknown. This is to distinguish.

And sure, they're noisy, so you actually see "there's corruption"- that's good thing, right?

Could you please revert that part? Thanks :-)

@bwoebi done, thanks :) 👍

@bwoebi bwoebi merged commit 8067015 into master Apr 28, 2026
23 of 203 checks passed
@bwoebi bwoebi deleted the gyuheon0h/advance-frame-unmapped-opcode branch April 28, 2026 18:24
@github-actions github-actions Bot added this to the 1.19.0 milestone Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants