This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: bug fix for gdb 16039
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: "Dave.Tian" <xhengdf at gmail dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 14 Oct 2013 01:26:00 -0300
- Subject: Re: bug fix for gdb 16039
- Authentication-results: sourceware.org; auth=none
- References: <CAKjgB1MjUS8rNApUMrE=98gf3spM0fxXKwW_Tx4PKzUsTk1uNA at mail dot gmail dot com>
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