Re: [RFA]: Fix for pending breakpoints in manually loaded/unloaded shlibs

[Daniel Jacobowitz <drow@false.org>]

On Tue, Aug 10, 2004 at 03:09:37PM -0400, Jeff Johnston wrote:
> The following patch fixes a problem with breakpoints set in shlibs that are 
> manually loaded/unloaded by the program.  What currently happens is that 
> pending breakpoints work properly for the first run of the program.  On the 
> 2nd run, the resolved breakpoint(s) can end up at the start of the 
> breakpoint list and is marked bp_shlib_disabled.  This is fine for a bit 
> and we reach the breakpoint again when the shared library is loaded.  
> However, when we unload the 2nd time, there is trouble.  We eventually get 
> a shlib_event from the dlclose() and we attempt to remove the breakpoint to 
> step over it.  Unfortunately, we try and remove all breakpoints and we end 
> attempting to remove a breakpoint that no longer exists (remember the 
> breakpoint for the shared library routine is now at the start of the 
> breakpoint list).  We fail trying to remove the first breakpoint and end up 
> failing remove_breakpoints.  We subsequently keep running into the 
> shlib_event breakpoint over and over again ad-infinitum.

I couldn't quite follow your explanation of the problem, but FWIW your
patch does make sense to me.

Please check it for coding style problems; I noticed a lot of operators
at the end of lines instead of the beginning of the next line.

> +#if defined (PC_SOLIB)
> +    if (((b->type == bp_breakpoint) ||
> +	 (b->type == bp_hardware_breakpoint)) &&
> +	breakpoint_enabled (b) &&
> +	!b->loc->duplicate)

You are just grabbing this from disable_breakpoints_in_shlibs, but the
b->type check is not correct.  Try
  (b->loc->type == bp_loc_hardware_breakpoint
   || b->loc->type == bp_loc_software_breakpoint)

[Conceptually, you want any breakpoint which corresponds to a code
address.]

Can this code be commonized rather than duplicated?

> +      {
> +	char *so_name = PC_SOLIB (b->loc->address);
> +	if (so_name &&
> +	    !strcmp (so_name, solib->so_name))
> +          {
> +	    b->enable_state = bp_shlib_disabled;
> +	    b->loc->inserted = 0;

Are we guaranteed that the breakpoint is not inserted right now?  This
is the only place in breakpoint.c that changes the inserted flag
directly other than initialization, insertion, and a hack for exec
following.

If you expect that the library has already been unmapped, so removing
it would fail, please add a comment saying so.

-- 
Daniel Jacobowitz