Skip to content

Conversation

@realFlowControl
Copy link
Member

@realFlowControl realFlowControl commented Dec 23, 2025

Description

A crash report ...

0x55af66530fbf _zend_mm_alloc
0x7fe72dea8401 alloc_prof_orig_alloc (/go/src/github.com/DataDog/apm-reliability/dd-trace-php/profiling/src/allocation/allocation_le83.rs:379)
0x7fe72dea82de alloc_prof_malloc (/go/src/github.com/DataDog/apm-reliability/dd-trace-php/profiling/src/allocation/allocation_le83.rs:350)
0x55af6654bcc5 zend_register_constant
0x7fe72dea7635 execute_internal (/go/src/github.com/DataDog/apm-reliability/dd-trace-php/profiling/src/wall_time.rs:79)
0x55af665cd353 zend_execute
0x55af6655d2c8 zend_execute_scripts
0x55af664f79ae php_execute_script
0x7fe732896305 __libc_start_main
0x55af66248c61 _start

... with a 0x0 si_addr indicated that _zend_mm_alloc() was being called with an invalid heap argument (being NULL and not dangling). This pointer to the heap we pass to _zend_mm_alloc() originates from a call to zend::zend_mm_get_heap() within alloc_prof_orig_alloc() in line 376:

unsafe fn alloc_prof_orig_alloc(len: size_t) -> *mut c_void {
let heap = zend::zend_mm_get_heap();
let (prepare, restore) = tls_zend_mm_state_get!(prepare_restore_zend_heap);
let custom_heap = prepare(heap);
let ptr: *mut c_void = zend::_zend_mm_alloc(heap, len);
restore(heap, custom_heap);
ptr
}

This is weird and can only mean that "something" changed the current heap via zend_mm_set_heap() after we installed our hooks in RINIT to NULL, but I could not find any other extension that are known for these kind of things in the crash report or that si_addr being 0x0 is misleading and for whatever reason this does not mean the fault address was 0x0, but it means that the kernel when creating the signal was not able to determine the fault address.

Either way: this got me thinking if fetching the current heap via zend_mm_get_heap() is the right call anyway, because we need to forward the allocation to the heap that was alive when we installed our custom handlers for observing memory allocations and this is exactly what this PR does.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@github-actions github-actions bot added profiling Relates to the Continuous Profiler tracing labels Dec 23, 2025
@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.02%. Comparing base (ed3089e) to head (292ae88).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3547   +/-   ##
=======================================
  Coverage   62.02%   62.02%           
=======================================
  Files         140      140           
  Lines       13309    13309           
  Branches     1762     1762           
=======================================
  Hits         8255     8255           
  Misses       4265     4265           
  Partials      789      789           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed3089e...292ae88. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pr-commenter
Copy link

pr-commenter bot commented Dec 23, 2025

Benchmarks [ profiler ]

Benchmark execution time: 2026-01-16 13:23:35

Comparing candidate commit 292ae88 in PR branch florian/fix-allocs with baseline commit ed3089e in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 29 metrics, 7 unstable metrics.

@realFlowControl realFlowControl marked this pull request as ready for review January 6, 2026 19:09
@realFlowControl realFlowControl requested a review from a team as a code owner January 6, 2026 19:09
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Jan 13, 2026

⚠️ Tests

Fix all issues with Cursor

⚠️ Warnings

❄️ 3 New flaky tests detected

testAuthor from tests/Integrations/WordPress/V6_1.DDTrace\Tests\Integrations\WordPress\V6_1\PathParamsTest (Datadog) (Fix with Cursor)
DDTrace\Tests\Integrations\WordPress\V6_1\PathParamsTest::testAuthor
Failed asserting that 0 matches expected 1.

tests/Integrations/WordPress/PathParamsTestSuite.php:49
tests/Common/RetryTraitVersionGeneric.php:28
phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:106
testCategory from tests/Integrations/WordPress/V6_1.DDTrace\Tests\Integrations\WordPress\V6_1\PathParamsTest (Datadog) (Fix with Cursor)
DDTrace\Tests\Integrations\WordPress\V6_1\PathParamsTest::testCategory
Failed asserting that 0 matches expected 1.

tests/Integrations/WordPress/PathParamsTestSuite.php:35
tests/Common/RetryTraitVersionGeneric.php:28
phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:106
testPost from tests/Integrations/WordPress/V6_1.DDTrace\Tests\Integrations\WordPress\V6_1\PathParamsTest (Datadog) (Fix with Cursor)
DDTrace\Tests\Integrations\WordPress\V6_1\PathParamsTest::testPost
Failed asserting that 0 matches expected 1.

tests/Integrations/WordPress/PathParamsTestSuite.php:21
tests/Common/RetryTraitVersionGeneric.php:28
phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:106

🧪 1022 Tests failed

    testSearchPhpBinaries from integration.DDTrace\Tests\Integration\PHPInstallerTest (Fix with Cursor)

testSimplePushAndProcess from laravel-58-test.DDTrace\Tests\Integrations\Laravel\V5_8\QueueTest (Datadog) (Fix with Cursor)
Risky Test
phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:97
testSimplePushAndProcess from laravel-8x-test.DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest (Datadog) (Fix with Cursor)
DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest::testSimplePushAndProcess
Test code or tested code printed unexpected output: spanLinksTraceId: 696a3a5000000000057124337c444d3b
tid: 696a3a5000000000
hexProcessTraceId: 057124337c444d3b
hexProcessSpanId: c7a734699be70d87
processTraceId: 392134446104726843
processSpanId: 14386525162850160007
View all
This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 292ae88 | Docs | Datadog PR Page | Was this helpful? Give us feedback!

A crash report indicated that `_zend_mm_alloc` was being called with an invalid heap pointer
This invalid pointer originated from the call to `zend::zend_mm_get_heap()` within `alloc_prof_orig_alloc`.
@realFlowControl realFlowControl marked this pull request as draft January 13, 2026 14:55
@realFlowControl
Copy link
Member Author

realFlowControl commented Jan 13, 2026

Let's wait for #3560 get merged and fix the flaky profiling test first

#3560 got merged

@realFlowControl realFlowControl marked this pull request as ready for review January 14, 2026 10:28
Copy link
Collaborator

@morrisonlevi morrisonlevi left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation both here and on slack!

@realFlowControl realFlowControl merged commit e252274 into master Jan 16, 2026
2003 of 2009 checks passed
@realFlowControl realFlowControl deleted the florian/fix-allocs branch January 16, 2026 20:04
@github-actions github-actions bot added this to the 1.16.0 milestone Jan 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants