From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 109550 invoked by alias); 23 Mar 2016 02:18:47 -0000 Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org Received: (qmail 109520 invoked by uid 89); 23 Mar 2016 02:18:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-qg0-f54.google.com Received: from mail-qg0-f54.google.com (HELO mail-qg0-f54.google.com) (209.85.192.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 23 Mar 2016 02:18:35 +0000 Received: by mail-qg0-f54.google.com with SMTP id j35so1825606qge.0 for ; Tue, 22 Mar 2016 19:18:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=fOagP1oSGxU0+mESED5pDpcYWdeHPQ2IJG6rEQEuOgA=; b=ZW7ahgLVOpDMk8B1se4Ez0TTCogM8iFO01ZsPyA6Hr8S5RWxxFzaMuAjUWpyYw48Uo KMIM8yyzdOCY0nfa+TScPfzN04O0q2KwMRk27sn3irG44DXv3GM0ZU5axkhCjS83/OMy dwZXP+CoUuWm5z4edGraIkLpeFkODLjBHIMf9cL/MBe3/YqfpcY3E1LMyIukrSwGVdOk ezY4WSWFy6lHRzT0Z7uDFPRA0wImqC8J1JK7IJR8UcmG8UOF0hOPwhOaNtyTM1BtX8OF /r8CIcvLw07FI6z1mEbKCUZa8VBnXgV4vMGReW4bFpbcDFLwqbA/hGMp4a3eijAhHoOG zN3g== X-Gm-Message-State: AD7BkJJVLxSl5BrZ4S2vCI8qmpQmTOQMaDcQmII2TFIUMWDJw2g8aOjFvq+gSBZJtV7SOuVue96MFnkQtN1oAg== X-Received: by 10.140.135.84 with SMTP id 81mr412607qhh.26.1458699513299; Tue, 22 Mar 2016 19:18:33 -0700 (PDT) MIME-Version: 1.0 Received: by 10.55.189.130 with HTTP; Tue, 22 Mar 2016 19:18:13 -0700 (PDT) In-Reply-To: <56F17A23.90909@redhat.com> References: <56F168D7.9050405@redhat.com> <56F16F8F.9050404@redhat.com> <56F1759F.3070100@redhat.com> <56F17A23.90909@redhat.com> From: Yichao Yu Date: Wed, 23 Mar 2016 02:18:00 -0000 Message-ID: Subject: Re: JIT debugging (Attach and speed) To: Pedro Alves Cc: gdb@sourceware.org, Paul Pluzhnikov Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-03/txt/msg00033.txt.bz2 Sorry for the delay, I was busy with other stuff... >>>>> I re-read the 2011 discussion, and it seems like we had an idea for a fix: >>>> >>>> IIUC the proposed fix might cause regression in some cases? >>> >>> Yeah, there's no full fix available, only some ideas thrown out. >>> The last discussed one wouldn't cause a regression -- the >>> "longjmp"-caching idea. We may still need to defer breakpoint re-set >>> to at most once per jit load event, something like Paul's original >>> patch, but with a breakpoint_re_set call somewhere. >>> >>> It'd even be better to somehow restrict breakpoint re-setting >>> to the jit modules that were added/removed/changed, but >>> that's harder. >>> >>>> >>>>>> >>>>>> Do you know whether this happens with 7.11 and master, and if so, >>>>>> would it be possible for you to git bisect the culprit? >>>> >>>> This is 7.11 package from ArchLinux. I could try bi-secting although >>>> apparently you are faster at pin-point the issue. >>>> >>>>> >>>>> Currently, jit_inferior_created_hook -> jit_inferior_init is only >>>>> called when the inferior execs... >>>>> >>>>> Grepping around, I think that might have been >>>>> the fix for PR gdb/13431 (03bef283c2d3): >>>>> https://sourceware.org/ml/gdb-patches/2012-02/msg00023.html >>>>> which removed the inferior_created (jit_inferior_created_observer). >>>>> >>>>> Adding an inferior_created observer back likely fixes the issue. >>>> >>>> I'm happy to test patches. >>> >>> I'm happy to provide guidance, but a fix would likely happen faster >>> if someone else stepped up to write it. >> >> Are these lines (or at least the first one) the ones you think should >> be added back? >> >> - observer_attach_inferior_created (jit_inferior_created_observer); >> observer_attach_inferior_exit (jit_inferior_exit_hook); >> - observer_attach_executable_changed (jit_executable_changed_observer); >> > > Something like that. At least the first one. Not sure the second is > needed, since with Tromey's change the data is associated with the objfile. > >> I can try that although I'm not particularly sure what was the reason >> they are removed > > Not sure either. I assume studying Tromey's description of the original > change helps bring that to light. > >> and how to check for regressions. > > GDB has a regression test suite under src/gdb/testsuite/. The > gdb/testsuite/README file has instructions. > > Basically, run "make check -j8" before the patch, "make check -j8" > after the patch, and diff the resulting testsuite/gdb.sum files. > > Note that there are some tests that may be racy on your machine, so you > may get unrelated some noise. Running a particular test a > couple times, with: > > make check TESTS="gdb.base/foo.exp" > > should help you determine whether that's the case. > > It'd be very nice if we had a _new_ test that covers your use case, > to avoid regressing again. That likely makes the patch bigger than > what we could accept without a copyright assignment though. If you'd > like to pursue that, let me know and I'll send you the forms. I've got a simple patch that fixes the issue for me and AFAICT all of the failing tests are racy and/or failing on this machine before the change too. I haven't add test yet since I'm not so sure how to add it (I found test for both jit interface and attach but haven't figured out how to write a new one yet...). I'm happy to complete copyright related forms necessary. > > Thanks, > Pedro Alves > >From a94b2c68d83e13ee80e5c21ab27dfedaddfda590 Mon Sep 17 00:00:00 2001 From: Yichao Yu Date: Tue, 22 Mar 2016 15:24:11 -0400 Subject: [PATCH] Fix JIT debug when attaching to a process. --- gdb/jit.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/gdb/jit.c b/gdb/jit.c index afc1c51..0bd127b 100644 --- a/gdb/jit.c +++ b/gdb/jit.c @@ -1026,7 +1026,7 @@ jit_breakpoint_deleted (struct breakpoint *b) } /* (Re-)Initialize the jit breakpoint if necessary. - Return 0 on success. */ + Return 0 if the jit breakpoint has been successfully initialized. */ static int jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, @@ -1070,7 +1070,7 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, paddress (gdbarch, addr)); if (ps_data->cached_code_address == addr) - return 1; + return 0; /* Delete the old breakpoint. */ if (ps_data->jit_breakpoint != NULL) @@ -1288,7 +1288,8 @@ static const struct frame_unwind jit_frame_unwind = jit_frame_prev_register, NULL, jit_frame_sniffer, - jit_dealloc_cache + jit_dealloc_cache, + NULL }; @@ -1375,6 +1376,12 @@ jit_inferior_created_hook (void) jit_inferior_init (target_gdbarch ()); } +static void +jit_inferior_created (struct target_ops *ops, int from_tty) +{ + jit_inferior_created_hook (); +} + /* Exported routine to call to re-set the jit breakpoints, e.g. when a program is rerun. */ @@ -1496,6 +1503,7 @@ _initialize_jit (void) show_jit_debug, &setdebuglist, &showdebuglist); + observer_attach_inferior_created (jit_inferior_created); observer_attach_inferior_exit (jit_inferior_exit_hook); observer_attach_breakpoint_deleted (jit_breakpoint_deleted); -- 2.7.4