[v4,12/14] Add new command "maint print core-file-backed-mappings"

Message ID 20200705225807.2264705-13-kevinb@redhat.com
State Superseded
Headers show
Series
  • Fix BZ 25631 - core file memory access problem
Related show

Commit Message

Pedro Franco de Carvalho via Gdb-patches July 5, 2020, 10:58 p.m.
I wrote a read_core_file_mappings method for FreeBSD and then registered
this gdbarch method.  I saw some strange behavior while testing it and
wanted a way to make sure that mappings were being correctly loaded
into corelow.c, so I wrote the new command which is the topic of this
commit.  I think it might be occasionally useful for debugging strange
corefile behavior.

With regard to FreeBSD, my work isn't ready yet.  Unlike Linux,
FreeBSD puts all mappings into its core file note.  And, unlike Linux,
it doesn't dump load segments which occupy no space in the file.  So
my (perhaps naive) implementation of a FreeBSD read_core_file_mappings
didn't work all that well:  I saw more failures in the corefile2.exp
tests than without it.  I think it should be possible to make FreeBSD
work as well as Linux, but it will require doing something with all of
the mappings, not just the file based mappings that I was considering.

gdb/ChangeLog:

	* corelow.c (gdbcmd.h): Include.
	(core_target::info_proc_mappings): New method.
	(get_current_core_target): New function.
	(maintenance_print_core_file_backed_mappings): New function.
	(_initialize_corelow): Add core-file-backed-mappings to
	"maint print" commands.
---
 gdb/corelow.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

-- 
2.26.2

Comments

Pedro Alves July 10, 2020, 8:08 p.m. | #1
On 7/5/20 11:58 PM, Kevin Buettner via Gdb-patches wrote:
> +  for (const struct target_section *tsp = m_core_file_mappings.sections;

> +       tsp < m_core_file_mappings.sections_end; tsp++ )

> +    {


Spurious space after tsp++.

If you need to break the for line, I personally prefer to then
put each statement in its own line, like:

 for (const struct target_section *tsp = m_core_file_mappings.sections;
      tsp < m_core_file_mappings.sections_end; 
      ++tsp)

> +static void

> +maintenance_print_core_file_backed_mappings (const char *args, int from_tty)

> +{

> +  core_target *targ = get_current_core_target ();

> +  gdb_assert (targ != nullptr);


This assert will crash GDB if you're debugging a core dump, right?
That doesn't look right.  (Please add a test for that.)

> +  targ->info_proc_mappings (targ->core_gdbarch ());

> +}



I don't understand what this command provides that "info proc mappings"
doesn't?  Can you give an example of when you'd use this command
over "info proc mappings" ?

Thanks,
Pedro Alves
Pedro Alves July 10, 2020, 8:10 p.m. | #2
On 7/10/20 9:08 PM, Pedro Alves wrote:> 
>> +static void

>> +maintenance_print_core_file_backed_mappings (const char *args, int from_tty)

>> +{

>> +  core_target *targ = get_current_core_target ();

>> +  gdb_assert (targ != nullptr);


> This assert will crash GDB if you're debugging a core dump, right?

> That doesn't look right.  (Please add a test for that.)


This assert will crash GDB if you're NOT debugging a core dump,
of course.
Pedro Franco de Carvalho via Gdb-patches July 13, 2020, 5:33 p.m. | #3
On Fri, 10 Jul 2020 21:08:15 +0100
Pedro Alves <pedro@palves.net> wrote:

> I don't understand what this command provides that "info proc mappings"

> doesn't?  Can you give an example of when you'd use this command

> over "info proc mappings" ?


I wanted it while adding core file note support for FreeBSD.  I wanted
to check that 1) the mappings were actually be loaded and 2) the
mappings loaded by corelow.c were correct.  I think it might be occasionally
useful by other GDB developers for debugging core file related problems.

A normal GDB user won't use this command; that's why it's a maintenance
command.  Depending on the OS there may be differences between
"info proc mappings" and this new maintenance command.  At the
moment, there aren't any differences on Linux, but there will be for
FreeBSD.  FreeBSD provides more mappings and more fields for each
mapping.  I would expect that "info proc mappings" on FreeBSD will
show all mappings along with the additional fields.  The maintenance
command will show a subset of that information; as such it might not
seem terribly useful, but the point of it is to output information from
the corelow.c created data structures so that a developer can check
that they exist and are correct.

I'll add a few more words to the commit log.

Kevin

Patch

diff --git a/gdb/corelow.c b/gdb/corelow.c
index ac52a1019e..173418745b 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -47,6 +47,7 @@ 
 #include "build-id.h"
 #include "gdbsupport/pathstuff.h"
 #include <unordered_map>
+#include "gdbcmd.h"
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -113,6 +114,9 @@  class core_target final : public process_stratum_target
 				  const char *human_name,
 				  bool required);
 
+  /* See definition.  */
+  void info_proc_mappings (struct gdbarch *gdbarch);
+
 private: /* per-core data */
 
   /* The core's section table.  Note that these target sections are
@@ -1027,9 +1031,87 @@  core_target::info_proc (const char *args, enum info_proc_what request)
   return true;
 }
 
+/* Get a pointer to the current core target.  If not connected to a
+   core target, return NULL.  */
+
+static core_target *
+get_current_core_target ()
+{
+  target_ops *proc_target = current_inferior ()->process_target ();
+  return dynamic_cast<core_target *> (proc_target);
+}
+
+/* Display file backed mappings from core file.  */
+
+void
+core_target::info_proc_mappings (struct gdbarch *gdbarch)
+{
+  if (m_core_file_mappings.sections != m_core_file_mappings.sections_end)
+    {
+      printf_filtered (_("Mapped address spaces:\n\n"));
+      if (gdbarch_addr_bit (gdbarch) == 32)
+	{
+	  printf_filtered ("\t%10s %10s %10s %10s %s\n",
+			   "Start Addr",
+			   "  End Addr",
+			   "      Size", "    Offset", "objfile");
+	}
+      else
+	{
+	  printf_filtered ("  %18s %18s %10s %10s %s\n",
+			   "Start Addr",
+			   "  End Addr",
+			   "      Size", "    Offset", "objfile");
+	}
+    }
+
+  for (const struct target_section *tsp = m_core_file_mappings.sections;
+       tsp < m_core_file_mappings.sections_end; tsp++ )
+    {
+      ULONGEST start = tsp->addr;
+      ULONGEST end = tsp->endaddr;
+      ULONGEST file_ofs = tsp->the_bfd_section->filepos;
+      const char *filename = bfd_get_filename (tsp->the_bfd_section->owner);
+
+      if (gdbarch_addr_bit (gdbarch) == 32)
+	printf_filtered ("\t%10s %10s %10s %10s %s\n",
+			 paddress (gdbarch, start),
+			 paddress (gdbarch, end),
+			 hex_string (end - start),
+			 hex_string (file_ofs),
+			 filename);
+      else
+	printf_filtered ("  %18s %18s %10s %10s %s\n",
+			 paddress (gdbarch, start),
+			 paddress (gdbarch, end),
+			 hex_string (end - start),
+			 hex_string (file_ofs),
+			 filename);
+    }
+}
+
+/* Implement "maintenance print core-file-backed-mappings" command.  
+
+   If mappings are loaded, the results should be similar to the
+   mappings shown by "info proc mappings".  This command is mainly
+   a debugging tool to make sure that the expected mappings are
+   present after loading a core file.  */
+
+static void
+maintenance_print_core_file_backed_mappings (const char *args, int from_tty)
+{
+  core_target *targ = get_current_core_target ();
+  gdb_assert (targ != nullptr);
+  targ->info_proc_mappings (targ->core_gdbarch ());
+}
+
 void _initialize_corelow ();
 void
 _initialize_corelow ()
 {
   add_target (core_target_info, core_target_open, filename_completer);
+  add_cmd ("core-file-backed-mappings", class_maintenance,
+           maintenance_print_core_file_backed_mappings,
+	   _("Print core file's file-backed mappings"),
+	   &maintenanceprintlist);
 }