Skip to content

ext/lexbor: Enable ASAN support in Lexbor#22826

Open
kocsismate wants to merge 5 commits into
php:masterfrom
kocsismate:lexbor-asan
Open

ext/lexbor: Enable ASAN support in Lexbor#22826
kocsismate wants to merge 5 commits into
php:masterfrom
kocsismate:lexbor-asan

Conversation

@kocsismate

@kocsismate kocsismate commented Jul 20, 2026

Copy link
Copy Markdown
Member

No description provided.

@kocsismate
kocsismate requested a review from ndossche July 20, 2026 12:00
@kocsismate kocsismate mentioned this pull request Jul 20, 2026
Open
ndossche
ndossche requested changes Jul 20, 2026
View reviewed changes

@ndossche ndossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't fully work, right?
For example in core/dobject.c in lexbor, it check for LEXBOR_HAVE_ADDRESS_SANITIZER but LEXBOR_HAVE_ADDRESS_SANITIZER is only defined in php_config.h...
I just tried this hypothesis by putting a syntax error under that define and PHP still compiled fine, so that indicates it indeed isn't being picked up there

@kocsismate

kocsismate commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

Yeah, sorry for the premature PR, I also tested the change (after I opened the PR), and it indeed didn't work that way... But now we'll have a proof that it works: ext/uri/tests/051.phpt will fail. I don't have capacity today to look into the root cause of the issue, so I'll continue tomorrow.

@kocsismate
kocsismate requested a review from petk July 20, 2026 20:11
@kocsismate
kocsismate requested a review from TimWolla as a code owner July 21, 2026 21:40
@kocsismate
kocsismate requested a review from iluuu1994 July 21, 2026 21:42
iluuu1994
iluuu1994 reviewed Jul 21, 2026
View reviewed changes
Comment thread .github/workflows/test-suite.yml Outdated
CFLAGS="-fsanitize=undefined,address -fno-sanitize=function -DZEND_TRACK_ARENA_ALLOC"
LDFLAGS="-fsanitize=undefined,address -fno-sanitize=function"
CFLAGS="-fno-sanitize=function -DZEND_TRACK_ARENA_ALLOC"
LDFLAGS="-fno-sanitize=function"

@iluuu1994 iluuu1994 Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is intentional. See #18535 / #20992.

@kocsismate kocsismate Jul 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, thanks for the info.. I didn't understand why not the --enable-*-sanitizer flags were used.

@kocsismate kocsismate Jul 22, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Desperate times call for desperate measures: https://github.com/php/php-src/pull/22826/changes#diff-8f50e816adac6afb8f364de04db3ca061f2e64f92834d17dc0ceec2a0da9cc21R6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iluuu1994 iluuu1994 iluuu1994 left review comments
@ndossche ndossche ndossche requested changes
@petk petk Awaiting requested review from petk
@TimWolla TimWolla Awaiting requested review from TimWolla
@devnexen devnexen Awaiting requested review from devnexen
@LamentXU123 LamentXU123 Awaiting requested review from LamentXU123

Assignees

No one assigned

Labels

Category: Build System Category: CI Extension: intl

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kocsismate @iluuu1994 @ndossche