[1/2] aarch64: extend decode_adrp to decode immediate offset
Commit Message
This is needed in order to figure out what an ADRP instruction is
attempting to calculate the address of, for use by a further
functionality.
gdb/
2014-06-03 Kyle McMartin <kmcmarti@redhat.com>
* aarch64-tdep.c (decode_adrp): Generalise function to handle
ADR as well as ADRP, and decode the offset.
(aarch64_analyze_prologue): Discard unneeded arguments to
decode_adrp.
---
gdb/aarch64-tdep.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
Comments
Hi,
On 3 June 2014 06:01, Kyle McMartin <kmcmarti@redhat.com> wrote:
> @@ -264,16 +264,28 @@ decode_add_sub_imm (CORE_ADDR addr, uint32_t insn, unsigned *rd, unsigned *rn,
> Return 1 if the opcodes matches and is decoded, otherwise 0. */
>
> static int
> -decode_adrp (CORE_ADDR addr, uint32_t insn, unsigned *rd)
> +decode_adrp (CORE_ADDR addr, uint32_t insn, int *page, unsigned *rd,
> + int64_t *imm)
Given that this now decodes both adrp and adr the function name seems
inappropriate, how about following the convention used in the other
decode_ functions and changing to something like decode_adrp_adr ()..
?
Returning both 'page' and 'imm' isn't necessary and I don't think it
makes sense given that logically both adr and adrp are used to
construct an address. Looking at this patch and the following patch I
think you can achieve the same functionality by having the decodes of
both adrp and adr return a value in 'imm'. In the case of an adrp
decode just returning: imm = page << 12 ....
Cheers
/Marcus
On Tue, Jun 03, 2014 at 09:22:28AM +0100, Marcus Shawcroft wrote:
> Hi,
>
> On 3 June 2014 06:01, Kyle McMartin <kmcmarti@redhat.com> wrote:
>
> > @@ -264,16 +264,28 @@ decode_add_sub_imm (CORE_ADDR addr, uint32_t insn, unsigned *rd, unsigned *rn,
> > Return 1 if the opcodes matches and is decoded, otherwise 0. */
> >
> > static int
> > -decode_adrp (CORE_ADDR addr, uint32_t insn, unsigned *rd)
> > +decode_adrp (CORE_ADDR addr, uint32_t insn, int *page, unsigned *rd,
> > + int64_t *imm)
>
> Given that this now decodes both adrp and adr the function name seems
> inappropriate, how about following the convention used in the other
> decode_ functions and changing to something like decode_adrp_adr ()..
> ?
>
Sure, sounds good.
> Returning both 'page' and 'imm' isn't necessary and I don't think it
> makes sense given that logically both adr and adrp are used to
> construct an address. Looking at this patch and the following patch I
> think you can achieve the same functionality by having the decodes of
> both adrp and adr return a value in 'imm'. In the case of an adrp
> decode just returning: imm = page << 12 ....
>
Hmm, I was trying to avoid doing the actual computation inside it. I'm
happy one way or another though.
--Kyle
> Cheers
> /Marcus
@@ -255,7 +255,7 @@ decode_add_sub_imm (CORE_ADDR addr, uint32_t insn, unsigned *rd, unsigned *rn,
return 0;
}
-/* Decode an opcode if it represents an ADRP instruction.
+/* Decode an opcode if it represents an ADRP/ADR instruction.
ADDR specifies the address of the opcode.
INSN specifies the opcode to test.
@@ -264,16 +264,28 @@ decode_add_sub_imm (CORE_ADDR addr, uint32_t insn, unsigned *rd, unsigned *rn,
Return 1 if the opcodes matches and is decoded, otherwise 0. */
static int
-decode_adrp (CORE_ADDR addr, uint32_t insn, unsigned *rd)
+decode_adrp (CORE_ADDR addr, uint32_t insn, int *page, unsigned *rd,
+ int64_t *imm)
{
- if (decode_masked_match (insn, 0x9f000000, 0x90000000))
+ if (decode_masked_match (insn, 0x1f000000, 0x10000000))
{
*rd = (insn >> 0) & 0x1f;
+ *imm = (extract_signed_bitfield (insn, 19, 5) << 2)
+ | ((insn >> 29) & 0x3);
+ *page = 0;
+
+ if (insn & 0x80000000)
+ {
+ *page = 1;
+ *imm <<= 12;
+ }
if (aarch64_debug)
fprintf_unfiltered (gdb_stdlog,
- "decode: 0x%s 0x%x adrp x%u, #?\n",
- core_addr_to_string_nz (addr), insn, *rd);
+ "decode: 0x%s 0x%x %s x%u, #0x%llx\n",
+ core_addr_to_string_nz (addr), insn,
+ *page ? "adrp" : "adr", *rd,
+ (unsigned long long)*imm);
return 1;
}
return 0;
@@ -681,8 +693,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
unsigned rt2;
int op_is_sub;
int32_t imm;
+ int64_t imm64;
unsigned cond;
int is64;
+ int page;
unsigned is_link;
unsigned op;
unsigned bit;
@@ -692,7 +706,7 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
if (decode_add_sub_imm (start, insn, &rd, &rn, &imm))
regs[rd] = pv_add_constant (regs[rn], imm);
- else if (decode_adrp (start, insn, &rd))
+ else if (decode_adrp (start, insn, &page, &rd, &imm64))
regs[rd] = pv_unknown ();
else if (decode_b (start, insn, &is_link, &offset))
{