From: Luis Machado via Gdb-patches <gdb-patches@sourceware.org>
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: Gdb-patches <gdb-patches@sourceware.org>
Subject: Re: [PATCH 3/5] gdb: convert aarch64 to new-style debug macros
Date: Mon, 11 Jan 2021 10:18:36 -0300 [thread overview]
Message-ID: <c0e8aaa4-b600-fa46-13b1-e120a196a8ee@linaro.org> (raw)
In-Reply-To: <f5203ed3-03f8-7186-b0a6-44f97d5bf920@polymtl.ca>
Hi,
On 1/9/21 1:31 AM, Simon Marchi wrote:
> Hi Luis,
>
> Would you mind giving this patch a quick look?
>
> Thanks!
>
> Simon
>
> On 2021-01-08 11:28 p.m., Simon Marchi wrote:
>> I haven't tried this on an actual aarch64 machine, but I am able to
>> exercise it like this:
>>
>> (gdb) set debug aarch64
>> (gdb) maintenance selftest aa
>> Running selftest aarch64-analyze-prologue.
>> [aarch64] aarch64_analyze_prologue: prologue analysis gave up addr=0x14 opcode=0xf94013e0
>> Running selftest aarch64-process-record.
>> Ran 2 unit tests, 0 failed
>>
>> gdb/ChangeLog:
>>
>> * arch/aarch64-insn.h (aarch64_debug_printf): New.
>> * arch/aarch64-insn.c: Use aarch64_debug_printf.
>> * aarch64-tdep.c: Use aarch64_debug_printf.
>>
>> Change-Id: Ifdb40e2816ab8e55a9aabb066d1833d9b5a46094
>> ---
>> gdb/aarch64-tdep.c | 89 ++++++++++++++++-------------------------
>> gdb/arch/aarch64-insn.c | 9 ++---
>> gdb/arch/aarch64-insn.h | 5 +++
>> 3 files changed, 42 insertions(+), 61 deletions(-)
>>
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index 77e6ad700fcd..a51b3341e789 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -388,12 +388,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>> regs[rd] = regs[rm];
>> else
>> {
>> - if (aarch64_debug)
>> - {
>> - debug_printf ("aarch64: prologue analysis gave up "
>> - "addr=%s opcode=0x%x (orr x register)\n",
>> - core_addr_to_string_nz (start), insn);
>> - }
>> + aarch64_debug_printf ("prologue analysis gave up "
>> + "addr=%s opcode=0x%x (orr x register)",
>> + core_addr_to_string_nz (start), insn);
>> +
>> break;
>> }
>> }
>> @@ -513,10 +511,9 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>> }
>> else
>> {
>> - if (aarch64_debug)
>> - debug_printf ("aarch64: prologue analysis gave up addr=%s"
>> - " opcode=0x%x (iclass)\n",
>> - core_addr_to_string_nz (start), insn);
>> + aarch64_debug_printf ("prologue analysis gave up addr=%s"
>> + " opcode=0x%x (iclass)",
>> + core_addr_to_string_nz (start), insn);
>> break;
>> }
>>
>> @@ -527,12 +524,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>> }
>> else
>> {
>> - if (aarch64_debug)
>> - {
>> - debug_printf ("aarch64: prologue analysis gave up addr=%s"
>> - " opcode=0x%x\n",
>> - core_addr_to_string_nz (start), insn);
>> - }
>> + aarch64_debug_printf ("prologue analysis gave up addr=%s"
>> + " opcode=0x%x",
>> + core_addr_to_string_nz (start), insn);
>> +
>> break;
>> }
>> }
>> @@ -1606,12 +1601,10 @@ pass_in_x (struct gdbarch *gdbarch, struct regcache *regcache,
>> && (typecode == TYPE_CODE_STRUCT || typecode == TYPE_CODE_UNION))
>> regval <<= ((X_REGISTER_SIZE - partial_len) * TARGET_CHAR_BIT);
>>
>> - if (aarch64_debug)
>> - {
>> - debug_printf ("arg %d in %s = 0x%s\n", info->argnum,
>> - gdbarch_register_name (gdbarch, regnum),
>> - phex (regval, X_REGISTER_SIZE));
>> - }
>> + aarch64_debug_printf ("arg %d in %s = 0x%s", info->argnum,
>> + gdbarch_register_name (gdbarch, regnum),
>> + phex (regval, X_REGISTER_SIZE));
>> +
>> regcache_cooked_write_unsigned (regcache, regnum, regval);
>> len -= partial_len;
>> buf += partial_len;
>> @@ -1646,11 +1639,9 @@ pass_in_v (struct gdbarch *gdbarch,
>> memcpy (reg, buf, len);
>> regcache->cooked_write (regnum, reg);
>>
>> - if (aarch64_debug)
>> - {
>> - debug_printf ("arg %d in %s\n", info->argnum,
>> - gdbarch_register_name (gdbarch, regnum));
>> - }
>> + aarch64_debug_printf ("arg %d in %s", info->argnum,
>> + gdbarch_register_name (gdbarch, regnum));
>> +
>> return 1;
>> }
>> info->nsrn = 8;
>> @@ -1680,11 +1671,8 @@ pass_on_stack (struct aarch64_call_info *info, struct type *type,
>> if (align > 16)
>> align = 16;
>>
>> - if (aarch64_debug)
>> - {
>> - debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
>> - info->nsaa);
>> - }
>> + aarch64_debug_printf ("arg %d len=%d @ sp + %d\n", info->argnum, len,
>> + info->nsaa);
>>
>> item.len = len;
>> item.data = buf;
>> @@ -1833,13 +1821,11 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>> /* The struct_return pointer occupies X8. */
>> if (return_method != return_method_normal)
>> {
>> - if (aarch64_debug)
>> - {
>> - debug_printf ("struct return in %s = 0x%s\n",
>> - gdbarch_register_name (gdbarch,
>> - AARCH64_STRUCT_RETURN_REGNUM),
>> - paddress (gdbarch, struct_addr));
>> - }
>> + aarch64_debug_printf
>> + ("struct return in %s = 0x%s",
>> + gdbarch_register_name (gdbarch, AARCH64_STRUCT_RETURN_REGNUM),
>> + paddress (gdbarch, struct_addr));
>> +
Would it be possible to split this up differently, while still keeping
the opening parenthesis on the same line as the call?
>> regcache_cooked_write_unsigned (regcache, AARCH64_STRUCT_RETURN_REGNUM,
>> struct_addr);
>> }
>> @@ -2246,12 +2232,10 @@ aarch64_extract_return_value (struct type *type, struct regcache *regs,
>> gdb_byte buf[register_size (gdbarch, regno)];
>> gdb_assert (len <= sizeof (buf));
>>
>> - if (aarch64_debug)
>> - {
>> - debug_printf ("read HFA or HVA return value element %d from %s\n",
>> - i + 1,
>> - gdbarch_register_name (gdbarch, regno));
>> - }
>> + aarch64_debug_printf
>> + ("read HFA or HVA return value element %d from %s",
>> + i + 1, gdbarch_register_name (gdbarch, regno));
>> +
Same here.
Otherwise, this looks OK. Thanks for converting it.
>> regs->cooked_read (regno, buf);
>>
>> memcpy (valbuf, buf, len);
>> @@ -2358,12 +2342,9 @@ aarch64_store_return_value (struct type *type, struct regcache *regs,
>> gdb_byte tmpbuf[register_size (gdbarch, regno)];
>> gdb_assert (len <= sizeof (tmpbuf));
>>
>> - if (aarch64_debug)
>> - {
>> - debug_printf ("write HFA or HVA return value element %d to %s\n",
>> - i + 1,
>> - gdbarch_register_name (gdbarch, regno));
>> - }
>> + aarch64_debug_printf
>> + ("write HFA or HVA return value element %d to %s",
>> + i + 1, gdbarch_register_name (gdbarch, regno));
>>
>> memcpy (tmpbuf, valbuf,
>> len > V_REGISTER_SIZE ? V_REGISTER_SIZE : len);
>> @@ -2438,8 +2419,7 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
>> {
>> if (aarch64_return_in_memory (gdbarch, valtype))
>> {
>> - if (aarch64_debug)
>> - debug_printf ("return value in memory\n");
>> + aarch64_debug_printf ("return value in memory");
>> return RETURN_VALUE_STRUCT_CONVENTION;
>> }
>> }
>> @@ -2450,8 +2430,7 @@ aarch64_return_value (struct gdbarch *gdbarch, struct value *func_value,
>> if (readbuf)
>> aarch64_extract_return_value (valtype, regcache, readbuf);
>>
>> - if (aarch64_debug)
>> - debug_printf ("return value in registers\n");
>> + aarch64_debug_printf ("return value in registers");
>>
>> return RETURN_VALUE_REGISTER_CONVENTION;
>> }
>> diff --git a/gdb/arch/aarch64-insn.c b/gdb/arch/aarch64-insn.c
>> index 125288909137..6a5abf729bb2 100644
>> --- a/gdb/arch/aarch64-insn.c
>> +++ b/gdb/arch/aarch64-insn.c
>> @@ -69,12 +69,9 @@ aarch64_decode_adr (CORE_ADDR addr, uint32_t insn, int *is_adrp,
>> else
>> *offset = (immhi | immlo);
>>
>> - if (aarch64_debug)
>> - {
>> - debug_printf ("decode: 0x%s 0x%x %s x%u, #?\n",
>> - core_addr_to_string_nz (addr), insn,
>> - *is_adrp ? "adrp" : "adr", *rd);
>> - }
>> + aarch64_debug_printf ("decode: 0x%s 0x%x %s x%u, #?",
>> + core_addr_to_string_nz (addr), insn,
>> + *is_adrp ? "adrp" : "adr", *rd);
>> return 1;
>> }
>> return 0;
>> diff --git a/gdb/arch/aarch64-insn.h b/gdb/arch/aarch64-insn.h
>> index 57aeb23feab1..1e8c5eac940e 100644
>> --- a/gdb/arch/aarch64-insn.h
>> +++ b/gdb/arch/aarch64-insn.h
>> @@ -21,6 +21,11 @@
>>
>> extern bool aarch64_debug;
>>
>> +/* Print an "aarch64" debug statement. */
>> +
>> +#define aarch64_debug_printf(fmt, ...) \
>> + debug_prefixed_printf_cond (aarch64_debug, "aarch64", fmt, ##__VA_ARGS__)
>> +
>> /* Support routines for instruction parsing. */
>>
>> /* Create a mask of X bits. */
>>
next prev parent reply other threads:[~2021-01-11 13:18 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-09 4:28 [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi via Gdb-patches
2021-01-09 4:28 ` [PATCH 2/5] gdb: convert jit to new-style debug macros Simon Marchi via Gdb-patches
2021-01-11 21:18 ` Tom Tromey
2021-01-11 21:20 ` Simon Marchi via Gdb-patches
2021-01-09 4:28 ` [PATCH 3/5] gdb: convert aarch64 " Simon Marchi via Gdb-patches
2021-01-09 4:31 ` Simon Marchi via Gdb-patches
2021-01-11 13:18 ` Luis Machado via Gdb-patches [this message]
2021-01-11 21:30 ` Simon Marchi via Gdb-patches
2021-01-11 21:50 ` Luis Machado via Gdb-patches
2021-01-11 21:54 ` Simon Marchi via Gdb-patches
2021-01-09 4:28 ` [PATCH 4/5] gdb: convert solib-aix " Simon Marchi via Gdb-patches
2021-01-11 21:19 ` Tom Tromey
2021-01-11 21:32 ` Simon Marchi via Gdb-patches
2021-01-09 4:28 ` [PATCH 5/5] gdb: convert arc " Simon Marchi via Gdb-patches
2021-01-09 4:37 ` Simon Marchi via Gdb-patches
2021-01-13 15:35 ` Shahab Vahedi via Gdb-patches
2021-01-09 4:29 ` [PATCH 1/5] gdb: change jit_debug to a bool Simon Marchi via Gdb-patches
2021-01-11 21:17 ` Tom Tromey
2021-01-11 21:19 ` Simon Marchi via Gdb-patches
2021-01-13 14:32 ` Shahab Vahedi via Gdb-patches
2021-01-13 15:49 ` Simon Marchi via Gdb-patches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c0e8aaa4-b600-fa46-13b1-e120a196a8ee@linaro.org \
--to=gdb-patches@sourceware.org \
--cc=luis.machado@linaro.org \
--cc=simon.marchi@polymtl.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox