Aarch64 be patches #985
Aarch64 be patches #985
Conversation
| break; | ||
| } | ||
|
|
||
| if (slot == last_slot) | ||
| { | ||
| *slot = GINT64_TO_LE (r->val); |
| for (slot = first_slot; slot != last_slot; slot++) | ||
| { | ||
| if (GINT32_FROM_LE (*slot) == r->val) |
| break; | ||
| } | ||
|
|
||
| if (slot == last_slot) | ||
| { | ||
| *slot = GINT32_TO_LE (r->val); |
gum/gumdefs.h
Outdated
| */ | ||
| #if G_BYTE_ORDER == G_LITTLE_ENDIAN || defined (__arm__) | ||
| #if G_BYTE_ORDER == G_LITTLE_ENDIAN || defined (__arm__) || defined (__aarch64__) |
WorksButNotTested
force-pushed
the
aarch64_be-patches
branch
from
c395225 to
65be963
Compare
gum/arch-arm64/gumarm64writer.c
Outdated
| * big-endian systems), the data is in native endian. Thus since we wish to | ||
| * support writing code for big-endian systems on little-endian targets and | ||
| * vice versa, we need to check the writer configuration. | ||
| */ |
There was a problem hiding this comment.
Let's move this comment to where the data_endian field is defined. We can then avoid duplicating it further down.
gum/arch-arm64/gumarm64writer.h
Outdated
| @@ -51,6 +52,15 @@ struct _GumArm64Writer | |||
| GumMetalArray label_refs; | |||
| GumMetalArray literal_refs; | |||
| const guint32 * earliest_literal_insn; | |||
|
|
|||
| GumArm64DataEndian data_endian; | |||
There was a problem hiding this comment.
| GumArm64DataEndian data_endian; | |
| gint data_endian; |
- Let's avoid introducing a new enum here. Our Vala bindings will be using the
ByteOrderenum fromglib-2.0.vapi. - This field should be moved up. The existing three configuration values are in their own section, starting with
target_os. I'd say this one should go first, as it's more important/fundamental thantarget_os.
There was a problem hiding this comment.
Done. I kept the typedef guint GumArm64DataEndian;, was that right? Also added a comment to it for what the values should be (since the type is otherwise not very constrained).
gum/arch-arm64/gumarm64writer.c
Outdated
| @@ -220,6 +220,7 @@ gum_arm64_writer_init (GumArm64Writer * writer, | |||
| writer->label_defs = NULL; | |||
| writer->label_refs.data = NULL; | |||
| writer->literal_refs.data = NULL; | |||
| writer->data_endian = GUM_ENDIAN_NATIVE; | |||
There was a problem hiding this comment.
Assignment should go just before target_os.
gum/arch-arm64/gumarm64writer.c
Outdated
| @@ -1992,15 +1993,37 @@ gum_arm64_writer_commit_literals (GumArm64Writer * self) | |||
| if (r->width != GUM_LITERAL_64BIT) | |||
| continue; | |||
|
|
|||
| /* | |||
| * Whilst instructions in aarch64 are always in little endian (even on | |||
There was a problem hiding this comment.
| * Whilst instructions in aarch64 are always in little endian (even on | |
| * Whilst instructions in AArch64 are always in little endian (even on |
(Here and elsewhere.) For consistency with our existing comments.
gum/arch-arm64/gumarm64writer.c
Outdated
| for (slot = first_slot; slot != last_slot; slot++) | ||
| { | ||
| if (GINT64_FROM_LE (*slot) == r->val) | ||
| break; | ||
| if (self->data_endian == GUM_ENDIAN_LITTLE) |
There was a problem hiding this comment.
Is the optimizer able to avoid repeatedly reading and checking this field's value, or should we introduce a local variable used throughout the function?
There was a problem hiding this comment.
I would be very surprised if the optimizer didn't take care of it anyway, but the actual code generation part isn't hugely performance critical (to the point where a few cycles matter), its the performance of the generated code which is most important (for applications such as fuzzing).
gum/arch-arm64/gumarm64writer.c
Outdated
| @@ -213,6 +213,7 @@ gum_arm64_writer_init (GumArm64Writer * writer, | |||
| writer->ref_count = 1; | |||
| writer->flush_on_destroy = TRUE; | |||
|
|
|||
| writer->data_endian = __BYTE_ORDER__; | |||
There was a problem hiding this comment.
Should use G_BYTE_ORDER and friends here and elsewhere.
There was a problem hiding this comment.
Fixed. Apologies for the delay.
No description provided.