Message ID | 867etxpn9k.fsf@gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 60817 invoked by alias); 8 Dec 2017 09:44:33 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 58440 invoked by uid 89); 8 Dec 2017 09:44:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=sal, H*r:sk:static. X-HELO: mail-wr0-f180.google.com Received: from mail-wr0-f180.google.com (HELO mail-wr0-f180.google.com) (209.85.128.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 08 Dec 2017 09:44:31 +0000 Received: by mail-wr0-f180.google.com with SMTP id a41so10227209wra.6 for <gdb-patches@sourceware.org>; Fri, 08 Dec 2017 01:44:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=hGiILdCGEgefaWK1bcNJddqg2BvANStEzLx7fv8q824=; b=g3Ro1Hxoz0FD4Lp7fL70nKRwOtHp7ugz55PzfUK0O+wNBY3I7paaHHotRDpXEqSkZp Q1RWM973uHJaMlnW0RP2/zc3/V11KyVrVR6rKAzPVFld06PhrY5qIcAPTDGTnY7lJf5L RpyxzIBvnTL/BMISkPsCWSos0EETwnEc8aLZa05RqDQTwCG2IBmhgUsk0zg85olazjt9 Rl53dEbsCSGrzW36SNvI0TQLU63fOtumKitQnpT9MU/WxNjKtICjd4boaFA887YJynC4 +6Zu18/w36Fx/Ft9pxC5l467FAqhTj4vXEHLWHex9BqU5GCBbD/yDcf7kGCnBCT7Lm5E WBfA== X-Gm-Message-State: AJaThX6ho3eWralB4WbNyXw1Ms2Xuhf9fkmnlE5M9XyPULKo1BWNDH9N ufImgOjJXO/BqXvZXLOlnCrmRg== X-Google-Smtp-Source: AGs4zMYvkOkP8aPiSNCye2aSyKvYi4tzzoEl2Dm/O+RypyE7xAtFyL46pGLPzCgKjwpmWupNNvKrzg== X-Received: by 10.223.190.134 with SMTP id i6mr25804485wrh.177.1512726269041; Fri, 08 Dec 2017 01:44:29 -0800 (PST) Received: from E107787-LIN (static.42.136.251.148.clients.your-server.de. [148.251.136.42]) by smtp.gmail.com with ESMTPSA id g7sm9272048wra.38.2017.12.08.01.44.26 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Fri, 08 Dec 2017 01:44:28 -0800 (PST) From: Yao Qi <qiyaoltc@gmail.com> To: Pedro Alves <palves@redhat.com> Cc: Ulrich Weigand <uweigand@de.ibm.com>, Jan Kratochvil <jan.kratochvil@redhat.com>, Simon Marchi <simon.marchi@ericsson.com>, Keith Seitz <keiths@redhat.com>, gdb-patches@sourceware.org Subject: Re: [PATCH] Fix setting-breakpoints regression on PPC64 (function descriptors) References: <20171126163756.1515ED802F9@oc3748833570.ibm.com> <0bea6805-b8eb-da2c-07f6-0f1ee917c7b5@redhat.com> Date: Fri, 08 Dec 2017 09:44:23 +0000 In-Reply-To: <0bea6805-b8eb-da2c-07f6-0f1ee917c7b5@redhat.com> (Pedro Alves's message of "Wed, 29 Nov 2017 19:07:54 +0000") Message-ID: <867etxpn9k.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes |
Commit Message
Yao Qi
Dec. 8, 2017, 9:44 a.m. UTC
Pedro Alves <palves@redhat.com> writes: > @@ -4309,22 +4309,16 @@ minsym_found (struct linespec_state *self, struct objfile *objfile, > struct minimal_symbol *msymbol, > std::vector<symtab_and_line> *result) > { > - struct gdbarch *gdbarch = get_objfile_arch (objfile); > - CORE_ADDR pc; > struct symtab_and_line sal; > > - if (msymbol_is_text (msymbol)) > - { > - sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (objfile, msymbol), > - (struct obj_section *) 0, 0); > - sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol); This line is removed, so that sal.section can be null. It causes PR 22567. How is the patch below? > + /* The minimal symbol might point to a function descriptor, which is > + not a text symbol; try resolving it to the actual code > + address. */ > > - /* The minimal symbol might point to a function descriptor; > - resolve it to the actual code address instead. */ > - pc = gdbarch_convert_from_func_ptr_addr (gdbarch, sal.pc, > - ¤t_target); > - if (pc != sal.pc) > - sal = find_pc_sect_line (pc, NULL, 0); > + CORE_ADDR func_addr; > + if (msymbol_is_function (objfile, msymbol, &func_addr)) > + { > + sal = find_pc_sect_line (func_addr, NULL, 0); > > if (self->funfirstline) > {
Comments
On 12/08/2017 09:44 AM, Yao Qi wrote: > Pedro Alves <palves@redhat.com> writes: > >> @@ -4309,22 +4309,16 @@ minsym_found (struct linespec_state *self, struct objfile *objfile, >> struct minimal_symbol *msymbol, >> std::vector<symtab_and_line> *result) >> { >> - struct gdbarch *gdbarch = get_objfile_arch (objfile); >> - CORE_ADDR pc; >> struct symtab_and_line sal; >> >> - if (msymbol_is_text (msymbol)) >> - { >> - sal = find_pc_sect_line (MSYMBOL_VALUE_ADDRESS (objfile, msymbol), >> - (struct obj_section *) 0, 0); >> - sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol); > > This line is removed, so that sal.section can be null. It causes > PR 22567. How is the patch below? That's fine with me. I guess we end up with the wrong section in the function descriptor / PPC64 case (".opd" instead of some kind of ".text" where the resolved function lives), but it shouldn't matter, since the old code did that as well, AFAICT. (I noticed that get_sal_arch doesn't consider sal.objfile, probably because it predates addition of the 'obfile' field. We could probably fill in / use that field more, but we don't need to do that now.) Pedro Alves
On Fri, Dec 8, 2017 at 11:34 AM, Pedro Alves <palves@redhat.com> wrote: > That's fine with me. I guess we end up with the wrong section > in the function descriptor / PPC64 case (".opd" instead of some kind > of ".text" where the resolved function lives), but it shouldn't > matter, since the old code did that as well, AFAICT. I tested the patch on gcc110, there is no regression. I pushed it in. > > (I noticed that get_sal_arch doesn't consider sal.objfile, probably > because it predates addition of the 'obfile' field. We could probably > fill in / use that field more, but we don't need to do that now.) I noticed that too, but one thing I am not sure is that sal.objfile is *only* used for probe, /* The probe associated with this symtab_and_line. */ probe *prob = NULL; /* If PROBE is not NULL, then this is the objfile in which the probe originated. */ struct objfile *objfile = NULL; so can we use it in other cases (when probe is not used)? I don't know. If so, are sal.objfile and sal.section->objfile different or same? I need to look at the code there.
diff --git a/gdb/linespec.c b/gdb/linespec.c index 09758762..8c36f2a 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -4365,9 +4365,10 @@ minsym_found (struct linespec_state *self, struct objfile *objfile, sal.objfile = objfile; sal.pc = MSYMBOL_VALUE_ADDRESS (objfile, msymbol); sal.pspace = current_program_space; - sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol); } + sal.section = MSYMBOL_OBJ_SECTION (objfile, msymbol); + if (maybe_add_address (self->addr_set, objfile->pspace, sal.pc)) add_sal_to_sals (self, result, &sal, MSYMBOL_NATURAL_NAME (msymbol), 0); }