From patchwork Wed Mar 7 11:10:47 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 26224 Received: (qmail 23422 invoked by alias); 7 Mar 2018 11:10:57 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 23405 invoked by uid 89); 7 Mar 2018 11:10:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LOTSOFHASH, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=10707, Rather, midway, gnuifuncexp X-HELO: EUR02-AM5-obe.outbound.protection.outlook.com Received: from mail-eopbgr00087.outbound.protection.outlook.com (HELO EUR02-AM5-obe.outbound.protection.outlook.com) (40.107.0.87) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 07 Mar 2018 11:10:51 +0000 Received: from AM3PR08MB0101.eurprd08.prod.outlook.com (10.160.211.19) by AM3PR08MB0101.eurprd08.prod.outlook.com (10.160.211.19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.548.13; Wed, 7 Mar 2018 11:10:48 +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.0548.016; Wed, 7 Mar 2018 11:10:48 +0000 From: Alan Hayward To: Pedro Alves CC: Joel Brobecker , Yao Qi , "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type Date: Wed, 7 Mar 2018 11:10:47 +0000 Message-ID: References: <20180302033204.v2wvjmquwy3dswyk@adacore.com> <20180302151824.dg4y23pwjmm6nqjb@adacore.com> <18C9D0DE-F18B-4F88-91F3-826208369A64@arm.com> <31749295-0d7e-11ab-8e13-e25a070c6595@redhat.com> In-Reply-To: <31749295-0d7e-11ab-8e13-e25a070c6595@redhat.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM3PR08MB0101; 7:YDEiNoshf2BzI5IReI+mgDvgfB6cqH/N7xfbO9gX47BSIdyj0CRGMX2fWAM8cOJQsGY7qJwCi9+PpRAznhUTHwSs1+Hlf8WPpbswIXYf2ch9HnuyNZyCvMdGFPrxEAeIkKNXVZVL7Ide8g8nXKfiD21TobWrlqotnONnEZVpCn1rcc7FFrU5IURtemZvG9hjJ2BEwG/jEu2YEc8W70y2gmCDO6JoySrgFmQCeDjBCi6e5fT9oVsLXmpJuFiT/XHW x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 88d9ed02-cb10-4fbf-4dcc-08d5841c147b x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(48565401081)(5600026)(4604075)(3008032)(4534165)(4627221)(201703031133081)(201702281549075)(2017052603328)(7153060)(7193020); SRVR:AM3PR08MB0101; x-ms-traffictypediagnostic: AM3PR08MB0101: nodisclaimer: True x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(180628864354917)(166708455590820); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040501)(2401047)(8121501046)(5005006)(3002001)(93006095)(93001095)(3231220)(944501244)(52105095)(10201501046)(6055026)(6041288)(20161123558120)(20161123560045)(20161123562045)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(6072148)(201708071742011); SRVR:AM3PR08MB0101; BCL:0; PCL:0; RULEID:; SRVR:AM3PR08MB0101; x-forefront-prvs: 0604AFA86B x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(39860400002)(376002)(346002)(39380400002)(396003)(377424004)(189003)(199004)(51914003)(72206003)(66066001)(2906002)(7736002)(305945005)(6436002)(6486002)(36756003)(106356001)(3660700001)(6916009)(575784001)(86362001)(2950100002)(25786009)(97736004)(229853002)(54906003)(5250100002)(99286004)(68736007)(2900100001)(105586002)(39060400002)(4326008)(83716003)(8936002)(76176011)(102836004)(6506007)(59450400001)(26005)(8676002)(81166006)(81156014)(186003)(53546011)(14454004)(93886005)(82746002)(966005)(478600001)(53936002)(316002)(6116002)(6246003)(6512007)(3846002)(5660300001)(6306002)(3280700002)(33656002); DIR:OUT; SFP:1101; SCL:1; SRVR:AM3PR08MB0101; H:AM3PR08MB0101.eurprd08.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: HYeuVKY1aHH/cf4RWPGhJmw78j/4HtztFn/YO7yy++SEDskVKZ+iXrneaMFTrSKYoO47T8B453kG1HbC9bqZaD66AyZl4f1tWfEc9/pY+upwZr6yg9PJtWVPw+7bGmB2/cTuiUrbf/1OC7JxFn4Q/A9qhCECDwy46x5KjuAD1wwYDdWINjLoR5iC9vHQwTBrrHtVC5Qk1mcfBPhE2xtG4E7nPADR5pX/Yp6SrqjFMoYMxyhWtiazagxQRNYipyf873Hpegaz2ymfavhYrfzddPTflEobk17EboW2UjUKAKvxe7d0USVaTWkwf9qiI85t5ct+uTLdulkCOz+f0dI41Q== spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-ID: MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: 88d9ed02-cb10-4fbf-4dcc-08d5841c147b X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Mar 2018 11:10:47.8962 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM3PR08MB0101 X-IsSubscribed: yes > On 5 Mar 2018, at 16:45, Pedro Alves wrote: > > On 03/05/2018 03:57 PM, Alan Hayward wrote: >> >> >>> On 2 Mar 2018, at 15:18, Joel Brobecker 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 for the branch, gave that it a try, but it has the same error. Digging a little more into this…. The target type for the *function is set to the type of the function pointer, not the return type. So, for IFUNC, TYPE_CODE_FUNC target type is TYPE_CODE_INT which gives the type of the function pointer. For the FUNC, there is no pointer, so the target type of TYPE_CODE_FUNC is set to null. That makes sense to me, but isn’t immediately obvious. In call_function_by_hand_dummy() there already exists code to extract the type of the function return, which is set as target_values_type. Patch below simply passes this through to the gdbarch_push_dummy_call method. Updated aarch64 and arm targets to use this. If this patch is ok for amd64/aarch64/arm, then I will post again with _push_dummy_call() fixed on every target. (Not done that yet because it’s a lot of tedious copy pasting). Most targets don’t even look at the return types, so I’m not expecting many issues when I look at them. Tested on aarch64 only with make check unitest and native_gdbserver. 2018-03-06 Alan Hayward PR gdb/22736 * aarch64-tdep.c (aarch64_push_dummy_call): Use passed in return type. * amd64-tdep.c (amd64_push_dummy_call): Add new arg. * arm-tdep.c (arm_push_dummy_call): Use passed in return type. * gdbarch.sh (gdbarch_push_dummy_call): Add return type. * gdbarch.h: Regenerate. * gdbarch.sh: Likewise. * infcall.c (call_function_by_hand_dummy): Pass down return type. diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index f08945ea07101e1cd7906ca640c023ac7d189dd9..bf9ae4475f80a8200eeb56a8edf4a9f7ee3daa37 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -1376,13 +1376,12 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, - CORE_ADDR struct_addr) + CORE_ADDR struct_addr, struct type *return_type) { int argnum; 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)); @@ -1411,20 +1410,11 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function, Rather that change the target interface we call the language code directly ourselves. */ - func_type = check_typedef (value_type (function)); - - /* Dereference function pointer types. */ - if (TYPE_CODE (func_type) == TYPE_CODE_PTR) - func_type = TYPE_TARGET_TYPE (func_type); - - gdb_assert (TYPE_CODE (func_type) == TYPE_CODE_FUNC - || TYPE_CODE (func_type) == TYPE_CODE_METHOD); - /* If language_pass_by_reference () returned true we will have been 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. */ diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index 6b92c9244c627af5fea78fdfd97b41a887fb679a..c50f611846c32364f31c33e0b4092af881fa6248 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -990,7 +990,8 @@ static CORE_ADDR amd64_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, - int struct_return, CORE_ADDR struct_addr) + int struct_return, CORE_ADDR struct_addr, + struct type *return_type) { enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); gdb_byte buf[8]; diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index ef7e66b36afe34aed3880b86d16b466984481131..78240d21ef1082bfdaaafa6be6c2f5a73ac819a6 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -3689,7 +3689,7 @@ static CORE_ADDR arm_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, - CORE_ADDR struct_addr) + CORE_ADDR struct_addr, struct type *return_type) { enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); int argnum; @@ -3700,12 +3700,8 @@ arm_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct type *ftype; unsigned vfp_regs_free = (1 << 16) - 1; - /* Determine the type of this function and whether the VFP ABI - applies. */ - ftype = check_typedef (value_type (function)); - if (TYPE_CODE (ftype) == TYPE_CODE_PTR) - ftype = check_typedef (TYPE_TARGET_TYPE (ftype)); - use_vfp_abi = arm_vfp_abi_for_function (gdbarch, ftype); + /* Determine whether the VFP ABI applies. */ + use_vfp_abi = arm_vfp_abi_for_function (gdbarch, return_type); /* Set the return address. For the ARM, the return breakpoint is always at BP_ADDR. */ diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h index 5cb131de1d27209107b5b83773ea7560ef0da6ac..086a230ffe6d295e1d1d1c3b5e3391ec979444eb 100644 --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -397,8 +397,8 @@ extern void set_gdbarch_deprecated_fp_regnum (struct gdbarch *gdbarch, int depre extern int gdbarch_push_dummy_call_p (struct gdbarch *gdbarch); -typedef CORE_ADDR (gdbarch_push_dummy_call_ftype) (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr); -extern CORE_ADDR gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr); +typedef CORE_ADDR (gdbarch_push_dummy_call_ftype) (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr, struct type *return_type); +extern CORE_ADDR gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr, struct type *return_type); extern void set_gdbarch_push_dummy_call (struct gdbarch *gdbarch, gdbarch_push_dummy_call_ftype *push_dummy_call); extern int gdbarch_call_dummy_location (struct gdbarch *gdbarch); diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index b8703e5a556e310e023cadf57037918d1bdd2850..50dd72db5d5a06a841391290c1afac345af49eb9 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -2368,13 +2368,13 @@ gdbarch_push_dummy_call_p (struct gdbarch *gdbarch) } CORE_ADDR -gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr) +gdbarch_push_dummy_call (struct gdbarch *gdbarch, struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr, struct type *return_type) { gdb_assert (gdbarch != NULL); gdb_assert (gdbarch->push_dummy_call != NULL); if (gdbarch_debug >= 2) fprintf_unfiltered (gdb_stdlog, "gdbarch_push_dummy_call called\n"); - return gdbarch->push_dummy_call (gdbarch, function, regcache, bp_addr, nargs, args, sp, struct_return, struct_addr); + return gdbarch->push_dummy_call (gdbarch, function, regcache, bp_addr, nargs, args, sp, struct_return, struct_addr, return_type); } void diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh index 33dfa6b349dee778f1a82a511a6cf57960d48f89..a1472069aa3d3a3bd355a772a4f9527f413330ca 100755 --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -488,7 +488,7 @@ M;struct frame_id;dummy_id;struct frame_info *this_frame;this_frame # deprecated_fp_regnum. v;int;deprecated_fp_regnum;;;-1;-1;;0 -M;CORE_ADDR;push_dummy_call;struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, struct_return, struct_addr +M;CORE_ADDR;push_dummy_call;struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr, struct type *return_type;function, regcache, bp_addr, nargs, args, sp, struct_return, struct_addr, return_type v;int;call_dummy_location;;;;AT_ENTRY_POINT;;0 M;CORE_ADDR;push_dummy_code;CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache;sp, funaddr, args, nargs, value_type, real_pc, bp_addr, regcache diff --git a/gdb/infcall.c b/gdb/infcall.c index b7f4a176db581c15c4fdd8c5299aab35d0fe4a68..773f43e884b3d831e0bd8254998cbb7ecb8daf33 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -1070,7 +1070,8 @@ call_function_by_hand_dummy (struct value *function, return address should be pointed. */ sp = gdbarch_push_dummy_call (gdbarch, function, get_current_regcache (), bp_addr, nargs, args, - sp, struct_return, struct_addr); + sp, struct_return, struct_addr, + target_values_type); /* Set up a frame ID for the dummy frame so we can pass it to set_momentary_breakpoint. We need to give the breakpoint a frame