From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18776 invoked by alias); 21 Oct 2013 04:29:08 -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 17594 invoked by uid 89); 21 Oct 2013 04:29:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 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-f181.google.com Received: from mail-ie0-f181.google.com (HELO mail-ie0-f181.google.com) (209.85.223.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 21 Oct 2013 04:29:05 +0000 Received: by mail-ie0-f181.google.com with SMTP id ar20so10863479iec.12 for ; Sun, 20 Oct 2013 21:29:03 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.50.141.133 with SMTP id ro5mr7903732igb.35.1382329742816; Sun, 20 Oct 2013 21:29:02 -0700 (PDT) Received: by 10.50.47.169 with HTTP; Sun, 20 Oct 2013 21:29:02 -0700 (PDT) In-Reply-To: References: Date: Mon, 21 Oct 2013 04:29:00 -0000 Message-ID: Subject: Re: [PATCH] bug fix for gdb 16039 From: "Dave.Tian" To: Sergio Durigan Junior , tromey@redhat.com Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes X-SW-Source: 2013-10/txt/msg00614.txt.bz2 Testcase below: ----Makefile---- so: g++ -shared -static-libstdc++ -static-libgcc -fPIC -rdynamic dy.cxx -m32 -o libdy.so all: so g++ -m32 -ldl main.cxx -g -o main ----dy.cxx---- #include using namespace std; extern "C" const char * getVerson() { std::string verson = "1.0"; return verson.c_str(); } ----main.cxx---- #include #include #include #include using namespace std; int main() { void *handle = dlopen("libdy.so", RTLD_LAZY); if (!handle) { fprintf (stderr, "%s\n", dlerror()); return 1; } typedef const char*(*FUNCTION)(); FUNCTION f = (FUNCTION)dlsym(handle, "getVerson"); std::cout << f(); dlclose(handle); // 'next' cmd stop work here int a, b, c; a = 1; b = 2; c = 3; std::cout << a << b << c; } thank you, -YT 2013/10/19 Sergio Durigan Junior : > On Wednesday, October 16 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. >> >> 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. > > Thanks for the patch. > > Just FYI: the formatting of this ChangeLog entry is wrong because it > doesn't use TABs. However, the entry you posted on the previous version > of this patch was right, so I think this was just a copy & paste done > wrong :-). Anyway, just FYI as I said. > > I would also like to point that you still haven't contacted me about the > copyright assignment. You need to do that paperwork before your patch > can be committed (don't worry, it's just this time). Just send me an > email offlist and I can send you the instructions. > >> 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); >> +} > > Thanks for extending this function, but I still think it's incomplete. > My rationale here is: I liked your previous idea of creating a function > to identify whether a breakpoint is internal. I think it is more > complete this way. However, such a function (IMO) should cover all the > known possibilites of internal breakpoints. And your list is not > exhaustive, for sure. So, IMO, you could take a closer at the possible > internal breakpoints and properly extend this list. Also, when you do > that, you could rename your function to "is_internal_breakpoint". > > Well, this is my opinion. I don't know what others think. But I > certainly don't like this new function the way it is. > > The rest of the patch looks good to me, but you should also provide a > testcase for it as Tom noted. > > Thanks a lot for doing that, > > -- > Sergio