This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: bug fix for gdb 16039


On Saturday, October 12 2013, Dave Tian wrote:

> 1, description : I create a defect for gdb Bug 16039
>        gdb "next " cmd stop working when a dynamic library(libc++
> static linked in) unloaded
>
>        there is a dynamic library with libc++ static linked.
>        when call dlclose on this library, gdb "next" cmd stop to work.
>        since gdb has internal breakpoints on
> std::terminate/longjmp/..., which set when dlopen on the library.
>        after dlclose, the address of mem-break is not valid, so
> remove_breakpoints/insert_breakpoints failed with EIO error. then gdb
> "next" failed with "cannot insert breapoint -XXX".

Thank you for the patch.  This is not really a review, but I saw some
formatting nits that I thought I'd better point.

> 2, ChangeLog :
>       2013-10-12 Tian Ye <xhengdf@gmail.com>
>
>                * breakpoint.c :  Bug 16039 fix

The correct format for ChangeLog entries is describe at
<https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog>.
Yours should be something like:

2013-10-12  Tian Ye  <xhengdf@gmail.com>

	PR gdb/16039
	* breakpoint.c (set_longjmp_breakpoint): Check if breakpoint's
	location address is not in an unloaded shared library.
	...

Or something like that.

> 3, Patch diff:
>
>  ---   breakpoint.c.old 2013-10-12 01:15:09.044081000 -0700
> +++ breakpoint.c.new 2013-10-12 03:04:16.992359000 -0700

Your patch is not properly formatted, so it does not apply cleanly.
Please take a look at your MUA's configuration and resend it.

> @@ -7147,8 +7147,8 @@ set_longjmp_breakpoint (struct thread_in
>       clones of those and enable them for the requested thread.  */
>    ALL_BREAKPOINTS_SAFE (b, b_tmp)
>      if (b->pspace == current_program_space
> - && (b->type == bp_longjmp_master
> -    || b->type == bp_exception_master))
> + && ((b->type == bp_longjmp_master
> +    || b->type == bp_exception_master) && !b->loc->shlib_disabled))
>        {
>   enum bptype type = b->type == bp_longjmp_master ? bp_longjmp : bp_exception;
>   struct breakpoint *clone;
> @@ -7463,7 +7463,11 @@ disable_breakpoints_in_unloaded_shlib (s
>        || b->type == bp_hardware_breakpoint)
>       && (loc->loc_type == bp_loc_hardware_breakpoint
>   || loc->loc_type == bp_loc_software_breakpoint))
> -     || is_tracepoint (b))
> +    || is_tracepoint (b)
> +    || (b->type == bp_longjmp_master
> +        || b->type == bp_std_terminate_master
> +        || b->type == bp_exception_master
> +        || b->type == bp_exception))

Just as a comment, this "if" will become really ugly after this :-/.
Maybe some helpers could be created to make it more clear, don't know.

>   && solib_contains_address_p (solib, loc->address))
>        {
>   loc->shlib_disabled = 1;

Thanks for the patch.  Those are the comments I have so far, but I
intend to take a closer look at it when you resubmit.  BTW, do you have
a copyright assignment on file?  If you don't, mail me offlist and I
will get you started on the process.

Regards,

-- 
Sergio


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]