From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 72030 invoked by alias); 9 Oct 2018 16:10:19 -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 72021 invoked by uid 89); 9 Oct 2018 16:10:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 09 Oct 2018 16:10:17 +0000 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 68527C049E3A; Tue, 9 Oct 2018 16:10:16 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8EE83194B7; Tue, 9 Oct 2018 16:10:15 +0000 (UTC) Subject: Re: [PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls To: Alan Hayward , gdb-patches@sourceware.org References: <20181001155255.14859-1-alan.hayward@arm.com> Cc: nd@arm.com From: Pedro Alves Message-ID: <57873989-fc65-634f-c6f8-8c2a976e4f9f@redhat.com> Date: Tue, 09 Oct 2018 16:10:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181001155255.14859-1-alan.hayward@arm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-10/txt/msg00214.txt.bz2 On 10/01/2018 04:52 PM, Alan Hayward wrote: > This is a reworking of a patch I posted in March. > V1 had a long discussion which was then paused to wait for > Pedro's IFUNC rewrite. > > > Prevent the int cast in the following causing a segfault on aarch64: > (gdb) b foo if (int)strcmp(name,"abc") == 0 > (gdb) run > > > This is because to aarch64_push_dummy_call determines the return type > of the function and then does not check for null pointer. > > A null pointer for the return type means either 1) the call has a > cast or 2) an error has occured. I'd think that "1) the call has a cast" is not accurate. If the called function has debug info, then GDB will know it's return type. The issue is that the called function may not have debug information, and then GDB does not know its return type (so its NULL), and then the only way to call the function is to add the cast. Right? It kind of sounds like IFUNCs were a red herring then. :-/ > You can see this in infcall.c:call_function_by_hand_dummy(): > > CORE_ADDR funaddr = find_function_addr (function, &values_type, &ftype); > > if (values_type == NULL) > values_type = default_return_type; > if (values_type == NULL) > { > const char *name = get_function_name (funaddr, > name_buf, sizeof (name_buf)); > error (_("'%s' has unknown return type; " > "cast the call to its declared return type"), > name); > } > > In aarch64_push_dummy_call we do not have default_return_type, so cannot > determine between the two cases. > > (In addition, aarch64_push_dummy_call incorrectly resolves the return > type for IFUNC). Can you expand a bit on this IFUNC remark? > However, aarch64_push_dummy_call only requires the return value in order > to calculate lang_struct_return ... which has previously been calculated > in the caller: > > This is slightly awkward, ideally the flag "lang_struct_return" > would be passed to the targets implementation of push_dummy_call. > Rather that change the target interface we call the language code > directly ourselves. > Ah, nice, the solution was right there. :-) > The fix is simple: > Patch 1: Update gdbarch interface to pass lang_struct_return. > Patch 2: Remove incorrect code and use the passed in lang_struct_return. > Since cover letters don't end up in git, this info should be somehow migrated into the commit logs of the two patches. Thanks, Pedro Alves