DynASM/arm64: support global label reference#6820
Closed
shqking wants to merge 1 commit intophp:masterfrom
Closed
Conversation
The PHP JIT has a number of stub functions generated with DynASM which
are compiled independently (i.e. dasm_link() and dasm_encode() are
called once for each stub). The LuaJIT VM from which DynASM is derived
generates the whole VM in a single pass (i.e. single call to dasm_link()
and dasm_encode()). This exposes a limitation/bug in the Arm64 port of
DynASM. For example:
| ->foo:
| nop
| ->bar:
| b ->foo
If these two snippets are compiled separately then the label "foo" is a
global relocation and the target address needs to be looked up in the
`D->globals` table. The dasm_link() step marks this by storing the
negated offset into the global labels table as the operand of the
DASM_REL_LG action (whereas it's normally a positive offset into the
local labels table). However in the arm64 port, dasm_encode() doesn't
handle negative operands for DASM_REL_LG and it fails with an "UNDEF_LG"
error. This patch handles this case by calculating the PC relative
displacement from the global label table.
One bug was found in function dasm_link(). It marks undefined global
labels by storing the negated offset into the global labels table.
However, the bias used to compute the index for global label in array
'D->lglabels' is incorrect.
'D->lglabels' stores both local labels and global ones, and the global
labels are stored after 'D->lglabels[10]'. That is, the bias for global
label indexing is 10, not 20. See function dasm_setupglobal().
I guess the authors mixed it up with 'global label number', which starts
with 20. See variable 'next_global' in file dasm_arm64.lua. Note that
the index is computed by subtracting this global label number by 10 when
acceessing array 'D->lglabels'. See the operation for action DASM_REL_LG
in function dasm_put().
Without this patch, global label reference is broken for those whose
global label number is between 20 and 29.
Example[1]:
// in function bar()
| b ->foo_0 // global label number 20
| b ->foo_9 // global label number 29
| b ->foo_10 // global label number 30
| b ->foo_13 // global label number 33
Disassembly:
function foo:
0xffff95b89000: brk #0 // label foo_0
0xffff95b89004: brk #0x1 // label foo_1
0xffff95b89008: brk #0x2
0xffff95b8900c: brk #0x3
0xffff95b89010: brk #0x4
0xffff95b89014: brk #0x5
0xffff95b89018: brk #0x6
0xffff95b8901c: brk #0x7
0xffff95b89020: brk #0x8
0xffff95b89024: brk #0x9 // label foo_9
0xffff95b89028: brk #0xa // label foo_10
0xffff95b8902c: brk #0xb
0xffff95b89030: brk #0xc
0xffff95b89034: brk #0xd // label foo_13
function bar:
0xffff95b88000: b #0xffff95b89000 // foo_0
0xffff95b88004: b #0xffff95b89024 // foo_9
0xffff95b88008: b #0xffff95b89028 // foo_10
0xffff95b8800c: b #0xffff95b89034 // foo_13
Test environment:
We're using an ARM-based server, with Ubuntu-20 and GCC-10. Disambler
library capstone[2] should be installed in advance.
After building the PHP, the 'minilua' can be found in
'PHP-SRC/ext/opcache/' directory. Our test case can be run with the
following commands:
$ PHP-SRC/ext/opcache/minilua \
PHP-SRC/ext/opcache/jit/dynasm/dynasm.lua -o test.c \
-D ARM64=1 test-glb-ref.c
$ gcc test.c -o test -lcapstone
$ ./test
[1]
https://github.com/shqking/misc/blob/main/php-dynasm-test/test-glb-ref.c
[2] https://www.capstone-engine.org/
Co-Developed-by: Nick Gasson <Nick.Gasson@arm.com>
Member
|
@MikePall this is the second AArch64 patch. |
|
Applied upstream. Thanks! |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The PHP JIT has a number of stub functions generated with DynASM which
are compiled independently (i.e. dasm_link() and dasm_encode() are
called once for each stub). The LuaJIT VM from which DynASM is derived
generates the whole VM in a single pass (i.e. single call to dasm_link()
and dasm_encode()). This exposes a limitation/bug in the Arm64 port of
DynASM. For example:
If these two snippets are compiled separately then the label "foo" is a
global relocation and the target address needs to be looked up in the
D->globalstable. The dasm_link() step marks this by storing thenegated offset into the global labels table as the operand of the
DASM_REL_LG action (whereas it's normally a positive offset into the
local labels table). However in the arm64 port, dasm_encode() doesn't
handle negative operands for DASM_REL_LG and it fails with an "UNDEF_LG"
error. This patch handles this case by calculating the PC relative
displacement from the global label table.
One bug was found in function dasm_link(). It marks undefined global
labels by storing the negated offset into the global labels table.
However, the bias used to compute the index for global label in array
'D->lglabels' is incorrect.
'D->lglabels' stores both local labels and global ones, and the global
labels are stored after 'D->lglabels[10]'. That is, the bias for global
label indexing is 10, not 20. See function dasm_setupglobal().
I guess the authors mixed it up with 'global label number', which starts
with 20. See variable 'next_global' in file dasm_arm64.lua. Note that
the index is computed by subtracting this global label number by 10 when
acceessing array 'D->lglabels'. See the operation for action DASM_REL_LG
in function dasm_put().
Without this patch, global label reference is broken for those whose
global label number is between 20 and 29.
Example[1]:
Disassembly:
Test environment:
We're using an ARM-based server, with Ubuntu-20 and GCC-10. Disambler
library capstone[2] should be installed in advance.
After building the PHP, the 'minilua' can be found in
'PHP-SRC/ext/opcache/' directory. Our test case can be run with the
following commands:
[1]
https://github.com/shqking/misc/blob/main/php-dynasm-test/test-glb-ref.c
[2] https://www.capstone-engine.org/
Co-Developed-by: Nick Gasson Nick.Gasson@arm.com