Message ID | F0AF0417-7A6F-48C6-B6E5-6999F9FEE3A7@arm.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 14230 invoked by alias); 1 Mar 2018 17:03:52 -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 14191 invoked by uid 89); 1 Mar 2018 17:03:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: EUR03-VE1-obe.outbound.protection.outlook.com Received: from mail-eopbgr50069.outbound.protection.outlook.com (HELO EUR03-VE1-obe.outbound.protection.outlook.com) (40.107.5.69) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Mar 2018 17:03:48 +0000 Received: from AM3PR08MB0101.eurprd08.prod.outlook.com (10.160.211.19) by AM3PR08MB0610.eurprd08.prod.outlook.com (10.163.188.148) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.548.13; Thu, 1 Mar 2018 17:03:44 +0000 Received: from AM3PR08MB0101.eurprd08.prod.outlook.com ([fe80::fc60:4b4d:7de8:f8b7]) by AM3PR08MB0101.eurprd08.prod.outlook.com ([fe80::fc60:4b4d:7de8:f8b7%16]) with mapi id 15.20.0527.022; Thu, 1 Mar 2018 17:03:44 +0000 From: Alan Hayward <Alan.Hayward@arm.com> To: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> CC: nd <nd@arm.com> Subject: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type Date: Thu, 1 Mar 2018 17:03:44 +0000 Message-ID: <F0AF0417-7A6F-48C6-B6E5-6999F9FEE3A7@arm.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM3PR08MB0610; 7:Q9vTUjfyetPCRup6YQyoNZk4GVNn6a4d+/vrb3jrex0Ovt2Mz4IlkMWIWgdzK/iWcjVRpqrdS1KOkgTOh4VYgq3AnY6js3FF213YSKUjhIaFX/h4BRK3QSDDkevGo1GkNnzvHX9JlqZbDKn2cwP7nxrzB3PPmVSing7opQhm1sLAShpPzdwXuLDtE9uX/7TuqPpp/8z36qF/jU/nYVLVL3AQdjTjQVrIEYVwQaU7yHLnTIzbnqns3p8zKbV1VkYL x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: e1689b41-8447-46c0-ddda-08d57f966454 x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(4534165)(4627221)(201703031133081)(201702281549075)(48565401081)(5600026)(4604075)(3008032)(2017052603307)(7153060)(7193020); SRVR:AM3PR08MB0610; x-ms-traffictypediagnostic: AM3PR08MB0610: nodisclaimer: True x-microsoft-antispam-prvs: <AM3PR08MB0610931DB9BF03DACAFA5D9D97C60@AM3PR08MB0610.eurprd08.prod.outlook.com> x-exchange-antispam-report-test: UriScan:(180628864354917); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040501)(2401047)(5005006)(8121501046)(10201501046)(3231220)(944501161)(52105095)(93006095)(93001095)(3002001)(6055026)(6041288)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(20161123560045)(20161123564045)(20161123562045)(6072148)(201708071742011); SRVR:AM3PR08MB0610; BCL:0; PCL:0; RULEID:; SRVR:AM3PR08MB0610; x-forefront-prvs: 05986C03E0 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39860400002)(39380400002)(396003)(366004)(376002)(346002)(199004)(189003)(377424004)(3846002)(68736007)(82746002)(25786009)(6512007)(316002)(6916009)(97736004)(2900100001)(8936002)(5660300001)(83716003)(99286004)(6116002)(66066001)(5640700003)(2501003)(36756003)(2906002)(81156014)(6506007)(106356001)(81166006)(2351001)(26005)(3660700001)(8676002)(186003)(33656002)(5250100002)(4326008)(6436002)(14454004)(72206003)(3280700002)(305945005)(478600001)(102836004)(7736002)(86362001)(6486002)(105586002)(53936002); DIR:OUT; SFP:1101; SCL:1; SRVR:AM3PR08MB0610; H:AM3PR08MB0101.eurprd08.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: EzIBJkCam32vMQ6IFAJEFSrDjpiQJ7zM2vOYCvaiKwDBAdt7lS65nX0YbtDZvKVB0jUGStJYvbVQJHLTrpGrM57+8ypT/ktj2IpdiyzVuSlLjxzwqqllJWbIRF7JO//QxwBnI30/DDBB5JU8lGwqzXPyK3JFAe7tDgwLGua0wfk= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-ID: <A649D4B91213BA4DB9DBF50283EB3EC7@eurprd08.prod.outlook.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: e1689b41-8447-46c0-ddda-08d57f966454 X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Mar 2018 17:03:44.5711 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM3PR08MB0610 X-IsSubscribed: yes |
Commit Message
Alan Hayward
March 1, 2018, 5:03 p.m. UTC
On aarch64, the (int) casting in the following causes a gdb segfault: $ ./gdb ./gdb (gdb) b dwarf2_physname if (int)strcmp (name, "another_thread_local") == 0 (gdb) run a.out // use any a.out This is due to getting a null pointer from TYPE_TARGET_TYPE, and then using it for language_pass_by_reference(). Fixed by adding a null check, similar to other occurrences in gdb. Tested on aarch64 with make check using unix, native_gdbserver. Alan. 2018-03-01 Alan Hayward <alan.hayward@arm.com> * aarch64-tdep.c (aarch64_push_dummy_call): Check for null return_type.
Comments
On Thu, Mar 01, 2018 at 05:03:44PM +0000, Alan Hayward wrote: > On aarch64, the (int) casting in the following causes a gdb segfault: > $ ./gdb ./gdb > (gdb) b dwarf2_physname if (int)strcmp (name, "another_thread_local") == 0 > (gdb) run a.out // use any a.out > > This is due to getting a null pointer from TYPE_TARGET_TYPE, and then > using it for language_pass_by_reference(). > > Fixed by adding a null check, similar to other occurrences in gdb. > > Tested on aarch64 with make check using unix, native_gdbserver. > > Alan. > > > 2018-03-01 Alan Hayward <alan.hayward@arm.com> > > * aarch64-tdep.c (aarch64_push_dummy_call): Check for null > return_type. The patch looks good to me, but do you think you could add a test for it? Intuitively, I think this should be fairly easily doable, but can you confirm? > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index f08945ea07101e1cd7906ca640c023ac7d189dd9..ef982c78fe64ceef3c7c378fd22d76604bf81c31 100644 > --- a/gdb/aarch64-tdep.c > +++ b/gdb/aarch64-tdep.c > @@ -1382,7 +1382,7 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function, > struct aarch64_call_info info; > struct type *func_type; > struct type *return_type; > - int lang_struct_return; > + int lang_struct_return = 0; > > memset (&info, 0, sizeof (info)); > > @@ -1424,7 +1424,8 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function, > given an additional initial argument, a hidden pointer to the > return slot in memory. */ > return_type = TYPE_TARGET_TYPE (func_type); > - lang_struct_return = language_pass_by_reference (return_type); > + if (return_type != nullptr) > + lang_struct_return = language_pass_by_reference (return_type); > > /* Set the return address. For the AArch64, the return breakpoint > is always at BP_ADDR. */ >
On Thu, Mar 1, 2018 at 5:03 PM, Alan Hayward <Alan.Hayward@arm.com> wrote: > 2018-03-01 Alan Hayward <alan.hayward@arm.com> > > * aarch64-tdep.c (aarch64_push_dummy_call): Check for null > return_type. Add "PR gdb/22736" in ChangeLog entry. Any idea on why TYPE_TARGET_TYPE (func_type) is NULL? (because there is no strcmp debug info?)
> On 2 Mar 2018, at 03:32, Joel Brobecker <brobecker@adacore.com> wrote: > > On Thu, Mar 01, 2018 at 05:03:44PM +0000, Alan Hayward wrote: >> On aarch64, the (int) casting in the following causes a gdb segfault: >> $ ./gdb ./gdb >> (gdb) b dwarf2_physname if (int)strcmp (name, "another_thread_local") == 0 >> (gdb) run a.out // use any a.out >> >> This is due to getting a null pointer from TYPE_TARGET_TYPE, and then >> using it for language_pass_by_reference(). >> >> Fixed by adding a null check, similar to other occurrences in gdb. >> >> Tested on aarch64 with make check using unix, native_gdbserver. >> >> Alan. >> >> >> 2018-03-01 Alan Hayward <alan.hayward@arm.com> >> >> * aarch64-tdep.c (aarch64_push_dummy_call): Check for null >> return_type. > > The patch looks good to me, but do you think you could add a test > for it? Intuitively, I think this should be fairly easily doable, > but can you confirm? Agreed, should be easy enough. I’ve not added anything to the .exp files yet, so this is a good excuse for me to look into them a bit more :) Thanks for the review. > On 2 Mar 2018, at 10:07, Yao Qi <qiyaoltc@gmail.com> wrote: > > On Thu, Mar 1, 2018 at 5:03 PM, Alan Hayward <Alan.Hayward@arm.com> wrote: >> 2018-03-01 Alan Hayward <alan.hayward@arm.com> >> >> * aarch64-tdep.c (aarch64_push_dummy_call): Check for null >> return_type. > > Add "PR gdb/22736" in ChangeLog entry. > Will add. > Any idea on why TYPE_TARGET_TYPE (func_type) is NULL? (because > there is no strcmp debug info?) > The cast to (int) is causing this - remove the cast and it finds the type. I’m assuming that’s causing it to drop the debug info. Also, thanks for the review. Alan.
On Fri, Mar 2, 2018 at 12:09 PM, Alan Hayward <Alan.Hayward@arm.com> wrote: >> Any idea on why TYPE_TARGET_TYPE (func_type) is NULL? (because >> there is no strcmp debug info?) >> > > The cast to (int) is causing this - remove the cast and it finds the type. > I’m assuming that’s causing it to drop the debug info. It sounds like a bug to me. If this bug "TYPE_TARGET_TYPE (func_type) becomes NULL caused by cast" is fixed, the GDB segfault will go away accordingly. To be clear, your patch here is fine to me. My suggestion is that we'd better dig it deeper.
> On 2 Mar 2018, at 12:21, Yao Qi <qiyaoltc@gmail.com> wrote: > > On Fri, Mar 2, 2018 at 12:09 PM, Alan Hayward <Alan.Hayward@arm.com> wrote: >>> Any idea on why TYPE_TARGET_TYPE (func_type) is NULL? (because >>> there is no strcmp debug info?) >>> >> >> The cast to (int) is causing this - remove the cast and it finds the type. >> I’m assuming that’s causing it to drop the debug info. > > It sounds like a bug to me. If this bug "TYPE_TARGET_TYPE (func_type) becomes > NULL caused by cast" is fixed, the GDB segfault will go away accordingly. > > To be clear, your patch here is fine to me. My suggestion is that we'd better > dig it deeper. > Agreed. I’ll raise a new bug, and have a look into it too whilst I’m in the area. Alan.
> >> The cast to (int) is causing this - remove the cast and it finds the type. > >> I’m assuming that’s causing it to drop the debug info. > > > > It sounds like a bug to me. If this bug "TYPE_TARGET_TYPE > > (func_type) becomes NULL caused by cast" is fixed, the GDB segfault > > will go away accordingly. > > > > To be clear, your patch here is fine to me. My suggestion is that > > we'd better dig it deeper. > > Agreed. I’ll raise a new bug, and have a look into it too whilst I’m > in the area. Having read what Yao said, and looking at the example you gave, I'm now thinking that one could very well be the cause of the other; it seems like the cast might indeed be returning a value with a struct type that's missing the return type. Even a subprogram which returns nothing has a TYPE_TARGET_TYPE set to a TYPE_CODE_VOID, right?
> On 2 Mar 2018, at 15:18, Joel Brobecker <brobecker@adacore.com> wrote: > >>>> The cast to (int) is causing this - remove the cast and it finds the type. >>>> I’m assuming that’s causing it to drop the debug info. >>> >>> It sounds like a bug to me. If this bug "TYPE_TARGET_TYPE >>> (func_type) becomes NULL caused by cast" is fixed, the GDB segfault >>> will go away accordingly. >>> >>> To be clear, your patch here is fine to me. My suggestion is that >>> we'd better dig it deeper. >> >> Agreed. I’ll raise a new bug, and have a look into it too whilst I’m >> in the area. > > Having read what Yao said, and looking at the example you gave, > I'm now thinking that one could very well be the cause of the other; > it seems like the cast might indeed be returning a value with > a struct type that's missing the return type. Even a subprogram > which returns nothing has a TYPE_TARGET_TYPE set to a TYPE_CODE_VOID, > right? Been debugging this a little more (and learnt quite a few things about gdb along the way!) Not sure if this reply is the best place to discuss, or if it should go onto the bug. But…. On x86, for strcmp the type and it's target types, as received in amd64_push_dummy_call are: TYPE_CODE_FUNC -> TYPE_CODE_INT -> 0x0 Whereas on aarch64 in aarch64_push_dummy_call we get: TYPE_CODE_FUNC -> 0x0 It turns out this occurs because strcmp has IFUNC vs FUNC differences: X86: $ readelf -s /lib/x86_64-linux-gnu/libc-2.19.so | grep strcmp 2085: 0000000000087380 60 IFUNC GLOBAL DEFAULT 12 strcmp@@GLIBC_2.2.5 Aarch64: $ readelf -s /lib/aarch64-linux-gnu/libc-2.23.so | grep strcmp 2043: 0000000000076380 164 FUNC GLOBAL DEFAULT 12 strcmp@@GLIBC_2.17 I decided to test this on X86 using a FUNC…. $ readelf -s /lib/x86_64-linux-gnu/libc-2.19.so | grep strlen 769: 0000000000088dd0 436 FUNC GLOBAL DEFAULT 12 strlen@@GLIBC_2.2.5 Changed my test to do: (gdb) b foo if (int)strlen(name) == 3 ...Now the type received by the target code is TYPE_CODE_FUNC -> 0, the same as aarch64. However, x86 does not segfault. This is because amd64_push_dummy_call doesn’t bother to check the function parameter. Looking into the code, when reading a FUNC, readlf.c : elf_symtab_read() hits: if (sym->flags & BSF_GNU_INDIRECT_FUNCTION) ms_type = mst_text_gnu_ifunc; else ms_type = mst_text; Setting the type to mst_text, which eventually causes find_minsym_type_and_address() to set the type to objfile_type (objfile)->nodebug_text_symbol Whereas an IFUNC gets set to objfile_type (objfile)->nodebug_text_gnu_ifunc_symbol. So, I think probably either: * Everything is correct * mst_text needs handling differently * FUNCs need a new minimal symbol type (mst_text_gnu_func?) (I’m not familiar enough with this part of the code to give a definitive answer). Alan.
On 03/05/2018 03:57 PM, Alan Hayward wrote: > > >> On 2 Mar 2018, at 15:18, Joel Brobecker <brobecker@adacore.com> wrote: >> >>>>> The cast to (int) is causing this - remove the cast and it finds the type. >>>>> I’m assuming that’s causing it to drop the debug info. >>>> >>>> It sounds like a bug to me. If this bug "TYPE_TARGET_TYPE >>>> (func_type) becomes NULL caused by cast" is fixed, the GDB segfault >>>> will go away accordingly. >>>> >>>> To be clear, your patch here is fine to me. My suggestion is that >>>> we'd better dig it deeper. >>> >>> Agreed. I’ll raise a new bug, and have a look into it too whilst I’m >>> in the area. >> >> Having read what Yao said, and looking at the example you gave, >> I'm now thinking that one could very well be the cause of the other; >> it seems like the cast might indeed be returning a value with >> a struct type that's missing the return type. Even a subprogram >> which returns nothing has a TYPE_TARGET_TYPE set to a TYPE_CODE_VOID, >> right? > > Been debugging this a little more (and learnt quite a few things about gdb along the way!) > Not sure if this reply is the best place to discuss, or if it should go onto the bug. > But…. > > On x86, for strcmp the type and it's target types, as received in amd64_push_dummy_call are: > TYPE_CODE_FUNC -> TYPE_CODE_INT -> 0x0 > Whereas on aarch64 in aarch64_push_dummy_call we get: > TYPE_CODE_FUNC -> 0x0 > > It turns out this occurs because strcmp has IFUNC vs FUNC differences: > > X86: > $ readelf -s /lib/x86_64-linux-gnu/libc-2.19.so | grep strcmp > 2085: 0000000000087380 60 IFUNC GLOBAL DEFAULT 12 strcmp@@GLIBC_2.2.5 > > Aarch64: > $ readelf -s /lib/aarch64-linux-gnu/libc-2.23.so | grep strcmp > 2043: 0000000000076380 164 FUNC GLOBAL DEFAULT 12 strcmp@@GLIBC_2.17 > > > I decided to test this on X86 using a FUNC…. > > $ readelf -s /lib/x86_64-linux-gnu/libc-2.19.so | grep strlen > 769: 0000000000088dd0 436 FUNC GLOBAL DEFAULT 12 strlen@@GLIBC_2.2.5 > > Changed my test to do: > (gdb) b foo if (int)strlen(name) == 3 > > ...Now the type received by the target code is TYPE_CODE_FUNC -> 0, the same as aarch64. > > However, x86 does not segfault. This is because amd64_push_dummy_call doesn’t bother to > check the function parameter. > > > Looking into the code, when reading a FUNC, readlf.c : elf_symtab_read() hits: > > if (sym->flags & BSF_GNU_INDIRECT_FUNCTION) > ms_type = mst_text_gnu_ifunc; > else > ms_type = mst_text; > > Setting the type to mst_text, which eventually causes find_minsym_type_and_address() to > set the type to objfile_type (objfile)->nodebug_text_symbol > Whereas an IFUNC gets set to objfile_type (objfile)->nodebug_text_gnu_ifunc_symbol. > > So, I think probably either: > * Everything is correct > * mst_text needs handling differently > * FUNCs need a new minimal symbol type (mst_text_gnu_func?) > (I’m not familiar enough with this part of the code to give a definitive answer). Funny enough, I started working on fixing ifunc-related problems early last week, but then got sidetracked into other high priority things. I've not run into a GDB crash, but just in case, maybe this branch helps: https://github.com/palves/gdb/commits/palves/ifunc The gnu-ifunc.exp test doesn't pass cleanly on that branch because I'm midway through augmenting that testcase to cover different scenarios and evaluating whether it's the test that needs fixing, or gdb. Thanks, Pedro Alves
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index f08945ea07101e1cd7906ca640c023ac7d189dd9..ef982c78fe64ceef3c7c378fd22d76604bf81c31 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -1382,7 +1382,7 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct aarch64_call_info info; struct type *func_type; struct type *return_type; - int lang_struct_return; + int lang_struct_return = 0; memset (&info, 0, sizeof (info)); @@ -1424,7 +1424,8 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function, given an additional initial argument, a hidden pointer to the return slot in memory. */ return_type = TYPE_TARGET_TYPE (func_type); - lang_struct_return = language_pass_by_reference (return_type); + if (return_type != nullptr) + lang_struct_return = language_pass_by_reference (return_type); /* Set the return address. For the AArch64, the return breakpoint is always at BP_ADDR. */