[Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Hui Zhu]

Hi guys,

This patch will make linux-record can check if the sys_brk will
release the memory or not.  If memory will be released, gdb will query
to user.

For example:
cat m.c
#include <sys/types.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdint.h>

int
main(int argc,char *argv[],char *envp[])
{
	sbrk (10);
	sbrk (-10);

	return (0);
}

gdb m
GNU gdb (GDB) 6.8.50.20090505-cvs
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-pc-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
(gdb) start
Temporary breakpoint 1 at 0x8048385: file m.c, line 14.
Starting program: /home/teawater/gdb/m

Temporary breakpoint 1, main (argc=<value optimized out>, argv=<value
optimized out>,
    envp=<value optimized out>) at m.c:14
14		sbrk (10);
(gdb) record
(gdb) n
15		sbrk (-10);
(gdb)
The next instruction is syscall brk.  It will release the memory that
will cause process record target get error.  Do you want to stop the
inferior?([y] or n)
Process record: inferior program stopped.

Program received signal SIGTRAP, Trace/breakpoint trap.
0xb7fe3405 in __kernel_vsyscall ()


2009-05-05  Hui Zhu  <teawater@gmail.com>

	Add a architecture process record and replay reset interface
	and i386 and i386-linux record and replay reset functions.

	* gdbarch.sh (process_record_reset): This interface point to
	the function that reset the architecture process record and
	replay.
	* record.c (record_open): Call process_record_reset.
	* i386-tdep.c (i386_linux_record_reset): New function. Call
	tdep interface "i386_record_reset".
	(i386_gdbarch_init): Set "i386_linux_record_reset" to GDBARCH
	"process_record_reset" interface.
	* i386-tdep.h (gdbarch_tdep): New function pointer
	i386_record_reset that point to the function that can reset
	the process record.
	* i386-linux-tdep.c (i386_linux_record_reset): New function.
	Call record_linux_reset.
	(i386_linux_init_abi): Set "i386_linux_record_reset" to
	"i386_record_reset".

	Check Linux sys_brk release memory in process record
	and replay.

	* linux-record.c (record_top_of_heap): New variable.
	The current top of heap of inferior.
	(record_linux_reset): New function.  The reset function of
	Linux process record and replay.  It will reset the value
	of record_top_of_heap.
	(record_linux_system_call): Add the sys_brk check code.
	If this sys_brk will release the memory, query to user.
	* linux-record.h (record_linux_reset): New function extern.


Thanks,
Hui

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Hui Zhu]

[-- Attachment #1: Type: text/plain, Size: 3426 bytes --]

Sorry I forget the patch.

On Tue, May 5, 2009 at 21:07, Hui Zhu <teawater@gmail.com> wrote:
> Hi guys,
>
> This patch will make linux-record can check if the sys_brk will
> release the memory or not.  If memory will be released, gdb will query
> to user.
>
> For example:
> cat m.c
> #include <sys/types.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <errno.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <stdint.h>
>
> int
> main(int argc,char *argv[],char *envp[])
> {
>        sbrk (10);
>        sbrk (-10);
>
>        return (0);
> }
>
> gdb m
> GNU gdb (GDB) 6.8.50.20090505-cvs
> Copyright (C) 2009 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "i686-pc-linux-gnu".
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>...
> (gdb) start
> Temporary breakpoint 1 at 0x8048385: file m.c, line 14.
> Starting program: /home/teawater/gdb/m
>
> Temporary breakpoint 1, main (argc=<value optimized out>, argv=<value
> optimized out>,
>    envp=<value optimized out>) at m.c:14
> 14              sbrk (10);
> (gdb) record
> (gdb) n
> 15              sbrk (-10);
> (gdb)
> The next instruction is syscall brk.  It will release the memory that
> will cause process record target get error.  Do you want to stop the
> inferior?([y] or n)
> Process record: inferior program stopped.
>
> Program received signal SIGTRAP, Trace/breakpoint trap.
> 0xb7fe3405 in __kernel_vsyscall ()
>
>
> 2009-05-05  Hui Zhu  <teawater@gmail.com>
>
>        Add a architecture process record and replay reset interface
>        and i386 and i386-linux record and replay reset functions.
>
>        * gdbarch.sh (process_record_reset): This interface point to
>        the function that reset the architecture process record and
>        replay.
>        * record.c (record_open): Call process_record_reset.
>        * i386-tdep.c (i386_linux_record_reset): New function. Call
>        tdep interface "i386_record_reset".
>        (i386_gdbarch_init): Set "i386_linux_record_reset" to GDBARCH
>        "process_record_reset" interface.
>        * i386-tdep.h (gdbarch_tdep): New function pointer
>        i386_record_reset that point to the function that can reset
>        the process record.
>        * i386-linux-tdep.c (i386_linux_record_reset): New function.
>        Call record_linux_reset.
>        (i386_linux_init_abi): Set "i386_linux_record_reset" to
>        "i386_record_reset".
>
>        Check Linux sys_brk release memory in process record
>        and replay.
>
>        * linux-record.c (record_top_of_heap): New variable.
>        The current top of heap of inferior.
>        (record_linux_reset): New function.  The reset function of
>        Linux process record and replay.  It will reset the value
>        of record_top_of_heap.
>        (record_linux_system_call): Add the sys_brk check code.
>        If this sys_brk will release the memory, query to user.
>        * linux-record.h (record_linux_reset): New function extern.
>
>
> Thanks,
> Hui
>

[-- Attachment #2: check-brk.txt --]
[-- Type: text/plain, Size: 10478 bytes --]

---
 gdbarch.c         |   33 +++++++++++++++++
 gdbarch.h         |    8 ++++
 gdbarch.sh        |    3 +
 i386-linux-tdep.c |    7 +++
 i386-tdep.c       |    8 ++++
 i386-tdep.h       |    2 +
 linux-record.c    |  101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 linux-record.h    |    1 
 record.c          |    4 ++
 9 files changed, 167 insertions(+)

--- a/gdbarch.c
+++ b/gdbarch.c
@@ -240,6 +240,7 @@ struct gdbarch
   gdbarch_static_transform_name_ftype *static_transform_name;
   int sofun_address_maybe_missing;
   gdbarch_process_record_ftype *process_record;
+  gdbarch_process_record_reset_ftype *process_record_reset;
   gdbarch_target_signal_from_host_ftype *target_signal_from_host;
   gdbarch_target_signal_to_host_ftype *target_signal_to_host;
   gdbarch_get_siginfo_type_ftype *get_siginfo_type;
@@ -376,6 +377,7 @@ struct gdbarch startup_gdbarch =
   0,  /* static_transform_name */
   0,  /* sofun_address_maybe_missing */
   0,  /* process_record */
+  0,  /* process_record_reset */
   default_target_signal_from_host,  /* target_signal_from_host */
   default_target_signal_to_host,  /* target_signal_to_host */
   0,  /* get_siginfo_type */
@@ -633,6 +635,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of static_transform_name, has predicate */
   /* Skip verify of sofun_address_maybe_missing, invalid_p == 0 */
   /* Skip verify of process_record, has predicate */
+  /* Skip verify of process_record_reset, has predicate */
   /* Skip verify of target_signal_from_host, invalid_p == 0 */
   /* Skip verify of target_signal_to_host, invalid_p == 0 */
   /* Skip verify of get_siginfo_type, has predicate */
@@ -955,6 +958,12 @@ gdbarch_dump (struct gdbarch *gdbarch, s
                       "gdbarch_dump: process_record = <%s>\n",
                       host_address_to_string (gdbarch->process_record));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_process_record_reset_p() = %d\n",
+                      gdbarch_process_record_reset_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: process_record_reset = <%s>\n",
+                      host_address_to_string (gdbarch->process_record_reset));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: ps_regnum = %s\n",
                       plongest (gdbarch->ps_regnum));
   fprintf_unfiltered (file,
@@ -3283,6 +3292,30 @@ set_gdbarch_process_record (struct gdbar
   gdbarch->process_record = process_record;
 }
 
+int
+gdbarch_process_record_reset_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->process_record_reset != NULL;
+}
+
+void
+gdbarch_process_record_reset (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->process_record_reset != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_process_record_reset called\n");
+  gdbarch->process_record_reset (gdbarch);
+}
+
+void
+set_gdbarch_process_record_reset (struct gdbarch *gdbarch,
+                                  gdbarch_process_record_reset_ftype process_record_reset)
+{
+  gdbarch->process_record_reset = process_record_reset;
+}
+
 enum target_signal
 gdbarch_target_signal_from_host (struct gdbarch *gdbarch, int signo)
 {
--- a/gdbarch.h
+++ b/gdbarch.h
@@ -818,6 +818,14 @@ typedef int (gdbarch_process_record_ftyp
 extern int gdbarch_process_record (struct gdbarch *gdbarch, struct regcache *regcache, CORE_ADDR addr);
 extern void set_gdbarch_process_record (struct gdbarch *gdbarch, gdbarch_process_record_ftype *process_record);
 
+/* Reset the inside value of process record if need. */
+
+extern int gdbarch_process_record_reset_p (struct gdbarch *gdbarch);
+
+typedef void (gdbarch_process_record_reset_ftype) (struct gdbarch *gdbarch);
+extern void gdbarch_process_record_reset (struct gdbarch *gdbarch);
+extern void set_gdbarch_process_record_reset (struct gdbarch *gdbarch, gdbarch_process_record_reset_ftype *process_record_reset);
+
 /* Signal translation: translate inferior's signal (host's) number into
    GDB's representation. */
 
--- a/gdbarch.sh
+++ b/gdbarch.sh
@@ -715,6 +715,9 @@ v:int:sofun_address_maybe_missing:::0:0:
 # Return -1 if something goes wrong, 0 otherwise.
 M:int:process_record:struct regcache *regcache, CORE_ADDR addr:regcache, addr
 
+# Reset the inside value of process record if need.
+M:void:process_record_reset:void
+
 # Signal translation: translate inferior's signal (host's) number into
 # GDB's representation.
 m:enum target_signal:target_signal_from_host:int signo:signo::default_target_signal_from_host::0
--- a/i386-linux-tdep.c
+++ b/i386-linux-tdep.c
@@ -352,6 +352,12 @@ i386_linux_write_pc (struct regcache *re
   regcache_cooked_write_unsigned (regcache, I386_LINUX_ORIG_EAX_REGNUM, -1);
 }
 
+static void
+i386_linux_record_reset (struct gdbarch *gdbarch)
+{
+  record_linux_reset (gdbarch);
+}
+
 /* Parse the arguments of current system call instruction and record
    the values of the registers and memory that will be changed into
    "record_arch_list".  This instruction is "int 0x80" (Linux
@@ -787,6 +793,7 @@ i386_linux_init_abi (struct gdbarch_info
   i386_linux_record_tdep.arg4 = I386_ESI_REGNUM;
   i386_linux_record_tdep.arg5 = I386_EDI_REGNUM;
 
+  tdep->i386_record_reset = i386_linux_record_reset;
   tdep->i386_intx80_record = i386_linux_intx80_sysenter_record;
   tdep->i386_sysenter_record = i386_linux_intx80_sysenter_record;
 
--- a/i386-tdep.c
+++ b/i386-tdep.c
@@ -5086,6 +5086,13 @@ no_support:
   return -1;
 }
 
+static void
+i386_process_record_reset (struct gdbarch *gdbarch)
+{
+  if (gdbarch_tdep (gdbarch)->i386_record_reset)
+    gdbarch_tdep (gdbarch)->i386_record_reset (gdbarch);
+}
+
 \f
 static struct gdbarch *
 i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
@@ -5278,6 +5285,7 @@ i386_gdbarch_init (struct gdbarch_info i
 					 i386_skip_permanent_breakpoint);
 
   set_gdbarch_process_record (gdbarch, i386_process_record);
+  set_gdbarch_process_record_reset (gdbarch, i386_process_record_reset);
 
   return gdbarch;
 }
--- a/i386-tdep.h
+++ b/i386-tdep.h
@@ -108,6 +108,8 @@ struct gdbarch_tdep
   struct type *i386_sse_type;
 
   /* Process record/replay target.  */
+  /* Reset */
+  void (*i386_record_reset) (struct gdbarch *gdbarch);
   /* Parse intx80 args.  */
   int (*i386_intx80_record) (struct regcache *regcache);
   /* Parse sysenter args.  */
--- a/linux-record.c
+++ b/linux-record.c
@@ -20,6 +20,12 @@
 #include "defs.h"
 #include "target.h"
 #include "regcache.h"
+#include "infcall.h"
+#include "objfiles.h"
+#include "value.h"
+#include "breakpoint.h"
+#include "inferior.h"
+#include "gdbthread.h"
 #include "record.h"
 #include "linux-record.h"
 
@@ -80,6 +86,56 @@
 #define RECORD_Q_XGETQSTAT	(('5' << 8) + 5)
 #define RECORD_Q_XGETQUOTA	(('3' << 8) + 3)
 
+/* record_top_of_heap is the current top of heap of inferior.
+   When record deal with sys_brk, it will be used.
+   Get it need call call_function_by_hand, this function will reset a lot
+   of status of inferior.  It will make process record get error
+   if call it in record_linux_system_call.  So get record_top_of_heap in
+   record_linux_reset.  */
+
+static bfd_vma record_top_of_heap;
+
+void
+record_linux_reset (struct gdbarch *gdbarch)
+{
+  struct value *sbrk_fn;
+  struct objfile *sbrk_objf;
+
+  record_top_of_heap = 0;
+
+  /* Get sbrk_fn and zero.  Because all of them
+     will be free by free_all_values in each command begin.  */
+  sbrk_fn = NULL;
+  if (lookup_minimal_symbol ("sbrk", NULL, NULL) != NULL)
+    sbrk_fn = find_function_in_inferior ("sbrk", &sbrk_objf);
+  if (!sbrk_fn)
+    {
+      if (lookup_minimal_symbol ("_sbrk", NULL, NULL) != NULL)
+	sbrk_fn = find_function_in_inferior ("_sbrk", &sbrk_objf);
+    }
+
+  if (sbrk_fn)
+    {
+      struct value *ret;
+      struct value *zero = value_from_longest
+	(builtin_type (get_objfile_arch (sbrk_objf))->builtin_int, 0);
+
+      remove_breakpoints ();
+      ret = call_function_by_hand (sbrk_fn, 1, &zero);
+      insert_breakpoints ();
+
+      if (ret)
+	{
+	  record_top_of_heap = value_as_long (ret);
+	}
+    }
+
+  if (!record_top_of_heap)
+    fprintf_unfiltered (gdb_stdlog,
+			_("Process record get current "
+			  "top of heap failied.\n"));
+}
+
 /* When the architecture process record get a Linux syscall
    instruction, it will get a Linux syscall number of this
    architecture and convert it to the Linux syscall number "num" which
@@ -246,8 +302,53 @@ record_linux_system_call (int num, struc
 
       /* sys_ni_syscall */
     case 44:
+      break;
+
       /* sys_brk */
     case 45:
+      {
+	int q;
+	bfd_vma end_data_segment;
+
+	regcache_raw_read (regcache, tdep->arg1,
+			   (gdb_byte *) & end_data_segment);
+
+	if (record_top_of_heap)
+	  {
+	    if (end_data_segment && record_top_of_heap > end_data_segment)
+	      {
+		target_terminal_ours ();
+		q =
+		  yquery (_("The next instruction is syscall brk.  "
+			    "It will release the memory that will cause "
+			    "process record target get error.  Do "
+			    "you want to stop the inferior?"));
+		target_terminal_inferior ();
+		if (q)
+		  return 1;
+	      }
+	  }
+	else
+	  {
+
+	    target_terminal_ours ();
+	    q =
+	      yquery (_("The next instruction is syscall brk.  "
+			"Process record cannot make sure it will "
+			"release memory or not.  "
+			"It may cause process record target get error.  "
+			"Do you want to stop the inferior?"));
+	    target_terminal_inferior ();
+	    if (q)
+	      return 1;
+	  }
+
+	/* If syscall brk execute, end_data_segment will be
+	   the top of heap.  */
+	record_top_of_heap = end_data_segment;
+      }
+      break;
+
       /* sys_setgid16 */
     case 46:
       /* sys_getgid16 */
--- a/linux-record.h
+++ b/linux-record.h
@@ -167,6 +167,7 @@ struct linux_record_tdep
   int arg5;
 };
 
+extern void record_linux_reset (struct gdbarch *gdbarch);
 extern int record_linux_system_call (int num, struct regcache *regcache,
 				     struct linux_record_tdep *tdep);
 
--- a/record.c
+++ b/record.c
@@ -440,6 +440,10 @@ record_open (char *name, int from_tty)
 	return;
     }
 
+  /*Reset the gdbarch part of process record.  */
+  if (gdbarch_process_record_reset_p (current_gdbarch))
+    gdbarch_process_record_reset (current_gdbarch);
+
   /*Reset the beneath function pointers.  */
   record_beneath_to_resume = NULL;
   record_beneath_to_wait = NULL;

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Eli Zaretskii]

> Date: Tue, 5 May 2009 21:07:13 +0800
> From: Hui Zhu <teawater@gmail.com>
> Cc: Michael Snyder <msnyder@vmware.com>
> 
> 15		sbrk (-10);
> (gdb)
> The next instruction is syscall brk.  It will release the memory that
> will cause process record target get error.  Do you want to stop the
> inferior?([y] or n)

Does every sbrk call with a negative argument cause an error in
process record target?  What is the reason for that error?

I'm asking because the message wording sounds very threatening.  It
probably needs rephrasing, but I need to understand the reasons better
to suggest how.

> 	* gdbarch.sh (process_record_reset): This interface point to
> 	the function that reset the architecture process record and
> 	replay.

I think "reset" is not the best name for this.  How about
"initialize"?

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Hui Zhu]

Hi Eli,

On Wed, May 6, 2009 at 02:41, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Tue, 5 May 2009 21:07:13 +0800
>> From: Hui Zhu <teawater@gmail.com>
>> Cc: Michael Snyder <msnyder@vmware.com>
>>
>> 15            sbrk (-10);
>> (gdb)
>> The next instruction is syscall brk.  It will release the memory that
>> will cause process record target get error.  Do you want to stop the
>> inferior?([y] or n)
>
> Does every sbrk call with a negative argument cause an error in
> process record target?  What is the reason for that error?
>
> I'm asking because the message wording sounds very threatening.  It
> probably needs rephrasing, but I need to understand the reasons better
> to suggest how.
>

Yes, I think we need it.
If inferior release some memory, the replay will got big error because
prec will set memory old value to this memory.
But we are lucky, glibc memory manage (malloc and free) try to doesn't
call sbrk to release the memory most of time.  I try to let use malloc
and free to let glibc to call sbrk to release the memory, but I
failed.

>>       * gdbarch.sh (process_record_reset): This interface point to
>>       the function that reset the architecture process record and
>>       replay.
>
> I think "reset" is not the best name for this.  How about
> "initialize"?

This interface will be call each time when prec open, so it will reset
the old value.
I think initialize looks like just call once.  For example
"_initialize_infcall".


Thanks,
Hui

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Eli Zaretskii]

> Date: Wed, 6 May 2009 10:13:15 +0800
> From: Hui Zhu <teawater@gmail.com>
> Cc: gdb-patches@sourceware.org, msnyder@vmware.com
> 
> If inferior release some memory, the replay will got big error because
> prec will set memory old value to this memory.

Yes, I understand that, but why will this cause an error?

> >>       * gdbarch.sh (process_record_reset): This interface point to
> >>       the function that reset the architecture process record and
> >>       replay.
> >
> > I think "reset" is not the best name for this.  How about
> > "initialize"?
> 
> This interface will be call each time when prec open, so it will reset
> the old value.
> I think initialize looks like just call once.  For example
> "_initialize_infcall".

"reset" has the opposite problem: the first time you call it, it has
no old state to reset.

If you don't like "initialize", perhaps "reinitialize" or "reinit" is
okay?  It is still better than "reset", IMO, because "reset" is very
ambiguous in the context of tracking machine instructions.  It took me
several minutes to understand what is that all about and why are you
introducing such an interface together with the sbrk handling.

Or maybe "prepare" is better?

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Hui Zhu]

On Wed, May 6, 2009 at 11:14, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Wed, 6 May 2009 10:13:15 +0800
>> From: Hui Zhu <teawater@gmail.com>
>> Cc: gdb-patches@sourceware.org, msnyder@vmware.com
>>
>> If inferior release some memory, the replay will got big error because
>> prec will set memory old value to this memory.
>
> Yes, I understand that, but why will this cause an error?

If this memory already release and gdb still write value to this
address,  the os mm will make this operation fail.
If you think the query words is too threatening.  Could you give me
some help?  But I still want user choice stop.  :)

>
>> >>       * gdbarch.sh (process_record_reset): This interface point to
>> >>       the function that reset the architecture process record and
>> >>       replay.
>> >
>> > I think "reset" is not the best name for this.  How about
>> > "initialize"?
>>
>> This interface will be call each time when prec open, so it will reset
>> the old value.
>> I think initialize looks like just call once.  For example
>> "_initialize_infcall".
>
> "reset" has the opposite problem: the first time you call it, it has
> no old state to reset.
>
> If you don't like "initialize", perhaps "reinitialize" or "reinit" is
> okay?  It is still better than "reset", IMO, because "reset" is very
> ambiguous in the context of tracking machine instructions.  It took me
> several minutes to understand what is that all about and why are you
> introducing such an interface together with the sbrk handling.
>
> Or maybe "prepare" is better?
>

I think both reinitialize and prepare is OK for me.  Do you have some
idea with it?

Thanks,
Hui

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Eli Zaretskii]

> Date: Wed, 6 May 2009 11:35:38 +0800
> From: Hui Zhu <teawater@gmail.com>
> Cc: gdb-patches@sourceware.org, msnyder@vmware.com
> 
> On Wed, May 6, 2009 at 11:14, Eli Zaretskii <eliz@gnu.org> wrote:
> >> Date: Wed, 6 May 2009 10:13:15 +0800
> >> From: Hui Zhu <teawater@gmail.com>
> >> Cc: gdb-patches@sourceware.org, msnyder@vmware.com
> >>
> >> If inferior release some memory, the replay will got big error because
> >> prec will set memory old value to this memory.
> >
> > Yes, I understand that, but why will this cause an error?
> 
> If this memory already release and gdb still write value to this
> address,  the os mm will make this operation fail.

Why would GDB write to the memory that no longer belongs to the
inferior?  Are you talking about GDB in general or process
record/replay in particular?  If the former, I'd say that's a bug.  If
the latter, when and under what conditions will record/replay need to
do that?

I thought the problem was that replaying the execution log before the
sbrk point would be impossible, because (I thought) there's no way of
regaining back the memory the inferior gave up.  Is this the problem
you are talking about?  If so, that is not a fatal limitation, and it
certainly does not justify stopping the program and asking the user to
make some grave decision.  The user just needs to be notified, when
she tries that, that she cannot reverse-replay the log past this
point.  If the user never tries to replay past that point, she never
needs to know about the problem.

> I think both reinitialize and prepare is OK for me.  Do you have some
> idea with it?

I can live with both.  What do others think?

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Hui Zhu]

On Thu, May 7, 2009 at 02:28, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Wed, 6 May 2009 11:35:38 +0800
>> From: Hui Zhu <teawater@gmail.com>
>> Cc: gdb-patches@sourceware.org, msnyder@vmware.com
>>
>> On Wed, May 6, 2009 at 11:14, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> Date: Wed, 6 May 2009 10:13:15 +0800
>> >> From: Hui Zhu <teawater@gmail.com>
>> >> Cc: gdb-patches@sourceware.org, msnyder@vmware.com
>> >>
>> >> If inferior release some memory, the replay will got big error because
>> >> prec will set memory old value to this memory.
>> >
>> > Yes, I understand that, but why will this cause an error?
>>
>> If this memory already release and gdb still write value to this
>> address,  the os mm will make this operation fail.
>
> Why would GDB write to the memory that no longer belongs to the
> inferior?  Are you talking about GDB in general or process
> record/replay in particular?  If the former, I'd say that's a bug.  If
> the latter, when and under what conditions will record/replay need to
> do that?
>
> I thought the problem was that replaying the execution log before the
> sbrk point would be impossible, because (I thought) there's no way of
> regaining back the memory the inferior gave up.  Is this the problem
> you are talking about?  If so, that is not a fatal limitation, and it
> certainly does not justify stopping the program and asking the user to
> make some grave decision.  The user just needs to be notified, when
> she tries that, that she cannot reverse-replay the log past this
> point.  If the user never tries to replay past that point, she never
> needs to know about the problem.

I am not sure make inferior cannot continue is good or not.  I think
let user choice continue or stop is better.



>
>> I think both reinitialize and prepare is OK for me.  Do you have some
>> idea with it?
>
> I can live with both.  What do others think?
>

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Eli Zaretskii]

> Date: Thu, 7 May 2009 10:21:04 +0800
> From: Hui Zhu <teawater@gmail.com>
> Cc: gdb-patches@sourceware.org, msnyder@vmware.com
> 
> > I thought the problem was that replaying the execution log before the
> > sbrk point would be impossible, because (I thought) there's no way of
> > regaining back the memory the inferior gave up.  Is this the problem
> > you are talking about?  If so, that is not a fatal limitation, and it
> > certainly does not justify stopping the program and asking the user to
> > make some grave decision.  The user just needs to be notified, when
> > she tries that, that she cannot reverse-replay the log past this
> > point.  If the user never tries to replay past that point, she never
> > needs to know about the problem.
> 
> I am not sure make inferior cannot continue is good or not.  I think
> let user choice continue or stop is better.

Well, what do others think?

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Hui Zhu]

PING.

On Thu, May 7, 2009 at 11:20, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Thu, 7 May 2009 10:21:04 +0800
>> From: Hui Zhu <teawater@gmail.com>
>> Cc: gdb-patches@sourceware.org, msnyder@vmware.com
>>
>> > I thought the problem was that replaying the execution log before the
>> > sbrk point would be impossible, because (I thought) there's no way of
>> > regaining back the memory the inferior gave up.  Is this the problem
>> > you are talking about?  If so, that is not a fatal limitation, and it
>> > certainly does not justify stopping the program and asking the user to
>> > make some grave decision.  The user just needs to be notified, when
>> > she tries that, that she cannot reverse-replay the log past this
>> > point.  If the user never tries to replay past that point, she never
>> > needs to know about the problem.
>>
>> I am not sure make inferior cannot continue is good or not.  I think
>> let user choice continue or stop is better.
>
> Well, what do others think?
>

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Hui Zhu]

Ping.

On Mon, May 11, 2009 at 15:06, Hui Zhu<teawater@gmail.com> wrote:
> PING.
>
> On Thu, May 7, 2009 at 11:20, Eli Zaretskii <eliz@gnu.org> wrote:
>>> Date: Thu, 7 May 2009 10:21:04 +0800
>>> From: Hui Zhu <teawater@gmail.com>
>>> Cc: gdb-patches@sourceware.org, msnyder@vmware.com
>>>
>>> > I thought the problem was that replaying the execution log before the
>>> > sbrk point would be impossible, because (I thought) there's no way of
>>> > regaining back the memory the inferior gave up.  Is this the problem
>>> > you are talking about?  If so, that is not a fatal limitation, and it
>>> > certainly does not justify stopping the program and asking the user to
>>> > make some grave decision.  The user just needs to be notified, when
>>> > she tries that, that she cannot reverse-replay the log past this
>>> > point.  If the user never tries to replay past that point, she never
>>> > needs to know about the problem.
>>>
>>> I am not sure make inferior cannot continue is good or not.  I think
>>> let user choice continue or stop is better.
>>
>> Well, what do others think?
>>
>

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Michael Snyder]

Hui Zhu wrote:
> Ping.

OK, my bad for taking so long to get to this... please allow me
to summarize the problem, to check my own understanding
(tell me if I'm wrong).

Currently linux-record.c does not know how to "undo" a sys_brk
system call.  You (teawater) are concerned because if the child
process calls sys_brk to free some memory, we cannot un-free it
and therefore we may get into trouble by writing to the freed
memory during replay.  Something like this:

   1) child allocates memory X
   2) child writes to memory X
   3) child frees memory X
   4) user asks for reverse-continue
   5) gdb tries to revert the write that happened in step #2,
      gets SIGSEGV because location has been freed.

So far so good?

Now, your proposal is that during the record mode, we will
detect any sys_brk call that frees memory, and query the
user whether to continue or give up.

I'm not too crazy about that solution.  I think it's
awkward, and drastic for a situation that may only be
a problem later on (or not at all).  Let me throw out
some other ideas:

A) Is it possible to actually "reverse" a sys_brk call?
Suppose we record the arguments, and when we want to reverse
it, we just change an increase into a decrease and vice versa?

B) Suppose we wait until an actual memory error occurs
during replay, and THEN inform the user?  It will avoid
warning him about something that may never happen.

We could use catch_errors to trap the SIGSEGV, and then
check to see if the error was caused by a write to memory
above the BRK boundary.  You will still need to keep track
of the BRK boundary, but you won't have that awkward early
query to deal with.

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Hui Zhu]

On Sun, Jun 14, 2009 at 06:56, Michael Snyder<msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>>
>> Ping.
>
> OK, my bad for taking so long to get to this... please allow me
> to summarize the problem, to check my own understanding
> (tell me if I'm wrong).
>

For that "nice people" words.  I just want to make a joke.  :)

> Currently linux-record.c does not know how to "undo" a sys_brk
> system call.  You (teawater) are concerned because if the child
> process calls sys_brk to free some memory, we cannot un-free it
> and therefore we may get into trouble by writing to the freed
> memory during replay.  Something like this:
>
>  1) child allocates memory X
>  2) child writes to memory X
>  3) child frees memory X
>  4) user asks for reverse-continue
>  5) gdb tries to revert the write that happened in step #2,
>     gets SIGSEGV because location has been freed.
>
> So far so good?
>
> Now, your proposal is that during the record mode, we will
> detect any sys_brk call that frees memory, and query the
> user whether to continue or give up.
>
> I'm not too crazy about that solution.  I think it's
> awkward, and drastic for a situation that may only be
> a problem later on (or not at all).  Let me throw out
> some other ideas:
>
> A) Is it possible to actually "reverse" a sys_brk call?
> Suppose we record the arguments, and when we want to reverse
> it, we just change an increase into a decrease and vice versa?
>
> B) Suppose we wait until an actual memory error occurs
> during replay, and THEN inform the user?  It will avoid
> warning him about something that may never happen.
>
> We could use catch_errors to trap the SIGSEGV, and then
> check to see if the error was caused by a write to memory
> above the BRK boundary.  You will still need to keep track
> of the BRK boundary, but you won't have that awkward early
> query to deal with.

The sys_brk just can increase and decrease data segment size.  The
decrease behavior is very hard to replay.
I read some code of malloc and free in glibc.  I found that most of
time, free will not call brk to release memory to system.  Because it
is low efficiency.
So I think when brk release really happen, give user a query is a easy
way to handle it.

What do you think about it?

thanks,
Hui

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Michael Snyder]

Hui Zhu wrote:
> On Sun, Jun 14, 2009 at 06:56, Michael Snyder<msnyder@vmware.com> wrote:
>> Hui Zhu wrote:
>>> Ping.
>> OK, my bad for taking so long to get to this... please allow me
>> to summarize the problem, to check my own understanding
>> (tell me if I'm wrong).
>>
> 
> For that "nice people" words.  I just want to make a joke.  :)
> 
>> Currently linux-record.c does not know how to "undo" a sys_brk
>> system call.  You (teawater) are concerned because if the child
>> process calls sys_brk to free some memory, we cannot un-free it
>> and therefore we may get into trouble by writing to the freed
>> memory during replay.  Something like this:
>>
>>  1) child allocates memory X
>>  2) child writes to memory X
>>  3) child frees memory X
>>  4) user asks for reverse-continue
>>  5) gdb tries to revert the write that happened in step #2,
>>     gets SIGSEGV because location has been freed.
>>
>> So far so good?
>>
>> Now, your proposal is that during the record mode, we will
>> detect any sys_brk call that frees memory, and query the
>> user whether to continue or give up.
>>
>> I'm not too crazy about that solution.  I think it's
>> awkward, and drastic for a situation that may only be
>> a problem later on (or not at all).  Let me throw out
>> some other ideas:
>>
>> A) Is it possible to actually "reverse" a sys_brk call?
>> Suppose we record the arguments, and when we want to reverse
>> it, we just change an increase into a decrease and vice versa?
>>
>> B) Suppose we wait until an actual memory error occurs
>> during replay, and THEN inform the user?  It will avoid
>> warning him about something that may never happen.
>>
>> We could use catch_errors to trap the SIGSEGV, and then
>> check to see if the error was caused by a write to memory
>> above the BRK boundary.  You will still need to keep track
>> of the BRK boundary, but you won't have that awkward early
>> query to deal with.
> 
> The sys_brk just can increase and decrease data segment size.  The
> decrease behavior is very hard to replay.

I admit my ignorance in this area, but why is it difficult?
In my simple-minded view, if we need to reverse over a sys_brk
decrease call, we just make an increase call in the same amount.

Please tell me what I am missing.

> I read some code of malloc and free in glibc.  I found that most of
> time, free will not call brk to release memory to system.  Because it
> is low efficiency.
> So I think when brk release really happen, give user a query is a easy
> way to handle it.
> 
> What do you think about it?

OK, assuming that we cannot actually reverse the call, I agree
that we may encounter a situation where we cannot go back any
further -- but I think we should wait until we actually encounter
that situation before we notify the user.

During record phase is too early for that notification.
The actual failure may never be encountered, especially if
the user never tries to reverse past this point in the recording.

What I think is that we should wait until we are replaying, and
we actually experience a failure to modify freed memory.  At that
point we tell the user what has happened, and explain that we
cannot go back any earlier in the recording.

So something like this:

    1) Remember the BRK boundary at start (as you do in this patch).
    2) Remember the new BRK boundary whenever it changes (as you do).
    3) During replay, compare every memory write against the BRK
boundary.  If the memory write will fail because it is above the
BRK boundary, stop and inform the user that we cannot go back
any further.

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Hui Zhu]

On Mon, Jun 15, 2009 at 01:43, Michael Snyder<msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>>
>> On Sun, Jun 14, 2009 at 06:56, Michael Snyder<msnyder@vmware.com> wrote:
>>>
>>> Hui Zhu wrote:
>>>>
>>>> Ping.
>>>
>>> OK, my bad for taking so long to get to this... please allow me
>>> to summarize the problem, to check my own understanding
>>> (tell me if I'm wrong).
>>>
>>
>> For that "nice people" words.  I just want to make a joke.  :)
>>
>>> Currently linux-record.c does not know how to "undo" a sys_brk
>>> system call.  You (teawater) are concerned because if the child
>>> process calls sys_brk to free some memory, we cannot un-free it
>>> and therefore we may get into trouble by writing to the freed
>>> memory during replay.  Something like this:
>>>
>>>  1) child allocates memory X
>>>  2) child writes to memory X
>>>  3) child frees memory X
>>>  4) user asks for reverse-continue
>>>  5) gdb tries to revert the write that happened in step #2,
>>>    gets SIGSEGV because location has been freed.
>>>
>>> So far so good?
>>>
>>> Now, your proposal is that during the record mode, we will
>>> detect any sys_brk call that frees memory, and query the
>>> user whether to continue or give up.
>>>
>>> I'm not too crazy about that solution.  I think it's
>>> awkward, and drastic for a situation that may only be
>>> a problem later on (or not at all).  Let me throw out
>>> some other ideas:
>>>
>>> A) Is it possible to actually "reverse" a sys_brk call?
>>> Suppose we record the arguments, and when we want to reverse
>>> it, we just change an increase into a decrease and vice versa?
>>>
>>> B) Suppose we wait until an actual memory error occurs
>>> during replay, and THEN inform the user?  It will avoid
>>> warning him about something that may never happen.
>>>
>>> We could use catch_errors to trap the SIGSEGV, and then
>>> check to see if the error was caused by a write to memory
>>> above the BRK boundary.  You will still need to keep track
>>> of the BRK boundary, but you won't have that awkward early
>>> query to deal with.
>>
>> The sys_brk just can increase and decrease data segment size.  The
>> decrease behavior is very hard to replay.
>
> I admit my ignorance in this area, but why is it difficult?
> In my simple-minded view, if we need to reverse over a sys_brk
> decrease call, we just make an increase call in the same amount.
>
> Please tell me what I am missing.
>

I think about it again.  Maybe let it increase and decrease can handle
this issue.  But call function when infrun is running is a very hard
job.   Because call_function_by_hand will clear a lot of thing of
inferior.
We can do it.  But it must need a long time to check in.
So maybe we can give user a warning first.

>> I read some code of malloc and free in glibc.  I found that most of
>> time, free will not call brk to release memory to system.  Because it
>> is low efficiency.
>> So I think when brk release really happen, give user a query is a easy
>> way to handle it.
>>
>> What do you think about it?
>
> OK, assuming that we cannot actually reverse the call, I agree
> that we may encounter a situation where we cannot go back any
> further -- but I think we should wait until we actually encounter
> that situation before we notify the user.
>
> During record phase is too early for that notification.
> The actual failure may never be encountered, especially if
> the user never tries to reverse past this point in the recording.
>
> What I think is that we should wait until we are replaying, and
> we actually experience a failure to modify freed memory.  At that
> point we tell the user what has happened, and explain that we
> cannot go back any earlier in the recording.
>
> So something like this:
>
>   1) Remember the BRK boundary at start (as you do in this patch).
>   2) Remember the new BRK boundary whenever it changes (as you do).
>   3) During replay, compare every memory write against the BRK
> boundary.  If the memory write will fail because it is above the
> BRK boundary, stop and inform the user that we cannot go back
> any further.

It will make user lost the record entry that before this memory operation.
And when this thing happen (sys_brk), user doesn't get any alert.


Thanks,
Hui

Re: [Precord RFA/RFC] Check Linux sys_brk release memory in process record and replay.

[Michael Snyder]

Hui Zhu wrote:
> On Mon, Jun 15, 2009 at 01:43, Michael Snyder<msnyder@vmware.com> wrote:
>> Hui Zhu wrote:
>>> On Sun, Jun 14, 2009 at 06:56, Michael Snyder<msnyder@vmware.com> wrote:
>>>> Hui Zhu wrote:
>>>>> Ping.
>>>> OK, my bad for taking so long to get to this... please allow me
>>>> to summarize the problem, to check my own understanding
>>>> (tell me if I'm wrong).
>>>>
>>> For that "nice people" words.  I just want to make a joke.  :)
>>>
>>>> Currently linux-record.c does not know how to "undo" a sys_brk
>>>> system call.  You (teawater) are concerned because if the child
>>>> process calls sys_brk to free some memory, we cannot un-free it
>>>> and therefore we may get into trouble by writing to the freed
>>>> memory during replay.  Something like this:
>>>>
>>>>  1) child allocates memory X
>>>>  2) child writes to memory X
>>>>  3) child frees memory X
>>>>  4) user asks for reverse-continue
>>>>  5) gdb tries to revert the write that happened in step #2,
>>>>    gets SIGSEGV because location has been freed.
>>>>
>>>> So far so good?
>>>>
>>>> Now, your proposal is that during the record mode, we will
>>>> detect any sys_brk call that frees memory, and query the
>>>> user whether to continue or give up.
>>>>
>>>> I'm not too crazy about that solution.  I think it's
>>>> awkward, and drastic for a situation that may only be
>>>> a problem later on (or not at all).  Let me throw out
>>>> some other ideas:
>>>>
>>>> A) Is it possible to actually "reverse" a sys_brk call?
>>>> Suppose we record the arguments, and when we want to reverse
>>>> it, we just change an increase into a decrease and vice versa?
>>>>
>>>> B) Suppose we wait until an actual memory error occurs
>>>> during replay, and THEN inform the user?  It will avoid
>>>> warning him about something that may never happen.
>>>>
>>>> We could use catch_errors to trap the SIGSEGV, and then
>>>> check to see if the error was caused by a write to memory
>>>> above the BRK boundary.  You will still need to keep track
>>>> of the BRK boundary, but you won't have that awkward early
>>>> query to deal with.
>>> The sys_brk just can increase and decrease data segment size.  The
>>> decrease behavior is very hard to replay.
>> I admit my ignorance in this area, but why is it difficult?
>> In my simple-minded view, if we need to reverse over a sys_brk
>> decrease call, we just make an increase call in the same amount.
>>
>> Please tell me what I am missing.
>>
> 
> I think about it again.  Maybe let it increase and decrease can handle
> this issue.  But call function when infrun is running is a very hard
> job.   Because call_function_by_hand will clear a lot of thing of
> inferior.
> We can do it.  But it must need a long time to check in.
> So maybe we can give user a warning first.

OK, I can see how it is difficult.  Maybe save for later.
For now, maybe we think about which short-term solution is
better:

     1) warn when sys_brk(<0), or
     2) warn when cannot undo a memory write.

Further comment below.

>> During record phase is too early for that notification.
>> The actual failure may never be encountered, especially if
>> the user never tries to reverse past this point in the recording.
>>
>> What I think is that we should wait until we are replaying, and
>> we actually experience a failure to modify freed memory.  At that
>> point we tell the user what has happened, and explain that we
>> cannot go back any earlier in the recording.
>>
>> So something like this:
>>
>>   1) Remember the BRK boundary at start (as you do in this patch).
>>   2) Remember the new BRK boundary whenever it changes (as you do).
>>   3) During replay, compare every memory write against the BRK
>> boundary.  If the memory write will fail because it is above the
>> BRK boundary, stop and inform the user that we cannot go back
>> any further.
> 
> It will make user lost the record entry that before this memory operation.
> And when this thing happen (sys_brk), user doesn't get any alert.

I think that warning when sys_brk(<0) is too early.
Nothing is really wrong at that time -- it only means
that something MIGHT be wrong later, during reverse.

I think the right time to warn is when we actually
encounter a memory write that we cannot undo.  At that
point, there is no way to reverse any further, but
there is really nothing we can do about it.

If we warn the user earlier (during record), there is still
nothing that he can do about the problem.