From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 80142 invoked by alias); 9 Mar 2018 08:51:28 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 80131 invoked by uid 89); 9 Mar 2018 08:51:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Mar 2018 08:51:26 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 7576756183; Fri, 9 Mar 2018 03:51:24 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 18PjZTYB0pXZ; Fri, 9 Mar 2018 03:51:24 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 3E7171161FB; Fri, 9 Mar 2018 03:51:24 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id A0BD3808D8; Fri, 9 Mar 2018 09:51:22 +0100 (CET) Date: Fri, 09 Mar 2018 08:51:00 -0000 From: Joel Brobecker To: Alan Hayward Cc: Pedro Alves , Yao Qi , "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH PR gdb/22736] [aarch64] gdb crashes on a conditional breakpoint with cast return type Message-ID: <20180309085122.v4fzh4vcii5plkkk@adacore.com> References: <20180302033204.v2wvjmquwy3dswyk@adacore.com> <20180302151824.dg4y23pwjmm6nqjb@adacore.com> <18C9D0DE-F18B-4F88-91F3-826208369A64@arm.com> <31749295-0d7e-11ab-8e13-e25a070c6595@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) X-SW-Source: 2018-03/txt/msg00190.txt.bz2 Hello Alan, > 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. I don't really understand what you mean by that -- maybe it is related to the '*' before 'function' above? If we're talking about the target_type for the function, it should be the type of the return value. At least according to the documentation for that field in gdb_types.h. But maybe you're talking about something else? > 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. Sounds like a good change even if we didn't have this issue, as it avoids a gdbarch method having to re-compute something that was already previously computed. > 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. Just a minor comment below (besides what you already remarked on about updating the remaining targets). Please give Pedro some time to comment; my reviews have not be exactly top notch, lately, and I don't want you to do useless work because I missed something. > @@ -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) I think return_type can never be nullptr (from what I understand of the code in call_function_by_hand_dummy). -- Joel