Re: [PREC/RFA] Add not_replay to make precord support release memory better

[Michael Snyder <msnyder at vmware dot com>]

Hui Zhu wrote:
> On Tue, Aug 4, 2009 at 01:30, Michael Snyder<msnyder@vmware.com> wrote:
> > Hui Zhu wrote:
> > > The record_exec_entry don't have big contact with core dump patch.
> > >
> > > I think the new version is more clear.
> > But we had this patch all worked out, and now you've added
> > a bunch of new stuff to it.  It's not a "new version" of the
> > "add not_replay" patch, it's a whole new change.
> >
> > That's not the way we do it.  If you want to add new stuff,
> > you start a new thread, you don't add it into a thread that's
> > already in review.
>
> OK.  I got it.
>
> So the patch http://sourceware.org/ml/gdb-patches/2009-08/msg00006.html
> looks OK.
> Could you check it in?
>
> I will update dump patch after this patch in.
>
> Thanks,
> Hui

Thanks.  Added some comments, and it's in as attached here.


2009-08-04  Hui Zhu  <teawater@gmail.com>
	    Michael Snyder  <msnyder@vmware.com>

	* record.c (record_mem_entry): New field 'mem_entry_not_accessible'.
	(record_arch_list_add_mem): Initialize 'mem_entry_not_accessible'.
	(record_wait): Set 'mem_entry_not_accessible' flag if target
	memory not readable.  Don't try to change target memory if
	'mem_entry_not_accessible' is set.

Index: record.c
===================================================================
RCS file: /cvs/src/src/gdb/record.c,v
retrieving revision 1.9
diff -u -p -r1.9 record.c
--- record.c	22 Jul 2009 05:31:26 -0000	1.9
+++ record.c	4 Aug 2009 18:17:15 -0000
@@ -51,6 +51,9 @@ struct record_mem_entry
 {
   CORE_ADDR addr;
   int len;
+  /* Set this flag if target memory for this entry
+     can no longer be accessed.  */
+  int mem_entry_not_accessible;
   gdb_byte *val;
 };
 
@@ -275,6 +278,7 @@ record_arch_list_add_mem (CORE_ADDR addr
   rec->type = record_mem;
   rec->u.mem.addr = addr;
   rec->u.mem.len = len;
+  rec->u.mem.mem_entry_not_accessible = 0;
 
   if (target_read_memory (addr, rec->u.mem.val, len))
     {
@@ -727,32 +731,55 @@ record_wait (struct target_ops *ops,
 	  else if (record_list->type == record_mem)
 	    {
 	      /* mem */
-	      gdb_byte *mem = alloca (record_list->u.mem.len);
-	      if (record_debug > 1)
-		fprintf_unfiltered (gdb_stdlog,
-				    "Process record: record_mem %s to "
-				    "inferior addr = %s len = %d.\n",
-				    host_address_to_string (record_list),
-				    paddress (gdbarch, record_list->u.mem.addr),
-				    record_list->u.mem.len);
-
-	      if (target_read_memory
-		  (record_list->u.mem.addr, mem, record_list->u.mem.len))
-		error (_("Process record: error reading memory at "
-			 "addr = %s len = %d."),
-		       paddress (gdbarch, record_list->u.mem.addr),
-		       record_list->u.mem.len);
-
-	      if (target_write_memory
-		  (record_list->u.mem.addr, record_list->u.mem.val,
-		   record_list->u.mem.len))
-		error (_
-		       ("Process record: error writing memory at "
-			"addr = %s len = %d."),
-		       paddress (gdbarch, record_list->u.mem.addr),
-		       record_list->u.mem.len);
-
-	      memcpy (record_list->u.mem.val, mem, record_list->u.mem.len);
+	      /* Nothing to do if the entry is flagged not_accessible.  */
+	      if (!record_list->u.mem.mem_entry_not_accessible)
+		{
+		  gdb_byte *mem = alloca (record_list->u.mem.len);
+		  if (record_debug > 1)
+		    fprintf_unfiltered (gdb_stdlog,
+				        "Process record: record_mem %s to "
+				        "inferior addr = %s len = %d.\n",
+				        host_address_to_string (record_list),
+				        paddress (gdbarch,
+					          record_list->u.mem.addr),
+				        record_list->u.mem.len);
+
+		  if (target_read_memory (record_list->u.mem.addr, mem,
+		                          record_list->u.mem.len))
+	            {
+		      if (execution_direction != EXEC_REVERSE)
+		        error (_("Process record: error reading memory at "
+			         "addr = %s len = %d."),
+		               paddress (gdbarch, record_list->u.mem.addr),
+		               record_list->u.mem.len);
+		      else
+			/* Read failed -- 
+			   flag entry as not_accessible.  */
+		        record_list->u.mem.mem_entry_not_accessible = 1;
+		    }
+		  else
+		    {
+		      if (target_write_memory (record_list->u.mem.addr,
+			                       record_list->u.mem.val,
+		                               record_list->u.mem.len))
+	                {
+			  if (execution_direction != EXEC_REVERSE)
+			    error (_("Process record: error writing memory at "
+			             "addr = %s len = %d."),
+		                   paddress (gdbarch, record_list->u.mem.addr),
+		                   record_list->u.mem.len);
+			  else
+			    /* Write failed -- 
+			       flag entry as not_accessible.  */
+			    record_list->u.mem.mem_entry_not_accessible = 1;
+			}
+		      else
+		        {
+			  memcpy (record_list->u.mem.val, mem,
+				  record_list->u.mem.len);
+			}
+		    }
+		}
 	    }
 	  else
 	    {