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

i#6662 regdeps ISA: virtual registers #6783

Merged
merged 22 commits into from
Apr 24, 2024
Merged

i#6662 regdeps ISA: virtual registers #6783

merged 22 commits into from
Apr 24, 2024

Conversation

edeiana
Copy link
Contributor

@edeiana edeiana commented Apr 18, 2024

Containing-register IDs can be >=256, hence their value does not fit
in the allotted 8 bits per register operand of regdeps encoding.
This was causing a memory corruption in instr_convert_to_isa_regdeps()
where src_reg_used and dst_reg_used have only 256 elements and are
laid out next to each other in memory. Writing to index >=256 into one was
overwriting the other. Fix: remap containing-register IDs to virtual-register
IDs starting from 0 for all architectures. We still have only up to 198 unique
containing registers (max number of containing registers for AARCH64),
so remapping allows to fit them in 8 bits.

In the re-mapping (from DR_REG_ to DR_REG_V) we exclude DR_REG_INVALID
and DR_REG_NULL to avoid issues with opnd_t operations for registers.

We introduce a private routine dr_reg_to_virtual() to do the mapping from real ISA
to virtual register. We use it in instr_convert_to_isa_regdeps() to avoid the issue
mentioned above.

We modified the get_register_name() public API to use the global dcontext and its
ISA mode to determine whether to return a real register name or a virtual one.
The signature of the API remained the same, but we document the use of the global
dcontext in doxygen.

We also re-introduce setting the size for register operands in
instr_convert_to_isa_reg_deps() and decode_isa_regdeps() as
instr_t.operation_size because not all DR_REG_V have a predefined size
based on their enum value (e.g., reserved DR_REG_XMM enum values).

We added tests to check that DR_REG_ with IDs >=256 don't cause problems.

Issue: #6662

Containing-register IDs can be >=256, hence their value does not fit
in the allotted 8 bits per register operand of regdeps encoding.
This was causing a memory corruption in instr_convert_to_isa_regdeps()
where src_reg_used and dst_reg_used have only 256 elements and are
laid out next to each other in memory. Writing to index >=256 into one
was overwriting the other. Fix: remap containing-register IDs to
virtual-register IDs starting from 0 for all architectures.
We still have only up to 198 unique containing registers (max number
of containing registers for AARCH64), so remapping them allows to fit
them in 8 bits.

In the re-mapping (from DR_REG_ to DR_REG_V) we exclude DR_REG_INVALID
to avoid issues with opnd_t operations for registers.

We introduce 2 new public APIs: dr_reg_to_virtual() and
get_virtual_register_name(). We use dr_reg_to_virtual() in
instr_convert_to_isa_regdeps() to avoid the issue mentioned above.

We also re-introduce setting the size for register operands in
instr_convert_to_isa_reg_deps() and decode_isa_regdeps() as
instr_t.operation_size because DR_REG_V don't have predefined size.

We added tests to check that DR_REG_ with IDs >=256 don't cause problems.

Issue: #6662
@edeiana edeiana marked this pull request as ready for review April 18, 2024 16:06
@edeiana edeiana requested a review from derekbruening April 18, 2024 16:46
core/ir/aarch64/encode.c Outdated Show resolved Hide resolved
core/ir/aarch64/encode.c Outdated Show resolved Hide resolved
core/ir/aarch64/encode.c Outdated Show resolved Hide resolved
core/ir/aarch64/encode.c Outdated Show resolved Hide resolved
core/ir/aarch64/encode.c Outdated Show resolved Hide resolved
suite/tests/api/ir_regdeps.c Outdated Show resolved Hide resolved
suite/tests/api/ir_regdeps.c Outdated Show resolved Hide resolved
suite/tests/api/ir_regdeps.c Outdated Show resolved Hide resolved
suite/tests/api/ir_regdeps.c Outdated Show resolved Hide resolved
suite/tests/api/ir_regdeps.c Outdated Show resolved Hide resolved
@edeiana edeiana requested a review from derekbruening April 19, 2024 06:16
api/docs/release.dox Outdated Show resolved Hide resolved
api/docs/release.dox Outdated Show resolved Hide resolved
core/ir/aarch64/encode.c Show resolved Hide resolved
core/ir/arm/encode.c Show resolved Hide resolved
core/ir/instr_shared.c Outdated Show resolved Hide resolved
core/ir/isa_regdeps/decode.c Outdated Show resolved Hide resolved
core/ir/opnd_api.h Outdated Show resolved Hide resolved
core/ir/opnd_api.h Show resolved Hide resolved
core/ir/opnd_api.h Show resolved Hide resolved
core/ir/x86/encode.c Show resolved Hide resolved
edeiana added 6 commits April 22, 2024 14:25
Now using global dcontext_t in existing get_register_name() API
to determine whether we want to return a virtual register name
or a real register name.
Added doxygen comment to get_register_name() to document this change.
core/arch/arch.c Outdated Show resolved Hide resolved
core/ir/opnd_shared.c Outdated Show resolved Hide resolved
@edeiana edeiana requested a review from derekbruening April 23, 2024 00:53
core/ir/opnd_shared.c Outdated Show resolved Hide resolved
core/ir/opnd_api.h Outdated Show resolved Hide resolved
core/ir/isa_regdeps/decode.c Outdated Show resolved Hide resolved
core/ir/instr_shared.c Outdated Show resolved Hide resolved
api/docs/release.dox Show resolved Hide resolved
core/arch/arch.c Outdated Show resolved Hide resolved
@edeiana edeiana merged commit 092b4f2 into master Apr 24, 2024
16 checks passed
@edeiana edeiana deleted the i6662-virtual-regs branch April 24, 2024 19:44
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.

2 participants