public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
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.

  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).