[5.1/breakpoint] shlib patch?

[5.1/breakpoint] shlib patch?

[Andrew Cagney]

Hello,

Did anything happen to the below in the end?  The test can get turned 
into a PR.

	Andrew

Wow, three bug reports for the same problem in one day!  We should
probably make fixing this a real priority :-).

Anyway, thanks for reporting.

The following patch will fix the problems with setting breakpoints in
dynamically loaded objects:

    http://sourceware.cygnus.com/ml/gdb-patches/2000-05/msg00230.html

This patch isn't checked in yet (ping Michael/JimB), but I hope this
will be in the next GDB release.

There should really be a test in the testsuite for this problem, since
it keeps coming up :-(.  Any volunteers?

Mark

Re: [5.1/breakpoint] shlib patch?

[Mark Kettenis]

Mark Kettenis <kettenis@science.uva.nl> writes:

> No nothing happened.  The patch listed is not quite right :-(.  The
> basic problem is that GDB assumes that shared libraries will be loaded
> at their own, fixed, address.  That's not true with modern shared
> library systems, and becomes problematic whith dynamically loaded
> modules.  I have a simpler patch that's also not quite right, but if
> we cannot fix the problem properly, we might want to add it (as a
> stopgap) to 5.1.

To follow up on my own message.  I tried to write a test that checks
for the broken behaviour, and I managed to come up with something
where event my workaround fails.  I'll start thinking about a proper
solution, but we probably must accept that we cannot fix this properly
for 5.1.

Mark

Re: [5.1/breakpoint] shlib patch?

[Andrew Cagney]

Just FYI,

This is probably the 5.1 release hack candidate (in branch, not in 
trunk).  Every release has one ...  Unless someone comes up with 
something that is.

enjoy,
Andrew

> Did anything happen to the below in the end?  The test can get turned into a PR.
> 
>     Andrew
> 
> Wow, three bug reports for the same problem in one day!  We should
> probably make fixing this a real priority [:-)] .
> 
> Anyway, thanks for reporting.
> 
> The following patch will fix the problems with setting breakpoints in
> dynamically loaded objects:
> 
>    http://sourceware.cygnus.com/ml/gdb-patches/2000-05/msg00230.html
> 
> This patch isn't checked in yet (ping Michael/JimB), but I hope this
> will be in the next GDB release.
> 
> There should really be a test in the testsuite for this problem, since
> it keeps coming up [:-(] .  Any volunteers?

Re: [5.1/breakpoint] shlib patch?

[Mark Kettenis]

   Date: Thu, 27 Sep 2001 00:38:35 -0400
   From: Andrew Cagney <ac131313@cygnus.com>

   Just FYI,

   This is probably the 5.1 release hack candidate (in branch, not in 
   trunk).  Every release has one ...  Unless someone comes up with 
   something that is.

And a hack it is, although Apple's Darwin version of GDB contains an
almost identical hack to implement a "future break" command.  It seems
to work in many real world cases, but it is easy to come up with a
test case where the patch doesn't help at all.

It seems that the way GDB implements breakpoints-in-shared-libraries
was designed for a.out shared library systems (SunOS 4) where shared
libraries were loaded at a fixed address in memory.  Since ELF shared
libraries can (and will) be loaded at any address in memory, things
break.  Fixing this is not trivial.  Therefore, I'm not sure whether
we should add this hack to the branch only.  I cannot guarantee that
things will be fixed on the trunk in the near future.

Anyway, for reference I attached the hack.

Mark


Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.53
diff -u -p -r1.53 breakpoint.c
--- breakpoint.c 2001/09/18 05:00:48 1.53
+++ breakpoint.c 2001/09/28 09:27:52
@@ -7009,10 +7009,14 @@ breakpoint_re_set_one (PTR bint)
 	  delete_breakpoint (b);
 	  return 0;
 	}
-      /* In case we have a problem, disable this breakpoint.  We'll restore
-         its status if we succeed.  */
+      /* In case we have a problem, disable this breakpoint.  We'll
+         restore its status if we succeed.  Don't disable a
+         shlib_disabled breakpoint though.  There's a fair chance we
+         can't re-set it if the shared library it's in hasn't been
+         loaded yet.  */
       save_enable = b->enable_state;
-      b->enable_state = bp_disabled;
+      if (b->enable_state != bp_shlib_disabled)
+        b->enable_state = bp_disabled;
 
       set_language (b->language);
       input_radix = b->input_radix;

Re: [5.1/breakpoint] shlib patch?

[Andrew Cagney]

So ok.  Mark K wrote part of:

>  Date: Thu, 27 Sep 2001 00:38:35 -0400
>    From: Andrew Cagney <ac131313@cygnus.com>
> 
> Just FYI,
> 
>    This is probably the 5.1 release hack candidate (in branch, not in 
>    trunk).  Every release has one ...  Unless someone comes up with 
>    something that is.
> 
> And a hack it is, although Apple's Darwin version of GDB contains an
> almost identical hack to implement a "future break" command.  It seems
> to work in many real world cases, but it is easy to come up with a
> test case where the patch doesn't help at all.
> 
> It seems that the way GDB implements breakpoints-in-shared-libraries
> was designed for a.out shared library systems (SunOS 4) where shared
> libraries were loaded at a fixed address in memory.  Since ELF shared
> libraries can (and will) be loaded at any address in memory, things
> break.  Fixing this is not trivial.  Therefore, I'm not sure whether
> we should add this hack to the branch only.  I cannot guarantee that
> things will be fixed on the trunk in the near future.
> 
> Anyway, for reference I attached the hack.

Can someone give me a good reason to not commit this.  Trunk and branch 
....!  While still broken, is it worse than before?

Andrew


> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 breakpoint.c
> --- breakpoint.c 2001/09/18 05:00:48 1.53
> +++ breakpoint.c 2001/09/28 09:27:52
> @@ -7009,10 +7009,14 @@ breakpoint_re_set_one (PTR bint)
>  	  delete_breakpoint (b);
>  	  return 0;
>  	}
> -      /* In case we have a problem, disable this breakpoint.  We'll restore
> -         its status if we succeed.  */
> +      /* In case we have a problem, disable this breakpoint.  We'll
> +         restore its status if we succeed.  Don't disable a
> +         shlib_disabled breakpoint though.  There's a fair chance we
> +         can't re-set it if the shared library it's in hasn't been
> +         loaded yet.  */
>        save_enable = b->enable_state;
> -      b->enable_state = bp_disabled;
> +      if (b->enable_state != bp_shlib_disabled)
> +        b->enable_state = bp_disabled;
>  
>        set_language (b->language);
>        input_radix = b->input_radix;
> 
> 
> 
> 

Re: [5.1/breakpoint] shlib patch?

[Andrew Cagney]

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

FYI,

I've checked in the attached. 5.1 and trunk.

Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 2289 bytes --]

2001-11-11  Andrew Cagney  <ac131313@redhat.com>

	From Mark Kettenis  <kettenis@gnu.org>:
	* breakpoint.c (breakpoint_re_set_one): Don't discard SHLIB
	breakpoints when they fail.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.51.2.1
diff -p -r1.51.2.1 breakpoint.c
*** breakpoint.c	2001/08/02 12:05:56	1.51.2.1
--- breakpoint.c	2001/11/11 16:33:51
*************** breakpoint_re_set_one (PTR bint)
*** 7009,7018 ****
  	  delete_breakpoint (b);
  	  return 0;
  	}
!       /* In case we have a problem, disable this breakpoint.  We'll restore
!          its status if we succeed.  */
        save_enable = b->enable_state;
!       b->enable_state = bp_disabled;
  
        set_language (b->language);
        input_radix = b->input_radix;
--- 7009,7040 ----
  	  delete_breakpoint (b);
  	  return 0;
  	}
!       /* HACK: cagney/2001-11-11: kettenis/2001-11-11: MarkK wrote:
! 
! 	 ``And a hack it is, although Apple's Darwin version of GDB
! 	 contains an almost identical hack to implement a "future
! 	 break" command.  It seems to work in many real world cases,
! 	 but it is easy to come up with a test case where the patch
! 	 doesn't help at all.''
! 
! 	 ``It seems that the way GDB implements breakpoints - in -
! 	 shared - libraries was designed for a.out shared library
! 	 systems (SunOS 4) where shared libraries were loaded at a
! 	 fixed address in memory.  Since ELF shared libraries can (and
! 	 will) be loaded at any address in memory, things break.
! 	 Fixing this is not trivial.  Therefore, I'm not sure whether
! 	 we should add this hack to the branch only.  I cannot
! 	 guarantee that things will be fixed on the trunk in the near
! 	 future.''
! 
!          In case we have a problem, disable this breakpoint.  We'll
!          restore its status if we succeed.  Don't disable a
!          shlib_disabled breakpoint though.  There's a fair chance we
!          can't re-set it if the shared library it's in hasn't been
!          loaded yet.  */
        save_enable = b->enable_state;
!       if (b->enable_state != bp_shlib_disabled)
!         b->enable_state = bp_disabled;
  
        set_language (b->language);
        input_radix = b->input_radix;