Invalidate auxv cache before creating a core target

Message ID 20220920122828.188190-1-luis.machado@arm.com
State Superseded
Headers
Series Invalidate auxv cache before creating a core target |

Commit Message

Luis Machado Sept. 20, 2022, 12:28 p.m. UTC
  While adding support for MTE corefiles and running the MTE corefile tests,
I noticed a strange situation where loading the symbol file + core file
through the command line has a different behavior compared to firing up
GDB, loading the symbol file with the "file" command and then loading the
core file with the "core" command.

I tracked this down to gdb/auxv.c:get_auxv_inferior_data returning empty
auxv data for pid 0, which gets cached. This is triggered by attempting to
read auxv data for the exec target.

In the early stages of reading the core file, we're still using inferior pid
0, so when we attempt to read auxv to determine corefile features, we get the
cached empty data vector again. This breaks core_gdbarch setup.

To overcome this, make sure we invalidate the auxv data when creating a new core
target.
---
 gdb/auxv.c    |  4 ++--
 gdb/auxv.h    |  2 ++
 gdb/corelow.c | 15 +++++++++++++++
 3 files changed, 19 insertions(+), 2 deletions(-)
  

Comments

John Baldwin Sept. 20, 2022, 5:49 p.m. UTC | #1
On 9/20/22 1:28 PM, Luis Machado wrote:
> While adding support for MTE corefiles and running the MTE corefile tests,
> I noticed a strange situation where loading the symbol file + core file
> through the command line has a different behavior compared to firing up
> GDB, loading the symbol file with the "file" command and then loading the
> core file with the "core" command.
> 
> I tracked this down to gdb/auxv.c:get_auxv_inferior_data returning empty
> auxv data for pid 0, which gets cached. This is triggered by attempting to
> read auxv data for the exec target.
> 
> In the early stages of reading the core file, we're still using inferior pid
> 0, so when we attempt to read auxv to determine corefile features, we get the
> cached empty data vector again. This breaks core_gdbarch setup.
> 
> To overcome this, make sure we invalidate the auxv data when creating a new core
> target.
> ---
>   gdb/auxv.c    |  4 ++--
>   gdb/auxv.h    |  2 ++
>   gdb/corelow.c | 15 +++++++++++++++
>   3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/auxv.c b/gdb/auxv.c
> index 6154988f6dd..2423f51f224 100644
> --- a/gdb/auxv.c
> +++ b/gdb/auxv.c
> @@ -342,9 +342,9 @@ invalidate_auxv_cache_inf (struct inferior *inf)
>     auxv_inferior_data.clear (inf);
>   }
>   
> -/* Invalidate current inferior's auxv cache.  */
> +/* See auxv.h */
>   
> -static void
> +void
>   invalidate_auxv_cache (void)
>   {
>     invalidate_auxv_cache_inf (current_inferior ());
> diff --git a/gdb/auxv.h b/gdb/auxv.h
> index a4801c34d2f..242bc2eff00 100644
> --- a/gdb/auxv.h
> +++ b/gdb/auxv.h
> @@ -79,5 +79,7 @@ extern int fprint_target_auxv (struct ui_file *file, struct target_ops *ops);
>   
>   extern target_xfer_partial_ftype memory_xfer_auxv;
>   
> +/* Invalidate current inferior's auxv cache.  */
> +void invalidate_auxv_cache (void);
>   
>   #endif
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index 293bc8d4f59..1c30c7d011c 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -53,6 +53,7 @@
>   #include "gdbcmd.h"
>   #include "xml-tdesc.h"
>   #include "memtag.h"
> +#include "auxv.h"
>   
>   #ifndef O_LARGEFILE
>   #define O_LARGEFILE 0
> @@ -515,6 +516,20 @@ core_target_open (const char *arg, int from_tty)
>   
>     current_program_space->cbfd = std::move (temp_bfd);
>   
> +  /* Some architectures identify features using the hwcap bits that
> +     come from the auxv.  Before creating a new core target, make sure we
> +     invalidate the auxv cache so we can get fresh data.
> +
> +     This is required when we have an exec target before loading the core
> +     file.  Under such conditions the pid is 0, and the exec target may
> +     attempt to fetch and cache the auxv.  This results in GDB caching
> +     empty contents as the exec target doesn't have auxv data.
> +
> +     Given we haven't determined the pid yet (we will read it later), it can
> +     still be 0 and simply fetching the auxv may return the cached empty
> +     data.  This ultimately results in problems coming up with the proper
> +     gdbarch data.  */
> +  invalidate_auxv_cache ();
>     core_target *target = new core_target ();
>   
>     /* Own the target until it is successfully pushed.  */

This looks good to me.
  

Patch

diff --git a/gdb/auxv.c b/gdb/auxv.c
index 6154988f6dd..2423f51f224 100644
--- a/gdb/auxv.c
+++ b/gdb/auxv.c
@@ -342,9 +342,9 @@  invalidate_auxv_cache_inf (struct inferior *inf)
   auxv_inferior_data.clear (inf);
 }
 
-/* Invalidate current inferior's auxv cache.  */
+/* See auxv.h */
 
-static void
+void
 invalidate_auxv_cache (void)
 {
   invalidate_auxv_cache_inf (current_inferior ());
diff --git a/gdb/auxv.h b/gdb/auxv.h
index a4801c34d2f..242bc2eff00 100644
--- a/gdb/auxv.h
+++ b/gdb/auxv.h
@@ -79,5 +79,7 @@  extern int fprint_target_auxv (struct ui_file *file, struct target_ops *ops);
 
 extern target_xfer_partial_ftype memory_xfer_auxv;
 
+/* Invalidate current inferior's auxv cache.  */
+void invalidate_auxv_cache (void);
 
 #endif
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 293bc8d4f59..1c30c7d011c 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -53,6 +53,7 @@ 
 #include "gdbcmd.h"
 #include "xml-tdesc.h"
 #include "memtag.h"
+#include "auxv.h"
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -515,6 +516,20 @@  core_target_open (const char *arg, int from_tty)
 
   current_program_space->cbfd = std::move (temp_bfd);
 
+  /* Some architectures identify features using the hwcap bits that
+     come from the auxv.  Before creating a new core target, make sure we
+     invalidate the auxv cache so we can get fresh data.
+
+     This is required when we have an exec target before loading the core
+     file.  Under such conditions the pid is 0, and the exec target may
+     attempt to fetch and cache the auxv.  This results in GDB caching
+     empty contents as the exec target doesn't have auxv data.
+
+     Given we haven't determined the pid yet (we will read it later), it can
+     still be 0 and simply fetching the auxv may return the cached empty
+     data.  This ultimately results in problems coming up with the proper
+     gdbarch data.  */
+  invalidate_auxv_cache ();
   core_target *target = new core_target ();
 
   /* Own the target until it is successfully pushed.  */