From: Luis Machado <luis.machado@arm.com>
To: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb: fix auxv caching
Date: Mon, 10 Oct 2022 10:33:16 +0100 [thread overview]
Message-ID: <d4382b69-1ad5-a7e9-bf2f-91a593568c03@arm.com> (raw)
In-Reply-To: <20221007204440.3041413-1-simon.marchi@polymtl.ca>
On 10/7/22 21:44, Simon Marchi wrote:
> There's a flaw in the interaction of the auxv caching and the fact that
> target_auxv_search allows reading auxv from an arbitrary target_ops
> (passed in as a parameter). This has consequences as explained in this
> thread:
>
> https://inbox.sourceware.org/gdb-patches/20220719144542.1478037-1-luis.machado@arm.com/
>
> In summary, when loading an AArch64 core file with MTE support by
> passing the executable and core file names directly to GDB, we see the
> MTE info:
>
> $ ./gdb -nx --data-directory=data-directory -q aarch64-mte-gcore aarch64-mte-gcore.core
> ...
> Program terminated with signal SIGSEGV, Segmentation fault
> Memory tag violation while accessing address 0x0000ffff8ef5e000
> Allocation tag 0x1
> Logical tag 0x0.
> #0 0x0000aaaade3d0b4c in ?? ()
> (gdb)
>
> But if we do it as two separate commands (file and core) we don't:
>
> $ ./gdb -nx --data-directory=data-directory -q -ex "file aarch64-mte-gcore" -ex "core aarch64-mte-gcore.core"
> ...
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0 0x0000aaaade3d0b4c in ?? ()
> (gdb)
>
> The problem with the latter is that auxv data gets improperly cached
> between the two commands. When executing the file command, auxv gets
> first queried here, when loading the executable:
>
> #0 target_auxv_search (ops=0x55555b842400 <exec_ops>, match=0x9, valp=0x7fffffffc5d0) at /home/simark/src/binutils-gdb/gdb/auxv.c:383
> #1 0x0000555557e576f2 in svr4_exec_displacement (displacementp=0x7fffffffc8c0) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2482
> #2 0x0000555557e594d1 in svr4_relocate_main_executable () at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2878
> #3 0x0000555557e5989e in svr4_solib_create_inferior_hook (from_tty=1) at /home/simark/src/binutils-gdb/gdb/solib-svr4.c:2933
> #4 0x0000555557e6e49f in solib_create_inferior_hook (from_tty=1) at /home/simark/src/binutils-gdb/gdb/solib.c:1253
> #5 0x0000555557f33e29 in symbol_file_command (args=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1) at /home/simark/src/binutils-gdb/gdb/symfile.c:1655
> #6 0x00005555573319c3 in file_command (arg=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1) at /home/simark/src/binutils-gdb/gdb/exec.c:555
> #7 0x0000555556e47185 in do_simple_func (args=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1, c=0x612000047740) at /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:95
> #8 0x0000555556e551c9 in cmd_func (cmd=0x612000047740, args=0x7fffffffe01c "aarch64-mte-gcore", from_tty=1) at /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:2543
> #9 0x00005555580e63fd in execute_command (p=0x7fffffffe02c "e", from_tty=1) at /home/simark/src/binutils-gdb/gdb/top.c:692
> #10 0x0000555557771913 in catch_command_errors (command=0x5555580e55ad <execute_command(char const*, int)>, arg=0x7fffffffe017 "file aarch64-mte-gcore", from_tty=1, do_bp_actions=true) at /home/simark/src/binutils-gdb/gdb/main.c:513
> #11 0x0000555557771fba in execute_cmdargs (cmdarg_vec=0x7fffffffd570, file_type=CMDARG_FILE, cmd_type=CMDARG_COMMAND, ret=0x7fffffffd230) at /home/simark/src/binutils-gdb/gdb/main.c:608
> #12 0x00005555577755ac in captured_main_1 (context=0x7fffffffda10) at /home/simark/src/binutils-gdb/gdb/main.c:1299
> #13 0x0000555557775c2d in captured_main (data=0x7fffffffda10) at /home/simark/src/binutils-gdb/gdb/main.c:1320
> #14 0x0000555557775cc2 in gdb_main (args=0x7fffffffda10) at /home/simark/src/binutils-gdb/gdb/main.c:1345
> #15 0x00005555568bdcbe in main (argc=10, argv=0x7fffffffdba8) at /home/simark/src/binutils-gdb/gdb/gdb.c:32
>
> Here, target_auxv_search is called on the inferior's target stack. The
> target stack only contains the exec target, so the query returns empty
> auxv data. This gets cached for that inferior in `auxv_inferior_data`.
>
> In its constructor (before it is pushed to the inferior's target stack),
> the core_target needs to identify the right target description from the
> core, and for that asks the gdbarch to read a target description from
> the core file. Because some implementations of
> gdbarch_core_read_description (such as AArch64's) need to read auxv data
> from the core in order to determine the right target description, the
> core_target passes a pointer to itself, allowing implementations to call
> target_auxv_search it. However, because we have previously cached
> (empty) auxv data for that inferior, target_auxv_search searched that
> cached (empty) auxv data, not auxv data read from the core. Remember
> that this data was obtained by reading auxv on the inferior's target
> stack, which only contained an exec target.
>
> The problem I see is that while target_auxv_search offers the
> flexibility of reading from an arbitrary (passed as an argument) target,
> the caching doesn't do the distinction of which target is being queried,
> and where the cached data came from. So, you could read auxv from a
> target A, it gets cached, then you try to read auxv from a target B, and
> it returns the cached data from target A. That sounds wrong. In our
> case, we expect to read different auxv data from the core target than
> what we have read from the target stack earlier, so it doesn't make
> sense to hit the cache in this case.
>
> To fix this, I propose splitting the code paths that read auxv data from
> an inferior's target stack and those that read from a passed-in target.
> The code path that reads from the target stack will keep caching,
> whereas the one that reads from a passed-in target won't. And since,
> searching in auxv data is independent from where this data came from,
> split the "read" part from the "search" part.
>
> From what I understand, auxv caching was introduced mostly to reduce
> latency on remote connections, when doing many queries. With the change
> I propose, only the queries done while constructing the core_target
> end up not using cached auxv data. This is fine, because there are just
> a handful of queries max, done at this point, and reading core files is
> local.
>
> The changes to auxv functions are:
>
> - Introduce 2 target_read_auxv functions. One reads from an explicit
> target_ops and doesn't do caching (to be used in
> gdbarch_core_read_description context). The other takes no argument,
> reads from the current inferior's target stack (it looks just like a
> standard target function wrapper) and does caching.
>
> The first target_read_auxv actually replaces get_auxv_inferior_data,
> since it became a trivial wrapper around it.
>
> - Change the existing target_auxv_search to not read auxv data from the
> target, but to accept it as a parameter (a gdb::byte_vector). This
> function doesn't care where the data came from, it just searches in
> it. It still needs to take a target_ops and gdbarch to know how to
> parse auxv entries.
>
> - Add a convenience target_auxv_search overload that reads auxv
> data from the inferior's target stack and searches in it. This
> overload is useful to replace the exist target_auxv_search calls that
> passed the `current_inferior ()->top_target ()` target and keep the
> call sites short.
>
> - Modify parse_auxv to accept a target_ops and gdbarch to use for
> parsing entries. Not strictly related to the rest of this change,
> but it seems like a good change in the context.
>
> Changes in architecture-specific files (tdep and nat):
>
> - In linux-tdep, linux_get_hwcap and linux_get_hwcap2 get split in two,
> similar to target_auxv_search. One version receives auxv data,
> target and arch as parameters. The other gets everything from the
> current inferior. The latter is for convenience, to avoid making
> call sites too ugly.
>
> - Call sites of linux_get_hwcap and linux_get_hwcap2 are adjusted to
> use either of the new versions. The call sites in
> gdbarch_core_read_description context explicitly read auxv data from
> the passed-in target and call the linux_get_hwcap{,2} function with
> parameters. Other call sites use the versions without parameters.
>
> - Same idea for arm_fbsd_read_description_auxv.
>
> - Call sites of target_auxv_search that passed
> `current_inferior ()->top_target ()` are changed to use the
> target_auxv_search overload that works in the current inferior.
>
> Change-Id: Ib775a220cf1e76443fb7da2fdff8fc631128fe66
> ---
> gdb/aarch64-linux-nat.c | 6 +--
> gdb/aarch64-linux-tdep.c | 6 ++-
> gdb/arm-fbsd-nat.c | 2 +-
> gdb/arm-fbsd-tdep.c | 25 ++++++++---
> gdb/arm-fbsd-tdep.h | 12 ++++-
> gdb/arm-linux-nat.c | 2 +-
> gdb/arm-linux-tdep.c | 3 +-
> gdb/auxv.c | 94 ++++++++++++++++++++++------------------
> gdb/auxv.h | 22 +++++++++-
> gdb/elfread.c | 2 +-
> gdb/fbsd-tdep.c | 7 ++-
> gdb/linux-tdep.c | 58 ++++++++++++++++++-------
> gdb/linux-tdep.h | 26 ++++++++---
> gdb/ppc-linux-nat.c | 19 +++-----
> gdb/ppc-linux-tdep.c | 3 +-
> gdb/rs6000-tdep.c | 3 +-
> gdb/s390-linux-nat.c | 2 +-
> gdb/s390-linux-tdep.c | 3 +-
> gdb/solib-svr4.c | 15 +++----
> gdb/sparc64-tdep.c | 6 +--
> 20 files changed, 200 insertions(+), 116 deletions(-)
>
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index eda79ec6d35c..caefcb364852 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -781,8 +781,8 @@ aarch64_linux_nat_target::read_description ()
> if (ret == 0)
> return aarch32_read_description ();
>
> - CORE_ADDR hwcap = linux_get_hwcap (this);
> - CORE_ADDR hwcap2 = linux_get_hwcap2 (this);
> + CORE_ADDR hwcap = linux_get_hwcap ();
> + CORE_ADDR hwcap2 = linux_get_hwcap2 ();
>
> aarch64_features features;
> features.vq = aarch64_sve_get_vq (tid);
> @@ -918,7 +918,7 @@ aarch64_linux_nat_target::thread_architecture (ptid_t ptid)
> bool
> aarch64_linux_nat_target::supports_memory_tagging ()
> {
> - return (linux_get_hwcap2 (this) & HWCAP2_MTE) != 0;
> + return (linux_get_hwcap2 () & HWCAP2_MTE) != 0;
> }
>
> /* Implement the "fetch_memtags" target_ops method. */
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 15773c75da83..9ce3569a277c 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -33,6 +33,7 @@
> #include "target.h"
> #include "target/target.h"
> #include "expop.h"
> +#include "auxv.h"
>
> #include "regcache.h"
> #include "regset.h"
> @@ -779,8 +780,9 @@ aarch64_linux_core_read_description (struct gdbarch *gdbarch,
> struct target_ops *target, bfd *abfd)
> {
> asection *tls = bfd_get_section_by_name (abfd, ".reg-aarch-tls");
> - CORE_ADDR hwcap = linux_get_hwcap (target);
> - CORE_ADDR hwcap2 = linux_get_hwcap2 (target);
> + gdb::optional<gdb::byte_vector> auxv = target_read_auxv (target);
> + CORE_ADDR hwcap = linux_get_hwcap (auxv, target, gdbarch);
> + CORE_ADDR hwcap2 = linux_get_hwcap2 (auxv, target, gdbarch);
>
> aarch64_features features;
> features.vq = aarch64_linux_core_read_vq (gdbarch, abfd);
> diff --git a/gdb/arm-fbsd-nat.c b/gdb/arm-fbsd-nat.c
> index b161b7ed9080..bbd722ed9230 100644
> --- a/gdb/arm-fbsd-nat.c
> +++ b/gdb/arm-fbsd-nat.c
> @@ -122,7 +122,7 @@ arm_fbsd_nat_target::read_description ()
> #ifdef PT_GETREGSET
> tls = have_regset (inferior_ptid, NT_ARM_TLS) != 0;
> #endif
> - desc = arm_fbsd_read_description_auxv (this, tls);
> + desc = arm_fbsd_read_description_auxv (tls);
> if (desc == NULL)
> desc = this->beneath ()->read_description ();
> return desc;
> diff --git a/gdb/arm-fbsd-tdep.c b/gdb/arm-fbsd-tdep.c
> index 61c8f0cecad4..8944f7e9b758 100644
> --- a/gdb/arm-fbsd-tdep.c
> +++ b/gdb/arm-fbsd-tdep.c
> @@ -190,15 +190,17 @@ arm_fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
> &arm_fbsd_vfpregset, "VFP floating-point", cb_data);
> }
>
> -/* Lookup a target description from a target's AT_HWCAP auxiliary
> - vector. */
> +/* See arm-fbsd-tdep.h. */
>
> const struct target_desc *
> -arm_fbsd_read_description_auxv (struct target_ops *target, bool tls)
> +arm_fbsd_read_description_auxv (const gdb::optional<gdb::byte_vector> &auxv,
> + target_ops *target, gdbarch *gdbarch, bool tls)
> {
> CORE_ADDR arm_hwcap = 0;
>
> - if (target_auxv_search (target, AT_FREEBSD_HWCAP, &arm_hwcap) != 1)
> + if (!auxv.has_value ()
> + || target_auxv_search (*auxv, target, gdbarch, AT_FREEBSD_HWCAP,
> + &arm_hwcap) != 1)
> return arm_read_description (ARM_FP_TYPE_NONE, tls);
>
> if (arm_hwcap & HWCAP_VFP)
> @@ -215,6 +217,18 @@ arm_fbsd_read_description_auxv (struct target_ops *target, bool tls)
> return arm_read_description (ARM_FP_TYPE_NONE, tls);
> }
>
> +/* See arm-fbsd-tdep.h. */
> +
> +const struct target_desc *
> +arm_fbsd_read_description_auxv (bool tls)
> +{
> + gdb::optional<gdb::byte_vector> auxv = target_read_auxv ();
> + return arm_fbsd_read_description_auxv (auxv,
> + current_inferior ()->top_target (),
> + current_inferior ()->gdbarch,
> + tls);
> +}
> +
> /* Implement the "core_read_description" gdbarch method. */
>
> static const struct target_desc *
> @@ -224,7 +238,8 @@ arm_fbsd_core_read_description (struct gdbarch *gdbarch,
> {
> asection *tls = bfd_get_section_by_name (abfd, ".reg-aarch-tls");
>
> - return arm_fbsd_read_description_auxv (target, tls != nullptr);
> + gdb::optional<gdb::byte_vector> auxv = target_read_auxv (target);
> + return arm_fbsd_read_description_auxv (auxv, target, gdbarch, tls != nullptr);
> }
>
> /* Implement the get_thread_local_address gdbarch method. */
> diff --git a/gdb/arm-fbsd-tdep.h b/gdb/arm-fbsd-tdep.h
> index 193eb76df3c9..85d7b59d1362 100644
> --- a/gdb/arm-fbsd-tdep.h
> +++ b/gdb/arm-fbsd-tdep.h
> @@ -42,7 +42,17 @@ extern const struct regset arm_fbsd_vfpregset;
> #define HWCAP_VFPv3 0x00002000
> #define HWCAP_VFPD32 0x00080000
>
> +/* Lookup a target description based on the AT_HWCAP value in the auxv data
> + AUXV. */
> +
> +extern const struct target_desc *
> + arm_fbsd_read_description_auxv (const gdb::optional<gdb::byte_vector> &auxv,
> + target_ops *target, gdbarch *gdbarch,
> + bool tls);
> +
> +/* Same as the above, but read the auxv data from the current inferior. */
> +
> extern const struct target_desc *
> -arm_fbsd_read_description_auxv (struct target_ops *target, bool tls);
> + arm_fbsd_read_description_auxv (bool tls);
>
> #endif /* ARM_FBSD_TDEP_H */
> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
> index 0188c78fe7a0..a8b582fbef32 100644
> --- a/gdb/arm-linux-nat.c
> +++ b/gdb/arm-linux-nat.c
> @@ -531,7 +531,7 @@ ps_get_thread_area (struct ps_prochandle *ph,
> const struct target_desc *
> arm_linux_nat_target::read_description ()
> {
> - CORE_ADDR arm_hwcap = linux_get_hwcap (this);
> + CORE_ADDR arm_hwcap = linux_get_hwcap ();
>
> if (have_ptrace_getregset == TRIBOOL_UNKNOWN)
> {
> diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
> index 1feb69fe6dd1..9db52de1a5f5 100644
> --- a/gdb/arm-linux-tdep.c
> +++ b/gdb/arm-linux-tdep.c
> @@ -732,7 +732,8 @@ arm_linux_core_read_description (struct gdbarch *gdbarch,
> struct target_ops *target,
> bfd *abfd)
> {
> - CORE_ADDR arm_hwcap = linux_get_hwcap (target);
> + gdb::optional<gdb::byte_vector> auxv = target_read_auxv (target);
> + CORE_ADDR arm_hwcap = linux_get_hwcap (auxv, target, gdbarch);
>
> if (arm_hwcap & HWCAP_VFP)
> {
> diff --git a/gdb/auxv.c b/gdb/auxv.c
> index 76fc821c07c3..5853437b0f24 100644
> --- a/gdb/auxv.c
> +++ b/gdb/auxv.c
> @@ -307,23 +307,21 @@ svr4_auxv_parse (struct gdbarch *gdbarch, const gdb_byte **readptr,
>
> /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
>
> - Use the auxv_parse method from the current inferior's gdbarch, if defined,
> - else use the current inferior's target stack's auxv_parse.
> + Use the auxv_parse method from GDBARCH, if defined, else use the auxv_parse
> + method of OPS.
>
> Return 0 if *READPTR is already at the end of the buffer.
> Return -1 if there is insufficient buffer for a whole entry.
> Return 1 if an entry was read into *TYPEP and *VALP. */
> +
> static int
> -parse_auxv (const gdb_byte **readptr, const gdb_byte *endptr, CORE_ADDR *typep,
> - CORE_ADDR *valp)
> +parse_auxv (target_ops *ops, gdbarch *gdbarch, const gdb_byte **readptr,
> + const gdb_byte *endptr, CORE_ADDR *typep, CORE_ADDR *valp)
> {
> - struct gdbarch *gdbarch = target_gdbarch();
> -
> if (gdbarch_auxv_parse_p (gdbarch))
> return gdbarch_auxv_parse (gdbarch, readptr, endptr, typep, valp);
>
> - return current_inferior ()->top_target ()->auxv_parse (readptr, endptr,
> - typep, valp);
> + return ops->auxv_parse (readptr, endptr, typep, valp);
> }
>
>
> @@ -354,45 +352,45 @@ invalidate_auxv_cache (void)
> invalidate_auxv_cache_inf (current_inferior ());
> }
>
> -/* Fetch the auxv object from inferior INF. If auxv is cached already,
> - return a pointer to the cache. If not, fetch the auxv object from the
> - target and cache it. This function always returns a valid INFO pointer. */
> +/* See auxv.h. */
>
> -static struct auxv_info *
> -get_auxv_inferior_data (struct target_ops *ops)
> +gdb::optional<gdb::byte_vector>
> +target_read_auxv ()
> {
> - struct auxv_info *info;
> - struct inferior *inf = current_inferior ();
> + inferior *inf = current_inferior ();
> + auxv_info *info = auxv_inferior_data.get (inf);
>
> - info = auxv_inferior_data.get (inf);
> - if (info == NULL)
> + if (info == nullptr)
> {
> info = auxv_inferior_data.emplace (inf);
> - info->data = target_read_alloc (ops, TARGET_OBJECT_AUXV, NULL);
> + info->data = target_read_alloc (inf->top_target (), TARGET_OBJECT_AUXV,
> + nullptr);
> }
>
> - return info;
> + return info->data;
> }
>
> -/* Extract the auxiliary vector entry with a_type matching MATCH.
> - Return zero if no such entry was found, or -1 if there was
> - an error getting the information. On success, return 1 after
> - storing the entry's value field in *VALP. */
> -int
> -target_auxv_search (struct target_ops *ops, CORE_ADDR match, CORE_ADDR *valp)
> +/* See auxv.h. */
> +
> +gdb::optional<gdb::byte_vector>
> +target_read_auxv (target_ops *ops)
> {
> - CORE_ADDR type, val;
> - auxv_info *info = get_auxv_inferior_data (ops);
> + return target_read_alloc (ops, TARGET_OBJECT_AUXV, NULL);
> +}
>
> - if (!info->data)
> - return -1;
> +/* See auxv.h. */
>
> - const gdb_byte *data = info->data->data ();
> +int
> +target_auxv_search (const gdb::byte_vector &auxv, target_ops *ops,
> + gdbarch *gdbarch, CORE_ADDR match, CORE_ADDR *valp)
> +{
> + CORE_ADDR type, val;
> + const gdb_byte *data = auxv.data ();
> const gdb_byte *ptr = data;
> - size_t len = info->data->size ();
> + size_t len = auxv.size ();
>
> while (1)
> - switch (parse_auxv (&ptr, data + len, &type, &val))
> + switch (parse_auxv (ops, gdbarch, &ptr, data + len, &type, &val))
> {
> case 1: /* Here's an entry, check it. */
> if (type == match)
> @@ -406,10 +404,21 @@ target_auxv_search (struct target_ops *ops, CORE_ADDR match, CORE_ADDR *valp)
> default: /* Bogosity. */
> return -1;
> }
> -
> - /*NOTREACHED*/
> }
>
> +/* See auxv.h. */
> +
> +int
> +target_auxv_search (CORE_ADDR match, CORE_ADDR *valp)
> +{
> + gdb::optional<gdb::byte_vector> auxv = target_read_auxv ();
> +
> + if (!auxv.has_value ())
> + return -1;
> +
> + return target_auxv_search (*auxv, current_inferior ()->top_target (),
> + current_inferior ()->gdbarch, match, valp);
> +}
>
> /* Print the description of a single AUXV entry on the specified file. */
>
> @@ -551,21 +560,23 @@ default_print_auxv_entry (struct gdbarch *gdbarch, struct ui_file *file,
> /* Print the contents of the target's AUXV on the specified file. */
>
> static int
> -fprint_target_auxv (struct ui_file *file, struct target_ops *ops)
> +fprint_target_auxv (struct ui_file *file)
> {
> struct gdbarch *gdbarch = target_gdbarch ();
> CORE_ADDR type, val;
> int ents = 0;
> - auxv_info *info = get_auxv_inferior_data (ops);
> + gdb::optional<gdb::byte_vector> auxv = target_read_auxv ();
>
> - if (!info->data)
> + if (!auxv.has_value ())
> return -1;
>
> - const gdb_byte *data = info->data->data ();
> + const gdb_byte *data = auxv->data ();
> const gdb_byte *ptr = data;
> - size_t len = info->data->size ();
> + size_t len = auxv->size ();
>
> - while (parse_auxv (&ptr, data + len, &type, &val) > 0)
> + while (parse_auxv (current_inferior ()->top_target (),
> + current_inferior ()->gdbarch,
> + &ptr, data + len, &type, &val) > 0)
> {
> gdbarch_print_auxv_entry (gdbarch, file, type, val);
> ++ents;
> @@ -583,8 +594,7 @@ info_auxv_command (const char *cmd, int from_tty)
> error (_("The program has no auxiliary information now."));
> else
> {
> - int ents = fprint_target_auxv (gdb_stdout,
> - current_inferior ()->top_target ());
> + int ents = fprint_target_auxv (gdb_stdout);
>
> if (ents < 0)
> error (_("No auxiliary vector found, or failed reading it."));
> diff --git a/gdb/auxv.h b/gdb/auxv.h
> index ab2a5dee5f74..983e3bc9b0d9 100644
> --- a/gdb/auxv.h
> +++ b/gdb/auxv.h
> @@ -46,13 +46,31 @@ extern int svr4_auxv_parse (struct gdbarch *gdbarch, const gdb_byte **readptr,
> const gdb_byte *endptr, CORE_ADDR *typep,
> CORE_ADDR *valp);
>
> -/* Extract the auxiliary vector entry with a_type matching MATCH.
> +/* Read auxv data from the current inferior's target stack. */
> +
> +extern gdb::optional<gdb::byte_vector> target_read_auxv ();
> +
> +/* Read auxv data from OPS. */
> +
> +extern gdb::optional<gdb::byte_vector> target_read_auxv (target_ops *ops);
> +
> +/* Search AUXV for an entry with a_type matching MATCH.
> +
> + OPS and GDBARCH are the target and architecture to use to parse auxv entries.
> +
> Return zero if no such entry was found, or -1 if there was
> an error getting the information. On success, return 1 after
> storing the entry's value field in *VALP. */
> -extern int target_auxv_search (struct target_ops *ops,
> +
> +extern int target_auxv_search (const gdb::byte_vector &auxv,
> + target_ops *ops, gdbarch *gdbarch,
> CORE_ADDR match, CORE_ADDR *valp);
>
> +/* Same as the above, but read the auxv data from the current inferior. Use
> + the current inferior's top target and arch to parse auxv entries. */
> +
> +extern int target_auxv_search (CORE_ADDR match, CORE_ADDR *valp);
> +
> /* Print a description of a single AUXV entry on the specified file. */
> enum auxv_format { AUXV_FORMAT_DEC, AUXV_FORMAT_HEX, AUXV_FORMAT_STR };
>
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index 8aee634b44b5..1e8935302e8f 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -909,7 +909,7 @@ elf_gnu_ifunc_resolve_addr (struct gdbarch *gdbarch, CORE_ADDR pc)
> parameter. FUNCTION is the function entry address. ADDRESS may be a
> function descriptor. */
>
> - target_auxv_search (current_inferior ()->top_target (), AT_HWCAP, &hwcap);
> + target_auxv_search (AT_HWCAP, &hwcap);
> hwcap_val = value_from_longest (builtin_type (gdbarch)
> ->builtin_unsigned_long, hwcap);
> address_val = call_function_by_hand (function, NULL, hwcap_val);
> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index 309777c55f28..8431caf8f597 100644
> --- a/gdb/fbsd-tdep.c
> +++ b/gdb/fbsd-tdep.c
> @@ -2308,9 +2308,7 @@ fbsd_vmmap_length (struct gdbarch *gdbarch, unsigned char *entries, size_t len,
> static bool
> fbsd_vdso_range (struct gdbarch *gdbarch, struct mem_range *range)
> {
> - struct target_ops *ops = current_inferior ()->top_target ();
> -
> - if (target_auxv_search (ops, AT_FREEBSD_KPRELOAD, &range->start) <= 0)
> + if (target_auxv_search (AT_FREEBSD_KPRELOAD, &range->start) <= 0)
> return false;
>
> if (!target_has_execution ())
> @@ -2337,7 +2335,8 @@ fbsd_vdso_range (struct gdbarch *gdbarch, struct mem_range *range)
> {
> /* Fetch the list of address space entries from the running target. */
> gdb::optional<gdb::byte_vector> buf =
> - target_read_alloc (ops, TARGET_OBJECT_FREEBSD_VMMAP, nullptr);
> + target_read_alloc (current_inferior ()->top_target (),
> + TARGET_OBJECT_FREEBSD_VMMAP, nullptr);
> if (!buf || buf->empty ())
> return false;
>
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index adf518023bbe..dccb45d73a8d 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -417,10 +417,9 @@ int
> linux_is_uclinux (void)
> {
> CORE_ADDR dummy;
> - target_ops *target = current_inferior ()->top_target ();
>
> - return (target_auxv_search (target, AT_NULL, &dummy) > 0
> - && target_auxv_search (target, AT_PAGESZ, &dummy) == 0);
> + return (target_auxv_search (AT_NULL, &dummy) > 0
> + && target_auxv_search (AT_PAGESZ, &dummy) == 0);
> }
>
> static int
> @@ -2379,8 +2378,7 @@ linux_vsyscall_range_raw (struct gdbarch *gdbarch, struct mem_range *range)
> char filename[100];
> long pid;
>
> - if (target_auxv_search (current_inferior ()->top_target (),
> - AT_SYSINFO_EHDR, &range->start) <= 0)
> + if (target_auxv_search (AT_SYSINFO_EHDR, &range->start) <= 0)
> return 0;
>
> /* It doesn't make sense to access the host's /proc when debugging a
> @@ -2570,8 +2568,7 @@ linux_displaced_step_location (struct gdbarch *gdbarch)
> local-store address and is thus not usable as displaced stepping
> location. The auxiliary vector gets us the PowerPC-side entry
> point address instead. */
> - if (target_auxv_search (current_inferior ()->top_target (),
> - AT_ENTRY, &addr) <= 0)
> + if (target_auxv_search (AT_ENTRY, &addr) <= 0)
> throw_error (NOT_SUPPORTED_ERROR,
> _("Cannot find AT_ENTRY auxiliary vector entry."));
>
> @@ -2658,13 +2655,15 @@ linux_displaced_step_restore_all_in_ptid (inferior *parent_inf, ptid_t ptid)
> per_inferior->disp_step_bufs->restore_in_ptid (ptid);
> }
>
> -/* See linux-tdep.h. */
> +/* Helper for linux_get_hwcap and linux_get_hwcap2. */
>
> -CORE_ADDR
> -linux_get_hwcap (struct target_ops *target)
> +static CORE_ADDR
> +linux_get_hwcap_helper (const gdb::optional<gdb::byte_vector> &auxv,
> + target_ops *target, gdbarch *gdbarch, CORE_ADDR match)
> {
> CORE_ADDR field;
> - if (target_auxv_search (target, AT_HWCAP, &field) != 1)
> + if (!auxv.has_value ()
> + || target_auxv_search (*auxv, target, gdbarch, match, &field) != 1)
> return 0;
> return field;
> }
> @@ -2672,12 +2671,39 @@ linux_get_hwcap (struct target_ops *target)
> /* See linux-tdep.h. */
>
> CORE_ADDR
> -linux_get_hwcap2 (struct target_ops *target)
> +linux_get_hwcap (const gdb::optional<gdb::byte_vector> &auxv,
> + target_ops *target, gdbarch *gdbarch)
> {
> - CORE_ADDR field;
> - if (target_auxv_search (target, AT_HWCAP2, &field) != 1)
> - return 0;
> - return field;
> + return linux_get_hwcap_helper (auxv, target, gdbarch, AT_HWCAP);
> +}
> +
> +/* See linux-tdep.h. */
> +
> +CORE_ADDR
> +linux_get_hwcap ()
> +{
> + return linux_get_hwcap (target_read_auxv (),
> + current_inferior ()->top_target (),
> + current_inferior ()->gdbarch);
> +}
> +
> +/* See linux-tdep.h. */
> +
> +CORE_ADDR
> +linux_get_hwcap2 (const gdb::optional<gdb::byte_vector> &auxv,
> + target_ops *target, gdbarch *gdbarch)
> +{
> + return linux_get_hwcap_helper (auxv, target, gdbarch, AT_HWCAP2);
> +}
> +
> +/* See linux-tdep.h. */
> +
> +CORE_ADDR
> +linux_get_hwcap2 ()
> +{
> + return linux_get_hwcap2 (target_read_auxv (),
> + current_inferior ()->top_target (),
> + current_inferior ()->gdbarch);
> }
>
> /* Display whether the gcore command is using the
> diff --git a/gdb/linux-tdep.h b/gdb/linux-tdep.h
> index bb907f2c8f35..95cc29c828c2 100644
> --- a/gdb/linux-tdep.h
> +++ b/gdb/linux-tdep.h
> @@ -90,13 +90,27 @@ extern void linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
>
> extern int linux_is_uclinux (void);
>
> -/* Fetch the AT_HWCAP entry from the auxv vector for the given TARGET. On
> - error, 0 is returned. */
> -extern CORE_ADDR linux_get_hwcap (struct target_ops *target);
> +/* Fetch the AT_HWCAP entry from auxv data AUXV. Use TARGET and GDBARCH to
> + parse auxv entries.
>
> -/* Fetch the AT_HWCAP2 entry from the auxv vector for the given TARGET. On
> - error, 0 is returned. */
> -extern CORE_ADDR linux_get_hwcap2 (struct target_ops *target);
> + On error, 0 is returned. */
> +extern CORE_ADDR linux_get_hwcap (const gdb::optional<gdb::byte_vector> &auxv,
> + struct target_ops *target, gdbarch *gdbarch);
> +
> +/* Same as the above, but obtain all the inputs from the current inferior. */
> +
> +extern CORE_ADDR linux_get_hwcap ();
> +
> +/* Fetch the AT_HWCAP2 entry from auxv data AUXV. Use TARGET and GDBARCH to
> + parse auxv entries.
> +
> + On error, 0 is returned. */
> +extern CORE_ADDR linux_get_hwcap2 (const gdb::optional<gdb::byte_vector> &auxv,
> + struct target_ops *target, gdbarch *gdbarch);
> +
> +/* Same as the above, but obtain all the inputs from the current inferior. */
> +
> +extern CORE_ADDR linux_get_hwcap2 ();
>
> /* Fetch (and possibly build) an appropriate `struct link_map_offsets'
> for ILP32 and LP64 Linux systems. */
> diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
> index dfa81e19a79f..795bb298955f 100644
> --- a/gdb/ppc-linux-nat.c
> +++ b/gdb/ppc-linux-nat.c
> @@ -1965,8 +1965,8 @@ ppc_linux_nat_target::read_description ()
>
> features.wordsize = ppc_linux_target_wordsize (tid);
>
> - CORE_ADDR hwcap = linux_get_hwcap (current_inferior ()->top_target ());
> - CORE_ADDR hwcap2 = linux_get_hwcap2 (current_inferior ()->top_target ());
> + CORE_ADDR hwcap = linux_get_hwcap ();
> + CORE_ADDR hwcap2 = linux_get_hwcap2 ();
>
> if (have_ptrace_getsetvsxregs
> && (hwcap & PPC_FEATURE_HAS_VSX))
> @@ -2123,8 +2123,7 @@ ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
> takes two hardware watchpoints though. */
> if (len > 1
> && hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_RANGE
> - && (linux_get_hwcap (current_inferior ()->top_target ())
> - & PPC_FEATURE_BOOKE))
> + && (linux_get_hwcap () & PPC_FEATURE_BOOKE))
> return 2;
> /* Check if the processor provides DAWR interface. */
> if (hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_DAWR)
> @@ -2152,8 +2151,7 @@ ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
> {
> gdb_assert (m_dreg_interface.debugreg_p ());
>
> - if (((linux_get_hwcap (current_inferior ()->top_target ())
> - & PPC_FEATURE_BOOKE)
> + if (((linux_get_hwcap () & PPC_FEATURE_BOOKE)
> && (addr + len) > (addr & ~3) + 4)
> || (addr + len) > (addr & ~7) + 8)
> return 0;
> @@ -2640,8 +2638,7 @@ ppc_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len,
> long wp_value;
> long read_mode, write_mode;
>
> - if (linux_get_hwcap (current_inferior ()->top_target ())
> - & PPC_FEATURE_BOOKE)
> + if (linux_get_hwcap () & PPC_FEATURE_BOOKE)
> {
> /* PowerPC 440 requires only the read/write flags to be passed
> to the kernel. */
> @@ -3014,11 +3011,9 @@ ppc_linux_nat_target::watchpoint_addr_within_range (CORE_ADDR addr,
> int mask;
>
> if (m_dreg_interface.hwdebug_p ()
> - && (linux_get_hwcap (current_inferior ()->top_target ())
> - & PPC_FEATURE_BOOKE))
> + && (linux_get_hwcap () & PPC_FEATURE_BOOKE))
> return start <= addr && start + length >= addr;
> - else if (linux_get_hwcap (current_inferior ()->top_target ())
> - & PPC_FEATURE_BOOKE)
> + else if (linux_get_hwcap () & PPC_FEATURE_BOOKE)
> mask = 3;
> else
> mask = 7;
> diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
> index 96eb931743fd..3bc972dfc9aa 100644
> --- a/gdb/ppc-linux-tdep.c
> +++ b/gdb/ppc-linux-tdep.c
> @@ -1599,7 +1599,8 @@ ppc_linux_core_read_description (struct gdbarch *gdbarch,
> if (vsx)
> features.vsx = true;
>
> - CORE_ADDR hwcap = linux_get_hwcap (target);
> + gdb::optional<gdb::byte_vector> auxv = target_read_auxv (target);
> + CORE_ADDR hwcap = linux_get_hwcap (auxv, target, gdbarch);
>
> features.isa205 = ppc_linux_has_isa205 (hwcap);
>
> diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
> index 2a396eff4c49..bbdc6f6e5555 100644
> --- a/gdb/rs6000-tdep.c
> +++ b/gdb/rs6000-tdep.c
> @@ -5503,8 +5503,7 @@ ppc_process_record_op31 (struct gdbarch *gdbarch, struct regcache *regcache,
> return 0;
>
> case 1014: /* Data Cache Block set to Zero */
> - if (target_auxv_search (current_inferior ()->top_target (),
> - AT_DCACHEBSIZE, &at_dcsz) <= 0
> + if (target_auxv_search (AT_DCACHEBSIZE, &at_dcsz) <= 0
> || at_dcsz == 0)
> at_dcsz = 128; /* Assume 128-byte cache line size (POWER8) */
>
> diff --git a/gdb/s390-linux-nat.c b/gdb/s390-linux-nat.c
> index 2b21e0822362..96833e804e9a 100644
> --- a/gdb/s390-linux-nat.c
> +++ b/gdb/s390-linux-nat.c
> @@ -1002,7 +1002,7 @@ s390_linux_nat_target::read_description ()
> that mode, report s390 architecture with 64-bit GPRs. */
> #ifdef __s390x__
> {
> - CORE_ADDR hwcap = linux_get_hwcap (current_inferior ()->top_target ());
> + CORE_ADDR hwcap = linux_get_hwcap ();
>
> have_regset_tdb = (hwcap & HWCAP_S390_TE)
> && check_regset (tid, NT_S390_TDB, s390_sizeof_tdbregset);
> diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
> index 05bf03973fc3..b8f4362c0d9e 100644
> --- a/gdb/s390-linux-tdep.c
> +++ b/gdb/s390-linux-tdep.c
> @@ -332,7 +332,8 @@ s390_core_read_description (struct gdbarch *gdbarch,
> struct target_ops *target, bfd *abfd)
> {
> asection *section = bfd_get_section_by_name (abfd, ".reg");
> - CORE_ADDR hwcap = linux_get_hwcap (target);
> + gdb::optional<gdb::byte_vector> auxv = target_read_auxv (target);
> + CORE_ADDR hwcap = linux_get_hwcap (auxv, target, gdbarch);
> bool high_gprs, v1, v2, te, vx, gs;
>
> if (!section)
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index a6a9ec5c86b3..db71cd31d33f 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -428,14 +428,11 @@ read_program_header (int type, int *p_arch_size, CORE_ADDR *base_addr)
> int pt_phdr_p = 0;
>
> /* Get required auxv elements from target. */
> - if (target_auxv_search (current_inferior ()->top_target (),
> - AT_PHDR, &at_phdr) <= 0)
> + if (target_auxv_search (AT_PHDR, &at_phdr) <= 0)
> return {};
> - if (target_auxv_search (current_inferior ()->top_target (),
> - AT_PHENT, &at_phent) <= 0)
> + if (target_auxv_search (AT_PHENT, &at_phent) <= 0)
> return {};
> - if (target_auxv_search (current_inferior ()->top_target (),
> - AT_PHNUM, &at_phnum) <= 0)
> + if (target_auxv_search (AT_PHNUM, &at_phnum) <= 0)
> return {};
> if (!at_phdr || !at_phnum)
> return {};
> @@ -2250,8 +2247,7 @@ enable_break (struct svr4_info *info, int from_tty)
> /* If we were not able to find the base address of the loader
> from our so_list, then try using the AT_BASE auxilliary entry. */
> if (!load_addr_found)
> - if (target_auxv_search (current_inferior ()->top_target (),
> - AT_BASE, &load_addr) > 0)
> + if (target_auxv_search (AT_BASE, &load_addr) > 0)
> {
> int addr_bit = gdbarch_addr_bit (target_gdbarch ());
>
> @@ -2479,8 +2475,7 @@ svr4_exec_displacement (CORE_ADDR *displacementp)
> if ((bfd_get_file_flags (current_program_space->exec_bfd ()) & DYNAMIC) == 0)
> return 0;
>
> - if (target_auxv_search (current_inferior ()->top_target (),
> - AT_ENTRY, &entry_point) <= 0)
> + if (target_auxv_search (AT_ENTRY, &entry_point) <= 0)
> return 0;
>
> exec_displacement
> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
> index 6b9d9eaa9575..3122c62a1c38 100644
> --- a/gdb/sparc64-tdep.c
> +++ b/gdb/sparc64-tdep.c
> @@ -214,12 +214,10 @@ adi_available (void)
> return proc->stat.is_avail;
>
> proc->stat.checked_avail = true;
> - if (target_auxv_search (current_inferior ()->top_target (),
> - AT_ADI_BLKSZ, &value) <= 0)
> + if (target_auxv_search (AT_ADI_BLKSZ, &value) <= 0)
> return false;
> proc->stat.blksize = value;
> - target_auxv_search (current_inferior ()->top_target (),
> - AT_ADI_NBITS, &value);
> + target_auxv_search (AT_ADI_NBITS, &value);
> proc->stat.nbits = value;
> proc->stat.max_version = (1 << proc->stat.nbits) - 2;
> proc->stat.is_avail = true;
>
> base-commit: ae17d05a4a58baf42f297dfd40ed29256f4bc44d
Thanks for the patch.
The code is a bit more verbose, but the reasoning makes sense to me. So LGTM.
I verified this on my end with mte corefiles and it works correctly.
As a general comment, it feels to me the auxv-handling code could use some C++-ification
to handle things in a better way.
next prev parent reply other threads:[~2022-10-10 9:33 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-19 14:45 [PATCH] Update auxv cache when there is no auxv cached data Luis Machado
2022-07-25 9:42 ` [PING][PATCH] " Luis Machado
2022-07-25 16:05 ` [PATCH] " John Baldwin
2022-07-25 18:03 ` Luis Machado
2022-07-25 19:13 ` John Baldwin
2022-08-02 15:05 ` Luis Machado
2022-08-02 16:05 ` John Baldwin
2022-08-05 15:46 ` [PATCH] Update auxv cache when inferior pid is 0 (no inferior) Luis Machado
2022-08-11 9:05 ` [PING][PATCH] " Luis Machado
2022-08-18 15:48 ` Luis Machado
2022-09-01 9:29 ` Luis Machado
2022-09-07 8:20 ` Luis Machado
2022-09-12 12:48 ` Luis Machado
2022-09-12 13:30 ` [PATCH] " Simon Marchi
2022-09-12 13:53 ` John Baldwin
2022-09-12 13:59 ` Luis Machado
2022-09-20 12:28 ` [PATCH] Invalidate auxv cache before creating a core target Luis Machado
2022-09-20 17:49 ` John Baldwin
2022-10-07 20:44 ` [PATCH] gdb: fix auxv caching Simon Marchi
2022-10-07 21:43 ` John Baldwin
2022-10-09 0:39 ` Simon Marchi
2022-10-10 18:32 ` John Baldwin
2022-10-11 17:52 ` Simon Marchi
2022-10-11 20:31 ` Pedro Alves
2022-10-11 20:34 ` Pedro Alves
2022-10-11 20:42 ` John Baldwin
2022-10-12 1:11 ` Simon Marchi
2022-10-10 9:33 ` Luis Machado [this message]
2022-10-11 17:53 ` Simon Marchi
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=d4382b69-1ad5-a7e9-bf2f-91a593568c03@arm.com \
--to=luis.machado@arm.com \
--cc=gdb-patches@sourceware.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).