public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [patch] Fix gcore writer for -Wl,-z,relro (PR corefiles/11804)
@ 2010-08-30  9:15 Jan Kratochvil
  2010-08-31 17:29 ` Jan Kratochvil
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Kratochvil @ 2010-08-30  9:15 UTC (permalink / raw)
  To: gdb-patches

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_t): 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_t) (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 (int (*func) (find_memory_region_t,
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -133,7 +133,7 @@ fbsd_find_memory_regions (find_memory_region_t 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_t 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_t 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_t 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_t 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_t 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_t 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_t 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_t 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, find_memory_region_t func,
 		  (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"

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] Fix gcore writer for -Wl,-z,relro (PR corefiles/11804)
  2010-08-30  9:15 [patch] Fix gcore writer for -Wl,-z,relro (PR corefiles/11804) Jan Kratochvil
@ 2010-08-31 17:29 ` Jan Kratochvil
  2010-09-23 14:47   ` Jan Kratochvil
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Kratochvil @ 2010-08-31 17:29 UTC (permalink / raw)
  To: gdb-patches

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"

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] Fix gcore writer for -Wl,-z,relro (PR corefiles/11804)
  2010-08-31 17:29 ` Jan Kratochvil
@ 2010-09-23 14:47   ` Jan Kratochvil
  2010-09-23 14:48     ` Ulrich Weigand
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Kratochvil @ 2010-09-23 14:47 UTC (permalink / raw)
  To: gdb-patches

Hi,

a slight warning() improvement.

There was a hint
	http://sourceware.org/bugzilla/show_bug.cgi?id=11804#c12
it may not always work right but that would be just a Linux kernel bug so that
GDB part should be OK.  Also it cannot be a regression, only more pages may be
written out with this patch.


Thanks,
Jan


On Tue, 31 Aug 2010 19:28:57 +0200, Jan Kratochvil wrote:
> technical rediff due to the patch update:
> 
> 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.


gdb/
2010-09-22  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): New parameters mapfilename and modified.
	(linux_nat_find_memory_regions): New variable `modified'.  Try
	"/proc/%d/smaps" first.  Pass `&modified' and `mapsfilename' to
	read_mapping.  Call func with MODIFIED.
	(linux_nat_info_proc_cmd): Pass `fname1' and NULL to read_mapping.
	* procfs.c (find_memory_regions_callback): Pass -1 for the `modified'
	parameter.

gdb/testsuite/
2010-09-22  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
@@ -638,9 +638,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.  */
@@ -431,10 +431,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
@@ -484,6 +486,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;
@@ -496,6 +499,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. */
@@ -504,6 +508,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
@@ -4078,12 +4078,9 @@ linux_child_pid_to_exec_file (int pid)
 /* Service function for corefiles and info proc.  */
 
 static int
-read_mapping (FILE *mapfile,
-	      long long *addr,
-	      long long *endaddr,
-	      char *permissions,
-	      long long *offset,
-	      char *device, long long *inode, char *filename)
+read_mapping (FILE *mapfile, const char *mapfilename, long long *addr,
+	      long long *endaddr, char *permissions, long long *offset,
+	      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);
@@ -4101,6 +4098,41 @@ read_mapping (FILE *mapfile,
       ret += fscanf (mapfile, "%[^\n]\n", filename);
     }
 
+  if (modified != NULL)
+    {
+      *modified = -1;
+      for (;;)
+	{
+	  int ch, got;
+	  char keyword[64 + 1];
+	  unsigned long number;
+
+	  ch = fgetc (mapfile);
+	  if (ch != EOF)
+	    ungetc (ch, mapfile);
+	  if (ch < 'A' || ch > 'Z')
+	    break;
+	  got = fscanf (mapfile, "%64s%lu", keyword, &number);
+	  do
+	    ch = fgetc (mapfile);
+	  while (ch != EOF && ch != '\n');
+	  if (got != 2)
+	    {
+	      warning (_("Error parsing file %s"), mapfilename);
+	      break;
+	    }
+	  if (number != 0 && (strcmp (keyword, "Shared_Dirty:") == 0
+			      || strcmp (keyword, "Private_Dirty:") == 0
+			      || strcmp (keyword, "Swap:") == 0))
+	    *modified = 1;
+	  else if (*modified == -1)
+	    {
+	      /* A valid line proves an smaps file is being read in.  */
+	      *modified = 0;
+	    }
+	}
+    }
+
   return (ret != 0 && ret != EOF);
 }
 
@@ -4115,13 +4147,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)
@@ -4129,8 +4165,9 @@ linux_nat_find_memory_regions (find_memory_region_ftype func, void *obfd)
 		      "Reading memory regions from %s\n", mapsfilename);
 
   /* Now iterate until end-of-file.  */
-  while (read_mapping (mapsfile, &addr, &endaddr, &permissions[0],
-		       &offset, &device[0], &inode, &filename[0]))
+  while (read_mapping (mapsfile, mapsfilename, &addr, &endaddr,
+		       &permissions[0], &offset, &device[0], &inode,
+		       &filename[0], &modified))
     {
       size = endaddr - addr;
 
@@ -4153,7 +4190,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;
@@ -4615,8 +4652,9 @@ linux_nat_info_proc_cmd (char *args, int from_tty)
 			   "      Size", "    Offset", "objfile");
 	    }
 
-	  while (read_mapping (procfile, &addr, &endaddr, &permissions[0],
-			       &offset, &device[0], &inode, &filename[0]))
+	  while (read_mapping (procfile, fname1, &addr, &endaddr,
+			       &permissions[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"

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch] Fix gcore writer for -Wl,-z,relro (PR corefiles/11804)
  2010-09-23 14:47   ` Jan Kratochvil
@ 2010-09-23 14:48     ` Ulrich Weigand
  2010-09-23 18:52       ` suspended: " Jan Kratochvil
  0 siblings, 1 reply; 5+ messages in thread
From: Ulrich Weigand @ 2010-09-23 14:48 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

Jan Kratochvil wrote:

> There was a hint
> 	http://sourceware.org/bugzilla/show_bug.cgi?id=11804#c12
> it may not always work right but that would be just a Linux kernel bug so that
> GDB part should be OK.  Also it cannot be a regression, only more pages may be
> written out with this patch.

[ I've been tracking down a Linux on z customer problem that turns out to be
exactly the same issue -- thanks for working on this! ]

It appears there are at least two problems with this approach currently
being discussed:

- The "dirty" pages count in /proc/.../smaps is sometimes wrong.  This is
  because the kernel currently only checks the per-PTE dirty bit, and not
  the per-page dirty bit.  This is drastically wrong on System z, because
  the platform *only* has a per-page hardware dirty bit; per-PTE dirty bit
  checks will always return "clean".  But on other systems there's a chance
  of mis-indication of the dirty state as well (e.g. if the page was dirtied
  by a syscall, or if a dirty page was swapped out, swapped back in, and
  subsequently removed from swap).

  This can indeed be considered a pure kernel bug that needs to be fixed.

- Even with the dirty accounting fixed, there are *still* cases where a page
  is genuinely clean as far as memory-management is concerned, but it was
  modified as compared to the file contents anyway.

  This happens if a page was swapped out and back in, but its contents
  remain present in swap space until the latter is reused (so-called "swap
  cache" pages).  These are legitimately considered clean pages because
  they can be simply dropped if needed -- the content is still there on
  the swap device.


To fix the second problem, it seems GDB will need to pursue a different
approach, for example by checking whether a mapping contains only file-backed
or also any anonymous pages (I'm told by kernel folks that swap cache pages
also count as anonymous).  However, this information is not readily available
in the general case (it's in /proc/kpageflags, but that's root-only, and it
is also in /proc/.../numa_maps, but only if NUMA support is enabled ...).

One proposal is to add a count of anonymous pages to /proc/.../smaps:
http://marc.info/?l=linux-kernel&m=128460743123381&w=2

If that goes in, I guess your patch should consider the new Anonymous:
field instead of (or maybe in addition to?) Shared_Dirty and Private_Dirty.
(Swap needs to be considered as well in either case.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* suspended: Re: [patch] Fix gcore writer for -Wl,-z,relro (PR corefiles/11804)
  2010-09-23 14:48     ` Ulrich Weigand
@ 2010-09-23 18:52       ` Jan Kratochvil
  0 siblings, 0 replies; 5+ messages in thread
From: Jan Kratochvil @ 2010-09-23 18:52 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: gdb-patches

On Thu, 23 Sep 2010 14:42:40 +0200, Ulrich Weigand wrote:
> If that goes in, I guess your patch should consider the new Anonymous:
> field instead of (or maybe in addition to?) Shared_Dirty and Private_Dirty.
> (Swap needs to be considered as well in either case.)

Therefore suspending now my FSF GDB patch proposal until it gets resolved for
Linux kernel first.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-09-23 17:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-30  9:15 [patch] Fix gcore writer for -Wl,-z,relro (PR corefiles/11804) Jan Kratochvil
2010-08-31 17:29 ` Jan Kratochvil
2010-09-23 14:47   ` Jan Kratochvil
2010-09-23 14:48     ` Ulrich Weigand
2010-09-23 18:52       ` suspended: " Jan Kratochvil

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