diff mbox

gdb/record-full: Use xmalloc instead of alloca for large buffers.

Message ID 1446489704-3173-1-git-send-email-koriakin@0x04.net
State New, archived
Headers

Commit Message

Marcin Kościelnicki Nov. 2, 2015, 6:41 p.m. UTC 
  On the newly added s390 target, it's possible for a single instruction
to write practically unbounded amount of memory (eg. MVCLE).  This caused
a stack overflow when alloca was used.

gdb/ChangeLog:

	* record-full.c (record_full_exec_insn): Use xmalloc for large buffers.
---
 gdb/ChangeLog     |  4 ++++
 gdb/record-full.c | 11 ++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

Comments

Joel Brobecker Nov. 3, 2015, 5:36 p.m. UTC | #1 
> > gdb/ChangeLog:
> > 
> > 	* record-full.c (record_full_exec_insn): Use xmalloc for large buffers.
> 
> I think this may leak memory if some code between the xmalloc and the xfree
> throws a GDB exception.  Usually, this is protected against by calling the
> xfree via the make_cleanup mechanism ...

Also, why not just call xmalloc every time instead of doing
a combination of alloca and xmalloc? I don't see enough benefits
to justify the extra complication.
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 38a42ea..f4c0f57 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@ 
+2015-11-02  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* record-full.c (record_full_exec_insn): Use xmalloc for large buffers.
+
 2015-10-30  Pedro Alves  <palves@redhat.com>
 
 	* breakpoint.c (breakpoint_in_range_p)
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 595e357..04d64ba 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -65,6 +65,8 @@ 
 
 #define RECORD_FULL_FILE_MAGIC	netorder32(0x20091016)
 
+#define RECORD_MEMORY_XMALLOC_THRESHOLD 0x1000
+
 /* These are the core structs of the process record functionality.
 
    A record_full_entry is a record of the value change of a register
@@ -726,7 +728,11 @@  record_full_exec_insn (struct regcache *regcache,
 	/* Nothing to do if the entry is flagged not_accessible.  */
         if (!entry->u.mem.mem_entry_not_accessible)
           {
-            gdb_byte *mem = (gdb_byte *) alloca (entry->u.mem.len);
+            gdb_byte *mem;
+	    if (entry->u.mem.len >= RECORD_MEMORY_XMALLOC_THRESHOLD)
+	      mem = (gdb_byte *) xmalloc (entry->u.mem.len);
+	    else
+	      mem = (gdb_byte *) alloca (entry->u.mem.len);
 
             if (record_debug > 1)
               fprintf_unfiltered (gdb_stdlog,
@@ -771,6 +777,9 @@  record_full_exec_insn (struct regcache *regcache,
 		      record_full_stop_reason = TARGET_STOPPED_BY_WATCHPOINT;
 		  }
               }
+
+	    if (entry->u.mem.len >= RECORD_MEMORY_XMALLOC_THRESHOLD)
+	      xfree(mem);
           }
       }
       break;