skip setting the svr4 shared library breakpoint if there's no execution (Re: Regression loading a tracefile in 7_3)

skip setting the svr4 shared library breakpoint if there's no execution (Re: Regression loading a tracefile in 7_3)

[Pedro Alves]

Moving to gdb-patches, from
<http://sourceware.org/ml/gdb/2011-05/msg00107.html>

On Friday 20 May 2011 12:57:53, Pedro Alves wrote:
> On Friday 20 May 2011 03:03:20, Marc Khouzam wrote:
> > Hi,
> > 
> > I believe I'm seeing a regression loading a tracefile in GDB 7.3
> > I create a simple tracefile using GDB/gdbserver 7.3 but when I try to load
> > that tracefile, I get an error about "PC register is not available", which
> > causes my Eclipse session to abort.  This is not happening in 7.2 or 7.2.1.
> > 
> > > gdb.7.3 a.out
> > GNU gdb (GDB) 7.2.90.20110519-cvs
> > (gdb) interpreter-exec mi "-target-select tfile trace.7.3"
> > ~"Created tracepoint 1 for target's tracepoint 1 at 0x804851f.\n"
> > ^error,msg="PC register is not available"
> > 
> 
> Hmm.
> 
> #0  throw_error (error=NOT_AVAILABLE_ERROR, fmt=0x7f7908 "PC register is not available") at ../../src/gdb/exceptions.c:423
> #1  0x000000000053de3a in regcache_read_pc (regcache=0xd4b280) at ../../src/gdb/regcache.c:994
> #2  0x0000000000497053 in enable_break (info=0xd0df60, from_tty=1) at ../../src/gdb/solib-svr4.c:1536
> #3  0x00000000004980a8 in svr4_solib_create_inferior_hook (from_tty=1) at ../../src/gdb/solib-svr4.c:2194
> #4  0x000000000046faa7 in solib_create_inferior_hook (from_tty=1) at ../../src/gdb/solib.c:1240
> #5  0x0000000000586de2 in post_create_inferior (target=0xbc7120, from_tty=1) at ../../src/gdb/infcmd.c:427
> #6  0x00000000004d2ca8 in tfile_open (filename=0xd0d790 "/home/pedro/gdb/marck_tfile/build/gdb/./testsuite/basic.tf", from_tty=1)
>     at ../../src/gdb/tracepoint.c:3410
> 
> Here, in solib-svr4.c:
> 
>       /* Otherwise we find the dynamic linker's base address by examining
> 	 the current pc (which should point at the entry point for the
> 	 dynamic linker) and subtracting the offset of the entry point.
> 
>          This is more fragile than the previous approaches, but is a good
>          fallback method because it has actually been working well in
>          most cases.  */
>       if (!load_addr_found)
> 	{
> 	  struct regcache *regcache
> 	    = get_thread_arch_regcache (inferior_ptid, target_gdbarch);
> 
> 	  load_addr = (regcache_read_pc (regcache)
> 		       - exec_entry_point (tmp_bfd, tmp_bfd_target));
> 	}
> 
> We could skip this if the PC is not available, but:
> 
>  - why do we even try to set a shared library breakpoint
>    if the target has no execution (e.g., when debugging a core
>    file) in the first place?  I'll try skipping enable_break in that
>    case and see if anything breaks.  By inspection, it looks like
>    nothing will.  svr4_in_dynsym_resolve_code will always
>    return false if we skip enable_break, but then again that function
>    is only used for run control.

Nothing did break.  Here's the patch.  Anyone see a problem with
this?  I'd like to apply this to mainline and 7.3.

-- 
Pedro Alves

2011-05-20  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* solib-svr4.c (svr4_solib_create_inferior_hook): Skip setting
	shared library event breakpoint if there's no execution.

	gdb/testsuite/
	* gdb.trace/tfile.exp: Add test that opening the basic.tf trace
	file doesn't error, using MI.

---
 gdb/solib-svr4.c                  |    5 +++++
 gdb/testsuite/gdb.trace/tfile.exp |    5 +++++
 2 files changed, 10 insertions(+)

Index: src/gdb/solib-svr4.c
===================================================================
--- src.orig/gdb/solib-svr4.c	2011-05-20 14:36:49.428819003 +0100
+++ src/gdb/solib-svr4.c	2011-05-20 14:54:58.548819003 +0100
@@ -2188,6 +2188,11 @@ svr4_solib_create_inferior_hook (int fro
   /* Relocate the main executable if necessary.  */
   svr4_relocate_main_executable ();
 
+  /* No point setting a breakpoint in the dynamic linker if we can't
+     hit it (e.g., a core file, or a trace file).  */
+  if (!target_has_execution)
+    return;
+
   if (!svr4_have_link_map_offsets ())
     return;
 
Index: src/gdb/testsuite/gdb.trace/tfile.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.trace/tfile.exp	2011-05-20 14:36:49.428819003 +0100
+++ src/gdb/testsuite/gdb.trace/tfile.exp	2011-05-20 14:54:58.548819003 +0100
@@ -116,3 +116,8 @@ Collected 0 trace frame.*
 Trace buffer has 256 bytes of 4096 bytes free \\(93% full\\).*
 Not looking at any trace frame.*" \
     "tstatus on error trace file"
+
+# Make sure we can reopen without error.
+gdb_test \
+    "interpreter-exec mi \"-target-select tfile basic.tf\"" \
+    "\\^connected.*"

Re: skip setting the svr4 shared library breakpoint if there's no execution (Re: Regression loading a tracefile in 7_3)

[Joel Brobecker]

> 2011-05-20  Pedro Alves  <pedro@codesourcery.com>
> 
> 	gdb/
> 	* solib-svr4.c (svr4_solib_create_inferior_hook): Skip setting
> 	shared library event breakpoint if there's no execution.
> 
> 	gdb/testsuite/
> 	* gdb.trace/tfile.exp: Add test that opening the basic.tf trace
> 	file doesn't error, using MI.

Seems very reasonable...

-- 
Joel

Re: skip setting the svr4 shared library breakpoint if there's no execution (Re: Regression loading a tracefile in 7_3)

[Pedro Alves]

On Friday 20 May 2011 16:35:34, Joel Brobecker wrote:
> > 2011-05-20  Pedro Alves  <pedro@codesourcery.com>
> > 
> > 	gdb/
> > 	* solib-svr4.c (svr4_solib_create_inferior_hook): Skip setting
> > 	shared library event breakpoint if there's no execution.
> > 
> > 	gdb/testsuite/
> > 	* gdb.trace/tfile.exp: Add test that opening the basic.tf trace
> > 	file doesn't error, using MI.
> 
> Seems very reasonable...

Thanks.  Applied mainline and branch.

-- 
Pedro Alves