[PATCH 0/3] Fix for PR 17808 and some related changes

[PATCH 0/3] Fix for PR 17808 and some related changes

[Andreas Arnez]

As seen in PR 17808, a test case with a forged (invalid) core file can
crash GDB with an assertion failure when a register section has the
wrong size.  This patch series is about improving GDB's behavior in
such cases: Patch 1 fixes the reported problem for i386 as well as for
other targets; patch 2 avoids out-of-bounds accesses when an x86
".reg-xstate" section is too short, and patch 3 adds a warning when a
core file register section is larger than expected.

This is based on the "lazy approach" suggested here:

  https://sourceware.org/ml/gdb-patches/2015-01/msg00229.html

OK to apply?


Andreas Arnez (3):
  [PR corefiles/17808] Fix internal error when core file section is too
    big
  x86: Use correct .reg-xstate section size
  Warn if core file register section is larger than expected

 gdb/alphanbsd-tdep.c     |  4 +++-
 gdb/amd64-linux-tdep.c   |  2 +-
 gdb/amd64-tdep.c         |  4 ++--
 gdb/armbsd-tdep.c        |  4 +++-
 gdb/corelow.c            |  5 +++++
 gdb/gdbarch.h            |  6 ++++++
 gdb/gdbarch.sh           |  6 ++++++
 gdb/hppa-hpux-tdep.c     |  4 +++-
 gdb/hppaobsd-tdep.c      |  4 +++-
 gdb/i386-linux-tdep.c    |  3 +--
 gdb/i386-tdep.c          |  8 ++++----
 gdb/m68kbsd-tdep.c       |  4 +++-
 gdb/mips-linux-tdep.c    | 16 ++++++++--------
 gdb/mipsnbsd-tdep.c      |  4 +++-
 gdb/mn10300-linux-tdep.c |  8 ++++----
 gdb/regset.h             |  7 +++++++
 16 files changed, 62 insertions(+), 27 deletions(-)

-- 
1.8.4.2

[PATCH 1/3] [PR corefiles/17808] Fix internal error when core file section is too big

[Andreas Arnez]

As reported in PR 17808, a test case with a forged (invalid) core file
can crash GDB with an assertion failure.  In that particular case the
prstatus of an i386 core file looks like that from an AMD64 core file.
Consequently the respective regset supply function i386_supply_gregset
is invoked with a larger buffer than usual.  But i386_supply_gregset
asserts a specific buffer size, and this assertion fails.

The patch relaxes all buffer size assertions in regset supply
functions such that they merely check for a sufficiently large buffer.
For consistency the regset collect functions are adjusted as well.

gdb/ChangeLog:

	* gdbarch.sh (iterate_over_regset_sections_cb): Document this
	function type, particularly its SIZE parameter.
	* gdbarch.h: Regenerate.
	* amd64-tdep.c (amd64_supply_fpregset): In gdb_assert, compare
	actual against required size using ">=" instead of "==".
	(amd64_collect_fpregset): Likewise.
	* i386-tdep.c (i386_supply_gregset): Likewise.
	(i386_collect_gregset): Likewise.
	(i386_supply_fpregset): Likewise.
	(i386_collect_fpregset): Likewise.
	* mips-linux-tdep.c (mips_supply_gregset_wrapper): Likewise.
	(mips_fill_gregset_wrapper): Likewise.
	(mips_supply_fpregset_wrapper): Likewise.
	(mips_fill_fpregset_wrapper): Likewise.
	(mips64_supply_gregset_wrapper): Likewise.
	(mips64_fill_gregset_wrapper): Likewise.
	(mips64_supply_fpregset_wrapper): Likewise.
	(mips64_fill_fpregset_wrapper): Likewise.
	* mn10300-linux-tdep.c (am33_supply_gregset_method): Likewise.
	(am33_supply_fpregset_method): Likewise.
	(am33_collect_gregset_method): Likewise.
	(am33_collect_fpregset_method): Likewise.
---
 gdb/amd64-tdep.c         |  4 ++--
 gdb/gdbarch.h            |  6 ++++++
 gdb/gdbarch.sh           |  6 ++++++
 gdb/i386-tdep.c          |  8 ++++----
 gdb/mips-linux-tdep.c    | 16 ++++++++--------
 gdb/mn10300-linux-tdep.c |  8 ++++----
 6 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index fa658de..a661b88 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -2854,7 +2854,7 @@ amd64_supply_fpregset (const struct regset *regset, struct regcache *regcache,
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
-  gdb_assert (len == tdep->sizeof_fpregset);
+  gdb_assert (len >= tdep->sizeof_fpregset);
   amd64_supply_fxsave (regcache, regnum, fpregs);
 }
 
@@ -2871,7 +2871,7 @@ amd64_collect_fpregset (const struct regset *regset,
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
-  gdb_assert (len == tdep->sizeof_fpregset);
+  gdb_assert (len >= tdep->sizeof_fpregset);
   amd64_collect_fxsave (regcache, regnum, fpregs);
 }
 
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index b266530..b67d9f6 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -85,6 +85,12 @@ extern struct gdbarch *target_gdbarch (void);
 typedef int (iterate_over_objfiles_in_search_order_cb_ftype)
   (struct objfile *objfile, void *cb_data);
 
+/* Callback type for regset section iterators.  The callback usually
+   invokes the REGSET's supply or collect method, to which it must
+   pass a buffer with at least the given SIZE.  SECT_NAME is a BFD
+   section name, and HUMAN_NAME is used for diagnostic messages.
+   CB_DATA should have been passed unchanged through the iterator.  */
+
 typedef void (iterate_over_regset_sections_cb)
   (const char *sect_name, int size, const struct regset *regset,
    const char *human_name, void *cb_data);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index e12b8b0..cffefc5 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1239,6 +1239,12 @@ extern struct gdbarch *target_gdbarch (void);
 typedef int (iterate_over_objfiles_in_search_order_cb_ftype)
   (struct objfile *objfile, void *cb_data);
 
+/* Callback type for regset section iterators.  The callback usually
+   invokes the REGSET's supply or collect method, to which it must
+   pass a buffer with at least the given SIZE.  SECT_NAME is a BFD
+   section name, and HUMAN_NAME is used for diagnostic messages.
+   CB_DATA should have been passed unchanged through the iterator.  */
+
 typedef void (iterate_over_regset_sections_cb)
   (const char *sect_name, int size, const struct regset *regset,
    const char *human_name, void *cb_data);
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 7d174c4..1c8842c 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3727,7 +3727,7 @@ i386_supply_gregset (const struct regset *regset, struct regcache *regcache,
   const gdb_byte *regs = gregs;
   int i;
 
-  gdb_assert (len == tdep->sizeof_gregset);
+  gdb_assert (len >= tdep->sizeof_gregset);
 
   for (i = 0; i < tdep->gregset_num_regs; i++)
     {
@@ -3752,7 +3752,7 @@ i386_collect_gregset (const struct regset *regset,
   gdb_byte *regs = gregs;
   int i;
 
-  gdb_assert (len == tdep->sizeof_gregset);
+  gdb_assert (len >= tdep->sizeof_gregset);
 
   for (i = 0; i < tdep->gregset_num_regs; i++)
     {
@@ -3779,7 +3779,7 @@ i386_supply_fpregset (const struct regset *regset, struct regcache *regcache,
       return;
     }
 
-  gdb_assert (len == tdep->sizeof_fpregset);
+  gdb_assert (len >= tdep->sizeof_fpregset);
   i387_supply_fsave (regcache, regnum, fpregs);
 }
 
@@ -3802,7 +3802,7 @@ i386_collect_fpregset (const struct regset *regset,
       return;
     }
 
-  gdb_assert (len == tdep->sizeof_fpregset);
+  gdb_assert (len >= tdep->sizeof_fpregset);
   i387_collect_fsave (regcache, regnum, fpregs);
 }
 
diff --git a/gdb/mips-linux-tdep.c b/gdb/mips-linux-tdep.c
index 5239c37..fe45dcc 100644
--- a/gdb/mips-linux-tdep.c
+++ b/gdb/mips-linux-tdep.c
@@ -163,7 +163,7 @@ mips_supply_gregset_wrapper (const struct regset *regset,
 			     struct regcache *regcache,
 			     int regnum, const void *gregs, size_t len)
 {
-  gdb_assert (len == sizeof (mips_elf_gregset_t));
+  gdb_assert (len >= sizeof (mips_elf_gregset_t));
 
   mips_supply_gregset (regcache, (const mips_elf_gregset_t *)gregs);
 }
@@ -231,7 +231,7 @@ mips_fill_gregset_wrapper (const struct regset *regset,
 			   const struct regcache *regcache,
 			   int regnum, void *gregs, size_t len)
 {
-  gdb_assert (len == sizeof (mips_elf_gregset_t));
+  gdb_assert (len >= sizeof (mips_elf_gregset_t));
 
   mips_fill_gregset (regcache, (mips_elf_gregset_t *)gregs, regnum);
 }
@@ -268,7 +268,7 @@ mips_supply_fpregset_wrapper (const struct regset *regset,
 			      struct regcache *regcache,
 			      int regnum, const void *gregs, size_t len)
 {
-  gdb_assert (len == sizeof (mips_elf_fpregset_t));
+  gdb_assert (len >= sizeof (mips_elf_fpregset_t));
 
   mips_supply_fpregset (regcache, (const mips_elf_fpregset_t *)gregs);
 }
@@ -311,7 +311,7 @@ mips_fill_fpregset_wrapper (const struct regset *regset,
 			    const struct regcache *regcache,
 			    int regnum, void *gregs, size_t len)
 {
-  gdb_assert (len == sizeof (mips_elf_fpregset_t));
+  gdb_assert (len >= sizeof (mips_elf_fpregset_t));
 
   mips_fill_fpregset (regcache, (mips_elf_fpregset_t *)gregs, regnum);
 }
@@ -413,7 +413,7 @@ mips64_supply_gregset_wrapper (const struct regset *regset,
 			       struct regcache *regcache,
 			       int regnum, const void *gregs, size_t len)
 {
-  gdb_assert (len == sizeof (mips64_elf_gregset_t));
+  gdb_assert (len >= sizeof (mips64_elf_gregset_t));
 
   mips64_supply_gregset (regcache, (const mips64_elf_gregset_t *)gregs);
 }
@@ -484,7 +484,7 @@ mips64_fill_gregset_wrapper (const struct regset *regset,
 			     const struct regcache *regcache,
 			     int regnum, void *gregs, size_t len)
 {
-  gdb_assert (len == sizeof (mips64_elf_gregset_t));
+  gdb_assert (len >= sizeof (mips64_elf_gregset_t));
 
   mips64_fill_gregset (regcache, (mips64_elf_gregset_t *)gregs, regnum);
 }
@@ -533,7 +533,7 @@ mips64_supply_fpregset_wrapper (const struct regset *regset,
 				struct regcache *regcache,
 				int regnum, const void *gregs, size_t len)
 {
-  gdb_assert (len == sizeof (mips64_elf_fpregset_t));
+  gdb_assert (len >= sizeof (mips64_elf_fpregset_t));
 
   mips64_supply_fpregset (regcache, (const mips64_elf_fpregset_t *)gregs);
 }
@@ -611,7 +611,7 @@ mips64_fill_fpregset_wrapper (const struct regset *regset,
 			      const struct regcache *regcache,
 			      int regnum, void *gregs, size_t len)
 {
-  gdb_assert (len == sizeof (mips64_elf_fpregset_t));
+  gdb_assert (len >= sizeof (mips64_elf_fpregset_t));
 
   mips64_fill_fpregset (regcache, (mips64_elf_fpregset_t *)gregs, regnum);
 }
diff --git a/gdb/mn10300-linux-tdep.c b/gdb/mn10300-linux-tdep.c
index d92e93d..9ac6c15 100644
--- a/gdb/mn10300-linux-tdep.c
+++ b/gdb/mn10300-linux-tdep.c
@@ -90,7 +90,7 @@ am33_supply_gregset_method (const struct regset *regset,
   const mn10300_elf_greg_t *regp = (const mn10300_elf_greg_t *) gregs;
   int i;
 
-  gdb_assert (len == sizeof (mn10300_elf_gregset_t));
+  gdb_assert (len >= sizeof (mn10300_elf_gregset_t));
 
   switch (regnum) {
   case E_D0_REGNUM:
@@ -245,7 +245,7 @@ am33_supply_fpregset_method (const struct regset *regset,
 {
   const mn10300_elf_fpregset_t *fpregset = fpregs;
 
-  gdb_assert (len == sizeof (mn10300_elf_fpregset_t));
+  gdb_assert (len >= sizeof (mn10300_elf_fpregset_t));
 
   if (regnum == -1)
     {
@@ -278,7 +278,7 @@ am33_collect_gregset_method (const struct regset *regset,
   mn10300_elf_gregset_t *regp = gregs;
   int i;
 
-  gdb_assert (len == sizeof (mn10300_elf_gregset_t));
+  gdb_assert (len >= sizeof (mn10300_elf_gregset_t));
 
   switch (regnum) {
   case E_D0_REGNUM:
@@ -425,7 +425,7 @@ am33_collect_fpregset_method (const struct regset *regset,
 {
   mn10300_elf_fpregset_t *fpregset = fpregs;
 
-  gdb_assert (len == sizeof (mn10300_elf_fpregset_t));
+  gdb_assert (len >= sizeof (mn10300_elf_fpregset_t));
 
   if (regnum == -1)
     {
-- 
1.8.4.2

[PATCH 2/3] x86: Use correct .reg-xstate section size

[Andreas Arnez]

When reading the XSAVE extended state from an i386 or AMD64 core file,
the respective regset iterator requests a minimum section size of
zero.  Since the respective regset supply function does not check the
size either, this may lead to accessing data out of range if the
section is too short.

In write mode, the iterator always uses the maximum supported size for
the XSAVE extended state.

This is now changed such that the iterator always requests the
expected size of this section based on xcr0, both for reading and
writing.

gdb/ChangeLog:

	* amd64-linux-tdep.c (amd64_linux_iterate_over_regset_sections):
	For ".reg-xstate", explicitly specify the requested section size
	via X86_XSTATE_SIZE instead of just 0 on input and
	X86_XSTATE_MAX_SIZE on output.
	* i386-linux-tdep.c (i386_linux_iterate_over_regset_sections):
	Likewise.
---
 gdb/amd64-linux-tdep.c | 2 +-
 gdb/i386-linux-tdep.c  | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 42d884e..5db04c0 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -1639,7 +1639,7 @@ amd64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
 
   cb (".reg", 27 * 8, &i386_gregset, NULL, cb_data);
   cb (".reg2", 512, &amd64_fpregset, NULL, cb_data);
-  cb (".reg-xstate",  regcache ? X86_XSTATE_MAX_SIZE : 0,
+  cb (".reg-xstate",  X86_XSTATE_SIZE (tdep->xcr0),
       &amd64_linux_xstateregset, "XSAVE extended state", cb_data);
 }
 
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 8a5209a..4a0ce60 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -690,8 +690,7 @@ i386_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
   cb (".reg", 68, &i386_gregset, NULL, cb_data);
 
   if (tdep->xcr0 & X86_XSTATE_AVX)
-    /* Use max size for writing, accept any size when reading.  */
-    cb (".reg-xstate", regcache ? X86_XSTATE_MAX_SIZE : 0,
+    cb (".reg-xstate", X86_XSTATE_SIZE (tdep->xcr0),
 	&i386_linux_xstateregset, "XSAVE extended state", cb_data);
   else if (tdep->xcr0 & X86_XSTATE_SSE)
     cb (".reg-xfp", 512, &i386_fpregset, "extended floating-point",
-- 
1.8.4.2

[PATCH 3/3] Warn if core file register section is larger than expected

[Andreas Arnez]

When reading a core file register section which is larger than
expected, emit a warning.  Assume that a register section usually has
exactly the size specified by the regset section iterator.  In some
special cases this assumption is wrong, or at least does not match the
regset supply function's logic.  Thus also add a way to suppress the
warning in those cases, using a new flag REGSET_VARIABLE_SIZE.

gdb/ChangeLog:

	* regset.h (struct regset): Add flags field.
	(REGSET_VARIABLE_SIZE): New value for a regset's flags field.
	* corelow.c (get_core_register_section): Add warning if the size
	exceeds the requested size and the regset does not have the
	REGSET_VARIABLE_SIZE flag set.
	* alphanbsd-tdep.c (alphanbsd_gregset): Add REGSET_VARIABLE_SIZE
	flag.
	* armbsd-tdep.c (armbsd_gregset): Likewise.
	* hppa-hpux-tdep.c (hppa_hpux_regset): Likewise.
	* hppaobsd-tdep.c (hppaobsd_gregset): Likewise.
	* m68kbsd-tdep.c (m68kbsd_gregset): Likewise.
	* mipsnbsd-tdep.c (mipsnbsd_gregset): Likewise.
---
 gdb/alphanbsd-tdep.c | 4 +++-
 gdb/armbsd-tdep.c    | 4 +++-
 gdb/corelow.c        | 5 +++++
 gdb/hppa-hpux-tdep.c | 4 +++-
 gdb/hppaobsd-tdep.c  | 4 +++-
 gdb/m68kbsd-tdep.c   | 4 +++-
 gdb/mipsnbsd-tdep.c  | 4 +++-
 gdb/regset.h         | 7 +++++++
 8 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/gdb/alphanbsd-tdep.c b/gdb/alphanbsd-tdep.c
index 6b13fad..69be265 100644
--- a/gdb/alphanbsd-tdep.c
+++ b/gdb/alphanbsd-tdep.c
@@ -145,7 +145,9 @@ alphanbsd_supply_gregset (const struct regset *regset,
 static const struct regset alphanbsd_gregset =
 {
   NULL,
-  alphanbsd_supply_gregset
+  alphanbsd_supply_gregset,
+  NULL,
+  REGSET_VARIABLE_SIZE
 };
 
 static const struct regset alphanbsd_fpregset =
diff --git a/gdb/armbsd-tdep.c b/gdb/armbsd-tdep.c
index 7923cad..c043b51 100644
--- a/gdb/armbsd-tdep.c
+++ b/gdb/armbsd-tdep.c
@@ -98,7 +98,9 @@ armbsd_supply_gregset (const struct regset *regset,
 static const struct regset armbsd_gregset =
 {
   NULL,
-  armbsd_supply_gregset
+  armbsd_supply_gregset,
+  NULL,
+  REGSET_VARIABLE_SIZE
 };
 
 static const struct regset armbsd_fpregset =
diff --git a/gdb/corelow.c b/gdb/corelow.c
index a9eadd5..12cbcba 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -523,6 +523,11 @@ get_core_register_section (struct regcache *regcache,
       warning (_("Section `%s' in core file too small."), section_name);
       return;
     }
+  if (size != min_size && !(regset->flags & REGSET_VARIABLE_SIZE))
+    {
+      warning (_("Unexpected size of section `%s' in core file."),
+	       section_name);
+    }
 
   contents = alloca (size);
   if (! bfd_get_section_contents (core_bfd, section, contents,
diff --git a/gdb/hppa-hpux-tdep.c b/gdb/hppa-hpux-tdep.c
index 0c8575d..3c0f390 100644
--- a/gdb/hppa-hpux-tdep.c
+++ b/gdb/hppa-hpux-tdep.c
@@ -1367,7 +1367,9 @@ hppa_hpux_supply_save_state (const struct regset *regset,
 static const struct regset hppa_hpux_regset =
 {
   NULL,
-  hppa_hpux_supply_save_state
+  hppa_hpux_supply_save_state,
+  NULL,
+  REGSET_VARIABLE_SIZE
 };
 
 static void
diff --git a/gdb/hppaobsd-tdep.c b/gdb/hppaobsd-tdep.c
index 9ec7fdf..c9bc1bf 100644
--- a/gdb/hppaobsd-tdep.c
+++ b/gdb/hppaobsd-tdep.c
@@ -131,7 +131,9 @@ hppaobsd_supply_fpregset (const struct regset *regset,
 static const struct regset hppaobsd_gregset =
 {
   NULL,
-  hppaobsd_supply_gregset
+  hppaobsd_supply_gregset,
+  NULL,
+  REGSET_VARIABLE_SIZE
 };
 
 static const struct regset hppaobsd_fpregset =
diff --git a/gdb/m68kbsd-tdep.c b/gdb/m68kbsd-tdep.c
index b1ae4c1..ae0cecf 100644
--- a/gdb/m68kbsd-tdep.c
+++ b/gdb/m68kbsd-tdep.c
@@ -105,7 +105,9 @@ m68kbsd_supply_gregset (const struct regset *regset,
 static const struct regset m68kbsd_gregset =
 {
   NULL,
-  m68kbsd_supply_gregset
+  m68kbsd_supply_gregset,
+  NULL,
+  REGSET_VARIABLE_SIZE
 };
 
 static const struct regset m68kbsd_fpregset =
diff --git a/gdb/mipsnbsd-tdep.c b/gdb/mipsnbsd-tdep.c
index ee68f3d..d15c88c 100644
--- a/gdb/mipsnbsd-tdep.c
+++ b/gdb/mipsnbsd-tdep.c
@@ -103,7 +103,9 @@ mipsnbsd_supply_gregset (const struct regset *regset,
 static const struct regset mipsnbsd_gregset =
 {
   NULL,
-  mipsnbsd_supply_gregset
+  mipsnbsd_supply_gregset,
+  NULL,
+  REGSET_VARIABLE_SIZE
 };
 
 static const struct regset mipsnbsd_fpregset =
diff --git a/gdb/regset.h b/gdb/regset.h
index 3585322..d6edaa5 100644
--- a/gdb/regset.h
+++ b/gdb/regset.h
@@ -43,6 +43,13 @@ struct regset
 
   /* Function collecting values in a register set from a register cache.  */
   collect_regset_ftype *collect_regset;
+
+  unsigned flags;
 };
 
+/* Values for a regset's 'flags' field.  */
+
+#define REGSET_VARIABLE_SIZE 1	/* Accept a larger regset section size
+				   in a core file without warning.  */
+
 #endif /* regset.h */
-- 
1.8.4.2

[PING] [PATCH 0/3] Fix for PR 17808 and some related changes

[Andreas Arnez]

Ping:

  https://sourceware.org/ml/gdb-patches/2015-01/msg00424.html

Also, here is Jan's test case that showed the problem:

  https://sourceware.org/ml/gdb-patches/2015-01/msg00199.html

On Thu, Jan 15 2015, Andreas Arnez wrote:

> As seen in PR 17808, a test case with a forged (invalid) core file can
> crash GDB with an assertion failure when a register section has the
> wrong size.  This patch series is about improving GDB's behavior in
> such cases: Patch 1 fixes the reported problem for i386 as well as for
> other targets; patch 2 avoids out-of-bounds accesses when an x86
> ".reg-xstate" section is too short, and patch 3 adds a warning when a
> core file register section is larger than expected.
>
> This is based on the "lazy approach" suggested here:
>
>   https://sourceware.org/ml/gdb-patches/2015-01/msg00229.html
>
> OK to apply?
>
>
> Andreas Arnez (3):
>   [PR corefiles/17808] Fix internal error when core file section is too
>     big
>   x86: Use correct .reg-xstate section size
>   Warn if core file register section is larger than expected
>
>  gdb/alphanbsd-tdep.c     |  4 +++-
>  gdb/amd64-linux-tdep.c   |  2 +-
>  gdb/amd64-tdep.c         |  4 ++--
>  gdb/armbsd-tdep.c        |  4 +++-
>  gdb/corelow.c            |  5 +++++
>  gdb/gdbarch.h            |  6 ++++++
>  gdb/gdbarch.sh           |  6 ++++++
>  gdb/hppa-hpux-tdep.c     |  4 +++-
>  gdb/hppaobsd-tdep.c      |  4 +++-
>  gdb/i386-linux-tdep.c    |  3 +--
>  gdb/i386-tdep.c          |  8 ++++----
>  gdb/m68kbsd-tdep.c       |  4 +++-
>  gdb/mips-linux-tdep.c    | 16 ++++++++--------
>  gdb/mipsnbsd-tdep.c      |  4 +++-
>  gdb/mn10300-linux-tdep.c |  8 ++++----
>  gdb/regset.h             |  7 +++++++
>  16 files changed, 62 insertions(+), 27 deletions(-)

Re: [PATCH 0/3] Fix for PR 17808 and some related changes

[Pedro Alves]

On 01/15/2015 03:21 PM, Andreas Arnez wrote:
> As seen in PR 17808, a test case with a forged (invalid) core file can
> crash GDB with an assertion failure when a register section has the
> wrong size.  This patch series is about improving GDB's behavior in
> such cases: Patch 1 fixes the reported problem for i386 as well as for
> other targets; patch 2 avoids out-of-bounds accesses when an x86
> ".reg-xstate" section is too short, and patch 3 adds a warning when a
> core file register section is larger than expected.
> 
> This is based on the "lazy approach" suggested here:
> 
>   https://sourceware.org/ml/gdb-patches/2015-01/msg00229.html
> 
> OK to apply?

This looks good to me.  Mark, any comments?

Thanks,
Pedro Alves

Re: [PING] [PATCH 0/3] Fix for PR 17808 and some related changes

[Joel Brobecker]

> >> Sounds good to me, but I have a counter proposal.
> >> 
> >> How about we push it all to master now?  That'd give it exposure on
> >> both auto testers and on others' machines immediately.  We can always
> >> address any additional comments as follow ups, of course.  The main
> >> difference is that the series would be exposed to testing one
> >> extra week.  Then if we see no fall out, we'd have a little more
> >> confidence pushing to 7.9.
> >
> > Sounds even better to me.
> >
> > Andreas, please push your changes now.
> 
> Done.  Thanks for your support!

And, for the record, I have just pushed the patch to gdb-7.9-branch
as well. Given that I was not having my best day yesterday, I re-
tested the patch on x86_64-linux before pushing.

-- 
Joel

Re: [PING] [PATCH 0/3] Fix for PR 17808 and some related changes

[Andreas Arnez]

On Wed, Feb 04 2015, Joel Brobecker wrote:

>> Sounds good to me, but I have a counter proposal.
>> 
>> How about we push it all to master now?  That'd give it exposure on
>> both auto testers and on others' machines immediately.  We can always
>> address any additional comments as follow ups, of course.  The main
>> difference is that the series would be exposed to testing one
>> extra week.  Then if we see no fall out, we'd have a little more
>> confidence pushing to 7.9.
>
> Sounds even better to me.
>
> Andreas, please push your changes now.

Done.  Thanks for your support!

BTW, do we also want to get the test case upstream that triggered this
PR?  Jan posted it here:

  https://sourceware.org/ml/gdb-patches/2015-01/msg00199.html

Re: [PING] [PATCH 0/3] Fix for PR 17808 and some related changes

[Joel Brobecker]

> Sounds good to me, but I have a counter proposal.
> 
> How about we push it all to master now?  That'd give it exposure on
> both auto testers and on others' machines immediately.  We can always
> address any additional comments as follow ups, of course.  The main
> difference is that the series would be exposed to testing one
> extra week.  Then if we see no fall out, we'd have a little more
> confidence pushing to 7.9.

Sounds even better to me.

Andreas, please push your changes now.

Thank you both!
-- 
Joel

Re: [PING] [PATCH 0/3] Fix for PR 17808 and some related changes

[Pedro Alves]

On 02/02/2015 04:50 AM, Joel Brobecker wrote:
> On Thu, Jan 29, 2015 at 04:58:07PM +0100, Andreas Arnez wrote:
>> Ping:
>>
>>   https://sourceware.org/ml/gdb-patches/2015-01/msg00424.html
>>
>> Pedro commented already, as shown below.  Mark has not replied so far.
>>
>> Note that the PR is on the TODO list for 7.9.  In my opinion at least
>> patch 1 should go in 7.9; it's fairly harmless and should fix the PR.
>> Patch 2 fixes a similar problem, but there might be a test gap, because
>> I currently don't have access to appropriate test hardware (with AVX and
>> AVX-512).  And with patch 3 there's a slight chance of introducing bogus
>> warnings when reading core files.  Thus I'd suggest to push all three
>> patches upstream, but only patch 1 into 7.9.  OK?
> 
> Let's give it another week for additional comments (or request for
> more time to review), and then push it to master.
> 
> For 7.9, let's confirm your assessment with Pedro. It sounds pretty
> good to me.
> 

Sounds good to me, but I have a counter proposal.

How about we push it all to master now?  That'd give it exposure on
both auto testers and on others' machines immediately.  We can always
address any additional comments as follow ups, of course.  The main
difference is that the series would be exposed to testing one
extra week.  Then if we see no fall out, we'd have a little more
confidence pushing to 7.9.

Thanks,
Pedro Alves

Re: [PING] [PATCH 0/3] Fix for PR 17808 and some related changes

[Joel Brobecker]

On Thu, Jan 29, 2015 at 04:58:07PM +0100, Andreas Arnez wrote:
> Ping:
> 
>   https://sourceware.org/ml/gdb-patches/2015-01/msg00424.html
> 
> Pedro commented already, as shown below.  Mark has not replied so far.
> 
> Note that the PR is on the TODO list for 7.9.  In my opinion at least
> patch 1 should go in 7.9; it's fairly harmless and should fix the PR.
> Patch 2 fixes a similar problem, but there might be a test gap, because
> I currently don't have access to appropriate test hardware (with AVX and
> AVX-512).  And with patch 3 there's a slight chance of introducing bogus
> warnings when reading core files.  Thus I'd suggest to push all three
> patches upstream, but only patch 1 into 7.9.  OK?

Let's give it another week for additional comments (or request for
more time to review), and then push it to master.

For 7.9, let's confirm your assessment with Pedro. It sounds pretty
good to me.

-- 
Joel

[PING] [PATCH 0/3] Fix for PR 17808 and some related changes

[Andreas Arnez]

Ping:

  https://sourceware.org/ml/gdb-patches/2015-01/msg00424.html

Pedro commented already, as shown below.  Mark has not replied so far.

Note that the PR is on the TODO list for 7.9.  In my opinion at least
patch 1 should go in 7.9; it's fairly harmless and should fix the PR.
Patch 2 fixes a similar problem, but there might be a test gap, because
I currently don't have access to appropriate test hardware (with AVX and
AVX-512).  And with patch 3 there's a slight chance of introducing bogus
warnings when reading core files.  Thus I'd suggest to push all three
patches upstream, but only patch 1 into 7.9.  OK?

On Fri, Jan 23 2015, Pedro Alves wrote:

> On 01/15/2015 03:21 PM, Andreas Arnez wrote:
>> As seen in PR 17808, a test case with a forged (invalid) core file can
>> crash GDB with an assertion failure when a register section has the
>> wrong size.  This patch series is about improving GDB's behavior in
>> such cases: Patch 1 fixes the reported problem for i386 as well as for
>> other targets; patch 2 avoids out-of-bounds accesses when an x86
>> ".reg-xstate" section is too short, and patch 3 adds a warning when a
>> core file register section is larger than expected.
>> 
>> This is based on the "lazy approach" suggested here:
>> 
>>   https://sourceware.org/ml/gdb-patches/2015-01/msg00229.html
>> 
>> OK to apply?
>
> This looks good to me.  Mark, any comments?
>
> Thanks,
> Pedro Alves