From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id FQU0Ej5hjV8PPwAAWB0awg (envelope-from ) for ; Mon, 19 Oct 2020 05:49:50 -0400 Received: by simark.ca (Postfix, from userid 112) id 3D7831EDF4; Mon, 19 Oct 2020 05:49:50 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id D6FCB1E58C for ; Mon, 19 Oct 2020 05:49:48 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 51804384243A; Mon, 19 Oct 2020 09:49:48 +0000 (GMT) Received: from mail-wr1-x444.google.com (mail-wr1-x444.google.com [IPv6:2a00:1450:4864:20::444]) by sourceware.org (Postfix) with ESMTPS id 350993858020 for ; Mon, 19 Oct 2020 09:49:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 350993858020 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x444.google.com with SMTP id n6so10583294wrm.13 for ; Mon, 19 Oct 2020 02:49:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=I+hSzw3rfncC8eSiXZXbG4Rla3rUCrOVYyizx8UJng4=; b=FhzEiPtiwEyJTKvUdIEgbC9JEmkeed0GgZ8tni+PDMtwK0+EZpEcIRY+cfpwO6tqsE WqXVNKWq+SJ/jS7Sd98ovY8QZrKuZm99rAF6QzZA1jN9JWl3yXbdEPphqI/NBm+JsPoL raf4ZODoZjoe9+tQEs8gpiEF2dGblP67EC7fjt3KKZiv7v3Cu93vsnTrO5+fJt17g1w4 Kjbp28gRuJKNuvJx7AM0EBLkdQqVddxKffcx0FCebvsAf7rZrShawyWRKzH+A+28cyKw yEOn+OtaBC6mMDqn3C88Pm/eLse0w/sVkBW0oBx3mw7xvVpO8YAk0K+0G6J9Cks9H7q6 K4ZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=I+hSzw3rfncC8eSiXZXbG4Rla3rUCrOVYyizx8UJng4=; b=iJ4D0L9nquoHp4Zi+ZqsjVlarNQ4/QFhbrWJ0gdBHs8cDueo4NEuxoRjJibNZhVGDx ZZBldaP9F2EhGd9aba4f0oSWc+EAv/l4DnHD+L6IYfsAYObb1f21oQwqNFECRnhuhdIN y0nIe7C6KZaSvf47ne7cAslUspDlogD6ZoLSEMadG6W/vlbnTCbZk9tbF/o1b+ewa83P sw4brvhQMDh3c5N/ot9Urw4dFnuriigF+O/owq5Oba8meMbZ/VqN3DmFpAUW4pAeIa9u 4lq+HtaeoDyR26K8DjutFfpyMBGFJfWtt8+x/jnQesc2RDZWK9jXnQFcyFHjgy8uYreg R9ag== X-Gm-Message-State: AOAM533rhGC5LvidkuOJ4u02zIAwLrZ/VjeKbiTWFas8/0BX1hrscc9C O501TuhlRubmeVouow5HVPsyuwxm6iYnLg== X-Google-Smtp-Source: ABdhPJyP3fEJMzU8rHAJQIjjnHpuGnqq8wTtJaeX6QNqJwD8m4P/XAC2RFPlhsMY5QmigtCf5/6kAw== X-Received: by 2002:adf:a345:: with SMTP id d5mr20236087wrb.55.1603100984890; Mon, 19 Oct 2020 02:49:44 -0700 (PDT) Received: from localhost (host86-166-129-235.range86-166.btcentralplus.com. [86.166.129.235]) by smtp.gmail.com with ESMTPSA id m1sm15580623wmm.34.2020.10.19.02.49.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Oct 2020 02:49:44 -0700 (PDT) Date: Mon, 19 Oct 2020 10:49:43 +0100 From: Andrew Burgess To: Mihails Strasuns Subject: Re: [PATCH] gdb: get jiter objfile from a bound minsym Message-ID: <20201019094943.GA28735@embecosm.com> References: <20201014090003.17736-1-mihails.strasuns@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201014090003.17736-1-mihails.strasuns@intel.com> X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 10:43:18 up 4 days, 1:48, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" * Mihails Strasuns via Gdb-patches [2020-10-14 11:00:03 +0200]: > This fixes a regression introduced by the following commit: > > fe053b9e853 gdb/jit: pass the jiter objfile as an argument to jit_event_handler > > In the refactoring `handle_jit_event` function was changed to pass a matching > objfile pointer to the `jit_event_handler` explicitly, rather using internal > storage: > > ``` > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -5448,8 +5448,9 @@ handle_jit_event (void) > > frame = get_current_frame (); > gdbarch = get_frame_arch (frame); > + objfile *jiter = symbol_objfile (get_frame_function (frame)); > > - jit_event_handler (gdbarch); > + jit_event_handler (gdbarch, jiter); > ``` > > This was needed to add support for a multiple jiters in a following commits. > However it has also introduced a regression, because `get_frame_function > (frame)` here may return `nullptr`, resulting in a crash. > > A more resilient way would be to use an approach mirroring > `jit_breakpoint_re_set` - to find a minimal symbol matching the breakpoint > location and use its object file. We know that this breakpoint events comes > from a breakpoint set by `jit_breakpoint_re_set`, thus using the reverse > approach should be reliable enough. > > gdb/Changelog: > 2020-10-14 Mihails Strasuns > > * breakpoint.c (handle_jit_event): and an argument, change how > `jit_event_handler` is called. > --- > gdb/breakpoint.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index 414208469f9..878d48c5984 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -5434,9 +5434,8 @@ bpstat_stop_status (const address_space *aspace, > } > > static void > -handle_jit_event (void) > +handle_jit_event (CORE_ADDR address) > { > - struct frame_info *frame; > struct gdbarch *gdbarch; > > if (debug_infrun) > @@ -5446,11 +5445,9 @@ handle_jit_event (void) > breakpoint_re_set. */ > target_terminal::ours_for_output (); > > - frame = get_current_frame (); > - gdbarch = get_frame_arch (frame); > - objfile *jiter = symbol_objfile (get_frame_function (frame)); > - > - jit_event_handler (gdbarch, jiter); > + gdbarch = get_frame_arch (get_current_frame ()); > + bound_minimal_symbol jit_bp_sym = lookup_minimal_symbol_by_pc (address); > + jit_event_handler (gdbarch, jit_bp_sym.objfile); It might be nice to add: gdb_assert (jit_bp_sym.objfile != nullptr); before the call to jit_event_handler. Even better (IMO) would be to add a short comment explaining why it is that the assertion should be true, though this isn't essential as the assertion can easily be tracked back to this commit which includes an explanation. I'm not very (or at all) familiar with this area of GDB, but the change seems reasonable so I'd be happy to see this merged with the assertion added. Thanks, Andrew > > target_terminal::inferior (); > } > @@ -5651,7 +5648,7 @@ bpstat_run_callbacks (bpstat bs_head) > switch (b->type) > { > case bp_jit_event: > - handle_jit_event (); > + handle_jit_event (bs->bp_location_at->address); > break; > case bp_gnu_ifunc_resolver: > gnu_ifunc_resolver_stop (b); > -- > 2.17.1 > > Intel Deutschland GmbH > Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, www.intel.de > Managing Directors: Christin Eisenschmid, Gary Kershaw > Chairperson of the Supervisory Board: Nicole Lau > Registered Office: Munich > Commercial Register: Amtsgericht Muenchen HRB 186928 >