Re: [patch] Fix gcore writer for -Wl,-z,relro (PR corefiles/11804)

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

technical rediff due to the patch update:
	Re: [patch] Code cleanup: Make function typedef for find memory region
	http://sourceware.org/ml/gdb-patches/2010-08/msg00554.html


On Mon, 30 Aug 2010 11:15:21 +0200, Jan Kratochvil wrote:
Hi,

currently for -Wl,-z,relro executables GDB will not dump the DYNAMIC segment
into the core file, failing to read shared library list later.

(gdb) info sharedlibrary 
>From                To                  Syms Read   Shared Object Library
0x00007ffff7dddb20  0x00007ffff7df5c74  Yes         /lib64/ld-linux-x86-64.so.2

This is just the fallback, GDB considers the executable uninitialized with
DT_DEBUG value zero.

Linux kernel dumps it correctly, respecting if the page got ever modified
since being read from the disk, even if it is not (no longer) writable.

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.

The "Shared_Dirty:" / "Private_Dirty:" / "Swap:" rule has been suggested by
Roland McGrath and confirmed by Larry Woodman.

This patch depends on:
	[patch] Code cleanup: Make function typedef for find memory region
	http://sourceware.org/ml/gdb-patches/2010-08/msg00498.html


Thanks,
Jan


gdb/
2010-08-30  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix gcore writer for -Wl,-z,relro.
	* defs.h (find_memory_region_ftype): New parameter `modified'.  New
	comment.
	* fbsd-nat.c (fbsd_find_memory_regions): Pass -1 to func.
	* gcore.c (gcore_create_callback): New parameter `modified'.
	Consider segment as unmodified only if also MODIFIED is 0.  Set
	SEC_READONLY just according to WRITE.
	(objfile_find_memory_regions): Pass new values for the `modified'
	parameter.
	* gnu-nat.c (gnu_find_memory_regions): Pass -1 for the
	`modified' parameter.
	* linux-nat.c (read_mapping) <modified>: New.
	(linux_nat_find_memory_regions): New variable `modified'.  Try
	"/proc/%d/smaps" first.  Pass `&modified' to read_mapping.  Call func
	with MODIFIED.
	(linux_nat_info_proc_cmd): Pass NULL to read_mapping.
	* procfs.c (find_memory_regions_callback): Pass -1 for the `modified'
	parameter.

gdb/testsuite/
2010-08-30  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix gcore writer for -Wl,-z,relro.
	* gdb.base/gcore-relro.exp: New file.
	* gdb.base/gcore-relro-main.c: New file.
	* gdb.base/gcore-relro-lib.c: New file.

--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -634,9 +634,10 @@ extern void init_source_path (void);
 
 /* From exec.c */
 
+/* MODIFIED has value -1 for unknown, 0 for not modified, 1 for modified.  */
 typedef int (*find_memory_region_ftype) (CORE_ADDR addr, unsigned long size,
 					 int read, int write, int exec,
-					 void *data);
+					 int modified, void *data);
 
 /* Take over the 'find_mapped_memory' vector from exec.c. */
 extern void exec_set_find_memory_regions
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -133,7 +133,7 @@ fbsd_find_memory_regions (find_memory_region_ftype func, void *obfd)
 	}
 
       /* Invoke the callback function to create the corefile segment. */
-      func (start, size, read, write, exec, obfd);
+      func (start, size, read, write, exec, -1, obfd);
     }
 
   do_cleanups (cleanup);
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -378,8 +378,8 @@ make_output_phdrs (bfd *obfd, asection *osec, void *ignored)
 }
 
 static int
-gcore_create_callback (CORE_ADDR vaddr, unsigned long size,
-		       int read, int write, int exec, void *data)
+gcore_create_callback (CORE_ADDR vaddr, unsigned long size, int read,
+		       int write, int exec, int modified, void *data)
 {
   bfd *obfd = data;
   asection *osec;
@@ -388,7 +388,7 @@ gcore_create_callback (CORE_ADDR vaddr, unsigned long size,
   /* If the memory segment has no permissions set, ignore it, otherwise
      when we later try to access it for read/write, we'll get an error
      or jam the kernel.  */
-  if (read == 0 && write == 0 && exec == 0)
+  if (read == 0 && write == 0 && exec == 0 && modified == 0)
     {
       if (info_verbose)
         {
@@ -399,7 +399,7 @@ gcore_create_callback (CORE_ADDR vaddr, unsigned long size,
       return 0;
     }
 
-  if (write == 0 && !solib_keep_data_in_core (vaddr, size))
+  if (write == 0 && modified == 0 && !solib_keep_data_in_core (vaddr, size))
     {
       /* See if this region of memory lies inside a known file on disk.
 	 If so, we can avoid copying its contents by clearing SEC_LOAD.  */
@@ -432,10 +432,12 @@ gcore_create_callback (CORE_ADDR vaddr, unsigned long size,
 	    }
 	}
 
-    keep:
-      flags |= SEC_READONLY;
+    keep:;
     }
 
+  if (write == 0)
+    flags |= SEC_READONLY;
+
   if (exec)
     flags |= SEC_CODE;
   else
@@ -485,6 +487,7 @@ objfile_find_memory_regions (find_memory_region_ftype func, void *obfd)
 			 1, /* All sections will be readable.  */
 			 (flags & SEC_READONLY) == 0, /* Writable.  */
 			 (flags & SEC_CODE) != 0, /* Executable.  */
+			 -1, /* Modified is unknown.  */
 			 obfd);
 	  if (ret != 0)
 	    return ret;
@@ -497,6 +500,7 @@ objfile_find_memory_regions (find_memory_region_ftype func, void *obfd)
 	     1, /* Stack section will be readable.  */
 	     1, /* Stack section will be writable.  */
 	     0, /* Stack section will not be executable.  */
+	     1, /* Stack section will be modified.  */
 	     obfd);
 
   /* Make a heap segment. */
@@ -505,6 +509,7 @@ objfile_find_memory_regions (find_memory_region_ftype func, void *obfd)
 	     1, /* Heap section will be readable.  */
 	     1, /* Heap section will be writable.  */
 	     0, /* Heap section will not be executable.  */
+	     1, /* Heap section will be modified.  */
 	     obfd);
 
   return 0;
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2544,7 +2544,7 @@ gnu_find_memory_regions (find_memory_region_ftype func, void *data)
 		     last_protection & VM_PROT_READ,
 		     last_protection & VM_PROT_WRITE,
 		     last_protection & VM_PROT_EXECUTE,
-		     data);
+		     -1, data);
 	  last_region_address = region_address;
 	  last_region_end = region_address += region_length;
 	  last_protection = protection;
@@ -2557,7 +2557,7 @@ gnu_find_memory_regions (find_memory_region_ftype func, void *data)
 	     last_protection & VM_PROT_READ,
 	     last_protection & VM_PROT_WRITE,
 	     last_protection & VM_PROT_EXECUTE,
-	     data);
+	     -1, data);
 
   return 0;
 }
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4079,7 +4079,7 @@ read_mapping (FILE *mapfile,
 	      long long *endaddr,
 	      char *permissions,
 	      long long *offset,
-	      char *device, long long *inode, char *filename)
+	      char *device, long long *inode, char *filename, int *modified)
 {
   int ret = fscanf (mapfile, "%llx-%llx %s %llx %s %llx",
 		    addr, endaddr, permissions, offset, device, inode);
@@ -4097,6 +4097,40 @@ read_mapping (FILE *mapfile,
       ret += fscanf (mapfile, "%[^\n]\n", filename);
     }
 
+  if (modified != NULL)
+    {
+      *modified = -1;
+      for (;;)
+	{
+	  int ch;
+	  char keyword[64 + 1];
+	  unsigned long number;
+
+	  ch = fgetc (mapfile);
+	  if (ch != EOF)
+	    ungetc (ch, mapfile);
+	  if (ch < 'A' || ch > 'Z')
+	    break;
+	  if (fscanf (mapfile, "%64s%lu kB\n", keyword, &number) != 2)
+	    {
+	      warning (_("Error parsing /proc/PID/{s,}maps file"));
+	      do
+		ch = fgetc (mapfile);
+	      while (ch != EOF && ch != '\n');
+	      break;
+	    }
+	  if (number != 0 && (strcmp (keyword, "Shared_Dirty:") == 0
+			      || strcmp (keyword, "Private_Dirty:") == 0
+			      || strcmp (keyword, "Swap:") == 0))
+	    *modified = 1;
+	  else if (*modified == -1)
+	    {
+	      /* Valid line proves an smaps file is being read in.  */
+	      *modified = 0;
+	    }
+	}
+    }
+
   return (ret != 0 && ret != EOF);
 }
 
@@ -4111,13 +4145,17 @@ linux_nat_find_memory_regions (find_memory_region_ftype func, void *obfd)
   FILE *mapsfile;
   long long addr, endaddr, size, offset, inode;
   char permissions[8], device[8], filename[MAXPATHLEN];
-  int read, write, exec;
+  int read, write, exec, modified;
   struct cleanup *cleanup;
 
   /* Compose the filename for the /proc memory map, and open it.  */
-  sprintf (mapsfilename, "/proc/%d/maps", pid);
+  sprintf (mapsfilename, "/proc/%d/smaps", pid);
   if ((mapsfile = fopen (mapsfilename, "r")) == NULL)
-    error (_("Could not open %s."), mapsfilename);
+    {
+      sprintf (mapsfilename, "/proc/%d/maps", pid);
+      if ((mapsfile = fopen (mapsfilename, "r")) == NULL)
+	error (_("Could not open %s."), mapsfilename);
+    }
   cleanup = make_cleanup_fclose (mapsfile);
 
   if (info_verbose)
@@ -4126,7 +4164,7 @@ linux_nat_find_memory_regions (find_memory_region_ftype func, void *obfd)
 
   /* Now iterate until end-of-file.  */
   while (read_mapping (mapsfile, &addr, &endaddr, &permissions[0],
-		       &offset, &device[0], &inode, &filename[0]))
+		       &offset, &device[0], &inode, &filename[0], &modified))
     {
       size = endaddr - addr;
 
@@ -4149,7 +4187,7 @@ linux_nat_find_memory_regions (find_memory_region_ftype func, void *obfd)
 
       /* Invoke the callback function to create the corefile
 	 segment.  */
-      func (addr, size, read, write, exec, obfd);
+      func (addr, size, read, write, exec, modified, obfd);
     }
   do_cleanups (cleanup);
   return 0;
@@ -4612,7 +4650,8 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
 	    }
 
 	  while (read_mapping (procfile, &addr, &endaddr, &permissions[0],
-			       &offset, &device[0], &inode, &filename[0]))
+			       &offset, &device[0], &inode, &filename[0],
+			       NULL))
 	    {
 	      size = endaddr - addr;
 
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -5285,7 +5285,7 @@ find_memory_regions_callback (struct prmap *map,
 		  (map->pr_mflags & MA_READ) != 0,
 		  (map->pr_mflags & MA_WRITE) != 0,
 		  (map->pr_mflags & MA_EXEC) != 0,
-		  data);
+		  -1, data);
 }
 
 /* External interface.  Calls a callback function once for each
--- /dev/null
+++ b/gdb/testsuite/gdb.base/gcore-relro-lib.c
@@ -0,0 +1,21 @@
+/* Copyright 2010 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+void
+lib (void)
+{
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/gcore-relro-main.c
@@ -0,0 +1,25 @@
+/* Copyright 2010 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+extern void lib (void);
+
+int
+main (void)
+{
+  lib ();
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/gcore-relro.exp
@@ -0,0 +1,80 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if {[skip_shlib_tests]} {
+    return 0
+}
+
+set testfile "gcore-relro"
+set srcmainfile ${testfile}-main.c
+set srclibfile ${testfile}-lib.c
+set libfile ${objdir}/${subdir}/${testfile}-lib.so
+set objfile ${objdir}/${subdir}/${testfile}-main.o
+set executable ${testfile}-main
+set binfile ${objdir}/${subdir}/${executable}
+set gcorefile ${objdir}/${subdir}/${executable}.gcore
+
+if { [gdb_compile_shlib ${srcdir}/${subdir}/${srclibfile} ${libfile} {debug}] != ""
+     || [gdb_compile ${srcdir}/${subdir}/${srcmainfile} ${objfile} object {debug}] != "" } {
+     untested ${testfile}.exp
+     return -1
+}
+set opts [list debug shlib=${libfile} additional_flags=-Wl,-z,relro]
+if { [gdb_compile ${objfile} ${binfile} executable $opts] != "" } {
+     unsupported "-Wl,-z,relro compilation failed"
+     return -1
+}
+
+clean_restart $executable
+gdb_load_shlibs $libfile
+
+# Does this gdb support gcore?
+set test "help gcore"
+gdb_test_multiple $test $test {
+    -re "Undefined command: .gcore.*\r\n$gdb_prompt $" {
+	# gcore command not supported -- nothing to test here.
+	unsupported "gdb does not support gcore on this target"
+	return -1;
+    }
+    -re "Save a core file .*\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+
+if { ![runto lib] } then {
+    return -1
+}
+
+set escapedfilename [string_to_regexp ${gcorefile}]
+
+set test "save a corefile"
+gdb_test_multiple "gcore ${gcorefile}" $test {
+    -re "Saved corefile ${escapedfilename}\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re "Can't create a corefile\r\n$gdb_prompt $" {
+	unsupported $test
+	return -1
+    }
+}
+
+# Now restart gdb and load the corefile.
+
+clean_restart $executable
+gdb_load_shlibs $libfile
+
+gdb_test "core ${gcorefile}" "Core was generated by .*" "re-load generated corefile"
+
+gdb_test "frame" "#0 \[^\r\n\]* lib .*" "library got loaded"