Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix post bailout hook execution in 8.3 unoptimized builds #2737

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

bwoebi
Copy link
Collaborator

@bwoebi bwoebi commented Jun 28, 2024

When optimized, all is fine; however, in unoptimized builds the compiler will write stacktarget back to the stack and read it later from there, even though the stack address has moved. (i.e. only affects development builds.)

Adding register keyword to avoid this.

Description

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

When optimized, all is fine; however, in unoptimized builds the compiler will write stacktarget back to the stack and read it later from there, even though the stack address has moved. (i.e. only affects development builds.)

Adding register keyword to avoid this.
@bwoebi bwoebi requested a review from a team as a code owner June 28, 2024 11:24
@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.95%. Comparing base (44a07ba) to head (21dd318).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2737      +/-   ##
============================================
- Coverage     79.24%   77.95%   -1.29%     
  Complexity     2216     2216              
============================================
  Files           201      227      +26     
  Lines         22595    26613    +4018     
  Branches          0      988     +988     
============================================
+ Hits          17905    20747    +2842     
- Misses         4690     5340     +650     
- Partials          0      526     +526     
Flag Coverage Δ
appsec-extension 69.13% <ø> (?)
tracer-extension 78.82% <100.00%> (+<0.01%) ⬆️
tracer-php 80.54% <ø> (+0.69%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
..._abstract_interface/interceptor/php8/interceptor.c 77.42% <100.00%> (+0.05%) ⬆️

... and 32 files with indirect coverage changes


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 44a07ba...21dd318. Read the comment docs.

@pr-commenter
Copy link

pr-commenter bot commented Jun 28, 2024

Benchmarks

Benchmark execution time: 2024-06-28 11:52:07

Comparing candidate commit 21dd318 in PR branch bob/fix-83-posthook with baseline commit 44a07ba in branch master.

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

scenario:ComposerTelemetryBench/benchTelemetryParsing-opcache

  • 🟥 execution_time [+1.845µs; +4.355µs] or [+3.455%; +8.155%]

scenario:EmptyFileBench/benchEmptyFileBaseline

  • 🟥 execution_time [+73.634µs; +172.486µs] or [+3.119%; +7.306%]

scenario:EmptyFileBench/benchEmptyFileOverhead

  • 🟥 execution_time [+56.849µs; +152.651µs] or [+2.207%; +5.926%]

scenario:LaravelBench/benchLaravelBaseline

  • 🟥 execution_time [+73.931µs; +211.029µs] or [+2.974%; +8.490%]

@bwoebi bwoebi merged commit 3cdc077 into master Jul 5, 2024
695 of 703 checks passed
@bwoebi bwoebi deleted the bob/fix-83-posthook branch July 5, 2024 00:29
@github-actions github-actions bot added this to the 1.2.0 milestone Jul 5, 2024
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