Skip to content

Fix -Wint-to-pointer-cast warnings in the CALL VM on 32-bit - #24360

Open
marc-mabe wants to merge 1 commit into
php:PHP-8.5from
marc-mabe:fix/zend-vm-enter-bit
Open

marc-mabe wants to merge 1 commit into
php:PHP-8.5from
marc-mabe:fix/zend-vm-enter-bit

Conversation

@marc-mabe

Copy link
Copy Markdown
Contributor

ZEND_VM_ENTER_BIT tags the opline returned by CALL VM handlers. It is 1ULL so that ~ZEND_VM_ENTER_BIT keeps the upper half of 64-bit pointers, but on 32-bit platforms it widens the tagged pointer expressions to 64 bits, and casting those back to a pointer warns. This breaks --enable-werror builds where no global VM registers are used, e.g. arm32, or i386 with --disable-gcc-global-regs, which also hits the copy of the definition in the JIT.

Use (uintptr_t)1 instead, which is exactly pointer-sized everywhere.

ZEND_VM_ENTER_BIT tags the opline returned by CALL VM handlers. It is
1ULL so that ~ZEND_VM_ENTER_BIT keeps the upper half of 64-bit pointers,
but on 32-bit platforms it widens the tagged pointer expressions to
64 bits, and casting those back to a pointer warns. This breaks
--enable-werror builds where no global VM registers are used, e.g. arm32,
or i386 with --disable-gcc-global-regs.

Use (uintptr_t)1 instead, which is exactly pointer-sized everywhere, also
in the copies of the definition in the JIT and the fuzzer SAPI.
@devnexen

Copy link
Copy Markdown
Member

the fix looks correct to me, two things:

  • deserve to be in 8.5 branch (if Arnaud agrees).
  • ZEND_VM_ENTER_BIT in the fuzzer sapi could be updated too if useful.

@marc-mabe
marc-mabe force-pushed the fix/zend-vm-enter-bit branch from a630057 to c88bd36 Compare October 17, 2026 11:11
@marc-mabe
marc-mabe changed the base branch from master to PHP-8.5 October 17, 2026 11:11
@marc-mabe

Copy link
Copy Markdown
Contributor Author

@devnexen You are right, I forgot the fuzzer - it's fixed now as well.
Also went ahead and rebased onto PHP-8.5. ping @arnaud-lb

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.

2 participants