Re: [PATCH] Implement new `info core mappings' command

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Wed, 26 Oct 2011 22:49:53 +0200, Sergio Durigan Junior wrote:
> --- a/gdb/corefile.c
> +++ b/gdb/corefile.c
[...]
> @@ -83,6 +89,186 @@ core_file_command (char *filename, int from_tty)
>  }
>  \f
>  
> +/* Helper function for `print_core_map'.  It is used to iterate
> +   over the corefile's sections and print proper information about
> +   memory-mappings.
> +
> +   BFD is the bfd used to get the sections.
> +   SECT is the current section being "visited".
> +   OBJ is not used.  */
> +
> +static void
> +print_proc_map_iter (bfd *bfd, asection *sect, void *obj)
> +{
> +  /* We're interested in matching sections' names beginning with
> +     `load', because they are the sections containing information
> +     about the process' memory regions.  */
> +  static const char *proc_map_match = "load";

I think they are pretty useful, for Linux kernel dumped core files with
MMF_DUMP_ELF_HEADERS
/usr/share/doc/kernel-doc-*/Documentation/filesystems/proc.txt
  - (bit 4) ELF header pages in file-backed private memory areas (it is
            effective only if the bit 2 is cleared)

Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  NOTE           0x001508 0x0000000000000000 0x0000000000000000 0x0008d0 0x000000     0
  LOAD           0x002000 0x0000000000400000 0x0000000000000000 0x001000 0x008000 R E 0x1000
  LOAD           0x003000 0x0000000000607000 0x0000000000000000 0x002000 0x002000 RW  0x1000

(gdb) info files
Local core dump file:
	0x0000000000400000 - 0x0000000000401000 is load1a
	0x0000000000401000 - 0x0000000000401000 is load1b
	0x0000000000607000 - 0x0000000000609000 is load2

But one does not see the ending address 0x408000 anywhere, one IMO has to use
readelf/objdump now to get the full information.


I think this function should not be based on sections at all, it should just
read the segments.  Linux kernel does not dump any sections.  bfd creates some
some sections from those segments (_bfd_elf_make_section_from_phdr) but they
cannot / do not contain any additional info, those are there IMO only for
better compatibility with sections-only consuming code.

gcore produces even sections but I think it is just redundant info and maybe
even a bug just as a result of the sections generated by bfd.


> +  int proc_map_match_size = strlen (proc_map_match);
> +  /* Flag to indicate whether we have found something.  */
> +  int found = 0;
> +  /* The section's size.  */
> +  bfd_size_type secsize;
> +  /* We have to know the bitness of this architecture.  */
> +  int bitness;
> +  /* We'll use these later.  They are basically used for iterating
> +     over every objfile in the system so that we can find needed
> +     information about the memory region being examinated.  */
> +  struct obj_section *s = NULL;
> +  struct objfile *objfile = NULL;
> +  /* Fields to be printed for the proc map.  */
> +  unsigned long start = 0, end = 0;
> +  unsigned int size = 0;

On 32bit host with --enable-64-bit-bfd: sizeof (bfd_vma) > sizeof (long)
moreover for sizeof (int) of `size'.


> +  char *filename = NULL;
> +
> +  if (strncmp (proc_map_match, sect->name, proc_map_match_size) != 0)
> +    /* This section is not useful.  */
> +    return;
> +
> +  bitness = gdbarch_addr_bit (gdbarch_from_bfd (bfd));
> +
> +  /* Unfortunately, some sections in the corefile don't have any
> +     content inside.  This is bad because we need to print, among
> +     other things, its final address in the memory (which is
> +     impossible to know if we don't have a size).  That's why we
> +     first need to check if the section's got anything inside it.  */
> +  secsize = bfd_section_size (bfd, sect);
> +
> +  if (secsize == 0)
> +    {
> +      /* Ok, the section is empty.  In this case, we must look inside
> +	 ELF's Program Header, because (at least) there we have
> +	 information about the section's size.  That's what we're doing
> +	 here.  */
> +      Elf_Internal_Phdr *p = elf_tdata (bfd)->phdr;
> +      if (p != NULL)
> +	{
> +	  int i;
> +	  unsigned int n = elf_elfheader (bfd)->e_phnum;
> +	  for (i = 0; i < n; i++, p++)
> +	    /* For each entry in the Program Header, we have to
> +	       check if the section's initial address is equal to
> +	       the entry's virtual address.  If it is, then we
> +	       have just found the section's entry in the Program
> +	       Header, and can use the entry's information to
> +	       complete missing data from the section.  */
> +	    if (sect->vma == p->p_vaddr)
> +	      {
> +		found = 1;
> +		break;
> +	      }

I do not understand what is a goal of this part.  Isn't it related to the
pairtally omitted segments above?  But those are named "load..." so they are
already skipped.


> +	  if (found)
> +	    secsize = p->p_memsz;
> +	}
> +    }
> +
> +  size = secsize;
> +  start = sect->vma;
> +  end = (unsigned long) (sect->vma + size);
> +
> +  /* Now begins a new part of the work.  We still don't have complete
> +     information about the memory region.  For example, we still need
> +     to know the filename which is represented by the region.  Such
> +     info can be gathered from the objfile's data structure, and for
> +     that we must iterate over all the objsections and check if the
> +     objsection's initial address is inside the section we have at hand.
> +     If it is, then we can use this specific objsection to obtain the
> +     missing data.  */
> +  found = 0;
> +  ALL_OBJSECTIONS (objfile, s)
> +    if (obj_section_addr (s) >= start
> +	&& obj_section_addr (s) <= end)

I think it should ignore S which is section_is_overlay.


> +      {
> +	found = 1;
> +	break;
> +      }
> +
> +  if (found)
> +    filename = s->objfile->name;
> +
> +  if (bitness == 32)
> +    printf_filtered ("\t%#10lx %#10lx %#10x %7s\n",
> +		     start,
> +		     end,
> +		     (int) size,
> +		     filename ? filename : "");
> +  else
> +    printf_filtered ("  %#18lx %#18lx %#10x %7s\n",
> +		     start,
> +		     end,
> +		     (int) size,
> +		     filename ? filename : "");
> +}
> +
> +/* Implements the `info proc map' command when the user has provided
> +   a corefile.  */
> +
> +static void
> +print_core_map (void)
> +{
> +  const char *exe;
> +  int bitness;
> +
> +  gdb_assert (core_bfd != NULL);
> +
> +  bitness = gdbarch_addr_bit (gdbarch_from_bfd (core_bfd));
> +
> +  /* Getting the executable name.  */
> +  exe = bfd_core_file_failing_command (core_bfd);
> +
> +  printf_filtered (_("exe = '%s'\n"), exe);

bfd_core_file_failing_command can return NULL, NULL is not compatible with %s;
also the bfd error may be printed in such case.


> +  printf_filtered (_("Mapped address spaces:\n\n"));
> +  if (bitness == 32)
> +    printf_filtered ("\t%10s %10s %10s %7s\n",
> +		     "Start Addr",
> +		     "  End Addr",
> +		     "      Size", "objfile");
> +  else
> +    printf_filtered ("  %18s %18s %10s %7s\n",
> +		     "Start Addr",
> +		     "  End Addr",
> +		     "      Size", "objfile");
> +
> +  bfd_map_over_sections (core_bfd,
> +                         print_proc_map_iter,
> +                         NULL);
> +}
> +
> +/* Implement the `info core' command.  */
> +
> +static void
> +info_core_cmd (char *args, int from_tty)
> +{
> +  char **argv = NULL;
> +  int mappings_f = 1;
> +  int all = 0;
> +
> +  if (!core_bfd)
> +    error (_("You are not using a corefile at the moment."));
> +
> +  if (args)
> +    {
> +      /* Break up 'args' into an argv array.  */
> +      argv = gdb_buildargv (args);
> +      make_cleanup_freeargv (argv);
> +    }
> +  while (argv != NULL && *argv != NULL)
> +    {
> +      if (strncmp (argv[0], "mappings", strlen (argv[0])) == 0)
> +	{
> +	  mappings_f = 1;
> +	}
> +      else if (strncmp (argv[0], "all", strlen (argv[0])) == 0)
> +	{
> +	  all = 1;
> +	}
> +      argv++;
> +    }
> +
> +  if (mappings_f || all)
> +    print_core_map ();
> +}
> +
>  /* If there are two or more functions that wish to hook into
>     exec_file_command, this function will call all of the hook
>     functions.  */
> @@ -450,6 +636,11 @@ _initialize_core (void)
>  {
>    struct cmd_list_element *c;
>  
> +  add_info ("core", info_core_cmd, _("\
> +Show information about a corefile.\n\
> +Specify any of the following keywords for detailed info:\n\
> +  mappings -- list of mapped memory regions."));

I think it should be add_prefix_cmd so that tab completion works.  "mappings
/ "all" should be commands, not parameters.  "info proc" already has this bug.



Thanks,
Jan