From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2714 invoked by alias); 16 Oct 2013 06:44:19 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 2699 invoked by uid 89); 16 Oct 2013 06:44:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ie0-f176.google.com Received: from mail-ie0-f176.google.com (HELO mail-ie0-f176.google.com) (209.85.223.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 16 Oct 2013 06:44:17 +0000 Received: by mail-ie0-f176.google.com with SMTP id u16so551816iet.21 for ; Tue, 15 Oct 2013 23:44:15 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.50.178.234 with SMTP id db10mr20478276igc.35.1381905855412; Tue, 15 Oct 2013 23:44:15 -0700 (PDT) Received: by 10.50.47.169 with HTTP; Tue, 15 Oct 2013 23:44:15 -0700 (PDT) In-Reply-To: References: Date: Wed, 16 Oct 2013 06:44:00 -0000 Message-ID: Subject: Re: [PATCH] bug fix for gdb 16039 From: "Dave.Tian" To: gdb-patches@sourceware.org Cc: Sergio Durigan Junior Content-Type: text/plain; charset=ISO-8859-1 X-SW-Source: 2013-10/txt/msg00469.txt.bz2 1 Description: Gdb bug 16039 created by me Title: Gdb next" command stop working when shared library unloaded. Root Cause: There is "libc++" static linked into the shared library, and since gdb insert internal breakpoints on std::terminate/longjump/... so after dlclose, the memory address is invalid,remove_breakpoints/insert_breakpoints failed with EIO error. Fix: Disable the internal breakpoints when dlclose hit. 2 ChangeLog: 2013-10-16 Tian Ye PR gdb/16039 * breakpoint.c (is_removable_in_unloaded_shlib): New function. (set_longjmp_breakpoint): Check if breakpoint's location address is not in an unloaded shared library (disable_breakpoints_in_unloaded_shlib): Disable the internal breakpoints in the hook function of shared library unload. 3 Patch Diffs: --- breakpoint.c.bak 2013-10-12 01:15:09.044081000 -0700 +++ breakpoint.c 2013-10-15 23:10:50.241167000 -0700 @@ -1118,6 +1118,23 @@ is_tracepoint (const struct breakpoint * return is_tracepoint_type (b->type); } +/* Return 1 if B is an internal memory breakpoint that + * can be removed when shlib is unloaded, or 0 otherwise. */ + +static int +is_removable_membreak_in_unloaded_shlib (const struct breakpoint *b) +{ + return (b->type == bp_longjmp + || b->type == bp_longjmp_resume + || b->type == bp_longjmp_call_dummy + || b->type == bp_exception + || b->type == bp_exception_resume + || b->type == bp_std_terminate + || b->type == bp_longjmp_master + || b->type == bp_std_terminate_master + || b->type == bp_exception_master); +} + /* A helper function that validates that COMMANDS are valid for a breakpoint. This function will throw an exception if a problem is found. */ @@ -7147,8 +7164,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 +7480,8 @@ 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) + || is_removable_in_unloaded_shlib (b)) && solib_contains_address_p (solib, loc->address)) { loc->shlib_disabled = 1; thank you, -YT 2013/10/15 Sergio Durigan Junior : > On Tuesday, October 15 2013, Dave Tian wrote: > >> 1 Description: Gdb bug 16039 created by me >> >> Title: Gdb next" command stop working when shared library unloaded. >> Root Cause: There is "libc++" static linked into the shared library, >> and since gdb insert internal breakpoints on std::terminate/longjump/... >> so after dlclose, the memory address is >> invalid,remove_breakpoints/insert_breakpoints >> failed with EIO error. >> >> Fix: Disable the internal breakpoints when dlclose hit. > > Almost there, Dave! Thanks :-). > >> 2 ChangeLog: >> >> 2013-10-16 Tian Ye >> >> PR gdb/16039 >> * breakpoint.c (is_removable_in_unloaded_shlib): New. > ^^^^ > > Write "New function" instead. > >> (set_longjmp_breakpoint): Check if breakpoint's location >> address is not in an unloaded shared library >> (disable_breakpoints_in_unloaded_shlib): Disable the >> internal breakpoints in the hook function of shared >> library unload. >> >> 3 Patch Diffs: >> >> --- breakpoint.c.bak 2013-10-12 01:15:09.044081000 -0700 >> +++ breakpoint.c 2013-10-15 18:50:59.842116000 -0700 >> @@ -1118,6 +1118,17 @@ is_tracepoint (const struct breakpoint * >> return is_tracepoint_type (b->type); >> } >> >> +/* Breakpoints should be disable when shlib unload. */ > > The comment should describe what the function does. Something like: > > /* Return 1 if B is an internal breakpoint that can be removed when a > shared library is unloaded, or 0 otherwise. */ > >> + >> +static int >> +is_removable_in_unloaded_shlib(const struct breakpoint *b) > ^^^ > > Space between function name and parenthesis. > >> +{ >> + return (b->type == bp_longjmp_master >> + || b->type == bp_std_terminate_master >> + || b->type == bp_exception_master >> + || b->type == bp_exception); >> +} > > Also, would you mind explaining how you came to the conclusion that only > those types of internal breakpoints can be present in a shared library? > I looked a little bit through the code and could not find any > explanation about this assumption. I remember that I commented that > maybe you should extend this function in order to identify the other > kinds of internal breakpoints... > > It would be nice to hear what other maintainers think of Dave's > approach, so that he doesn't waste his with this detail. > >> + >> /* A helper function that validates that COMMANDS are valid for a >> breakpoint. This function will throw an exception if a problem is >> found. */ >> @@ -7147,8 +7158,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 +7474,8 @@ 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) >> + || is_removable_in_unloaded_shlib(b)) > ^^^ > > Space between function name and parenthesis. > >> && solib_contains_address_p (solib, loc->address)) >> { >> loc->shlib_disabled = 1; > > -- > Sergio