From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94006 invoked by alias); 13 Dec 2017 18:42:15 -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 93993 invoked by uid 89); 13 Dec 2017 18:42:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 Dec 2017 18:42:13 +0000 Received: from pps.filterd (m0098413.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id vBDIdXSl042575 for ; Wed, 13 Dec 2017 13:42:12 -0500 Received: from e06smtp12.uk.ibm.com (e06smtp12.uk.ibm.com [195.75.94.108]) by mx0b-001b2d01.pphosted.com with ESMTP id 2eu6k7smkk-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 13 Dec 2017 13:42:11 -0500 Received: from localhost by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 13 Dec 2017 18:42:10 -0000 Received: from b06cxnps4074.portsmouth.uk.ibm.com (9.149.109.196) by e06smtp12.uk.ibm.com (192.168.101.142) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 13 Dec 2017 18:42:08 -0000 Received: from d06av24.portsmouth.uk.ibm.com (mk.ibm.com [9.149.105.60]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id vBDIg7T247317194; Wed, 13 Dec 2017 18:42:07 GMT Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6CA7042045; Wed, 13 Dec 2017 18:36:22 +0000 (GMT) Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5D5214204D; Wed, 13 Dec 2017 18:36:22 +0000 (GMT) Received: from oc3748833570.ibm.com (unknown [9.152.213.29]) by d06av24.portsmouth.uk.ibm.com (Postfix) with ESMTP; Wed, 13 Dec 2017 18:36:22 +0000 (GMT) Received: by oc3748833570.ibm.com (Postfix, from userid 1000) id 454ADD85C63; Wed, 13 Dec 2017 19:42:07 +0100 (CET) Subject: Re: [PATCH v3 07/10] s390: Hook s390 into OSABI mechanism To: prudo@linux.vnet.ibm.com (Philipp Rudo) Date: Wed, 13 Dec 2017 18:42:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, arnez@linux.vnet.ibm.com (Andreas Arnez), qiyaoltc@gmail.com (Yao Qi) In-Reply-To: <20171208110436.30199-8-prudo@linux.vnet.ibm.com> from "Philipp Rudo" at Dec 08, 2017 12:04:33 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 17121318-0008-0000-0000-000004B6C371 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17121318-0009-0000-0000-00001E49D381 Message-Id: <20171213184207.454ADD85C63@oc3748833570.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-12-13_10:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1712130262 X-SW-Source: 2017-12/txt/msg00320.txt.bz2 Philipp Rudo wrote: > + /* The DWARF unwinders must be appended before the ABI is initialized. > + Otherwise it is possible that a ABI default unwinder gets called before > + the DWARF unwinder even gets the chance. */ > + dwarf2_append_unwinders (gdbarch); It would be better to also move the related dwarf2 routines here (see below). > - /* Use default target description if none provided by the target. */ > - if (!tdesc_has_registers (tdesc)) > - { > - if (tdep->abi == ABI_LINUX_S390) > - tdesc = tdesc_s390_linux32; > - else > - tdesc = tdesc_s390x_linux64; > - } > - tdep->tdesc = tdesc; > + gdbarch_init_osabi (info, gdbarch); One (potential) problem with this is that gdbarch_init_osabi is now called very early. This means an OSABI has no way of overriding any of the gdbarch functions that are installed below this point. Usually, most targets try to call gdbarch_init_osabi as late as possible so as to make such overrides possible, e.g. at the place where we now call linux_init_abi. I understand you moved this call so early so that you can get a default tdesc from ABI code if no tdesc is specified. However: - Even so, many of the gdbarch routines do not depend in any way on the selected tdep->abi or tdesc. At least those should preferably still be done before the gdbarch_init_osabi call. - It also might be worthwhile to reconsider whether we even need to use a default (fallback) tdesc at all. The only targets where we do not get a tdesc would be remote connection to a gdbserver that does not send a tdesc (but all gdbservers built in the last 10 years do so ...) or some other older remote stub (e.g. in qemu) that doesn't send a tdesc. Maybe it is not necessary to try to support this any longer. But that is probably a separate discussion, and it isn't necessary for this issue to block this patch set from going in. > set_gdbarch_num_regs (gdbarch, S390_NUM_REGS); > set_gdbarch_sp_regnum (gdbarch, S390_SP_REGNUM); > set_gdbarch_fp0_regnum (gdbarch, S390_F0_REGNUM); > + set_gdbarch_guess_tracepoint_registers (gdbarch, > + s390_guess_tracepoint_registers); > set_gdbarch_stab_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum); > set_gdbarch_dwarf2_reg_to_regnum (gdbarch, s390_dwarf_reg_to_regnum); > set_gdbarch_value_from_register (gdbarch, s390_value_from_register); > - set_gdbarch_core_read_description (gdbarch, s390_core_read_description); > - set_gdbarch_iterate_over_regset_sections (gdbarch, > - s390_iterate_over_regset_sections); > - set_gdbarch_cannot_store_register (gdbarch, s390_cannot_store_register); > - set_gdbarch_write_pc (gdbarch, s390_write_pc); > - set_gdbarch_guess_tracepoint_registers (gdbarch, s390_guess_tracepoint_registers); There's no reason to move that last function around, it seems. > /* Frame handling. */ > dwarf2_frame_set_init_reg (gdbarch, s390_dwarf2_frame_init_reg); > dwarf2_frame_set_adjust_regnum (gdbarch, s390_adjust_frame_regnum); > - dwarf2_append_unwinders (gdbarch); > frame_base_append_sniffer (gdbarch, dwarf2_frame_base_sniffer); All the dwarf2 routines should move up in one block. > - frame_unwind_append_unwinder (gdbarch, &s390_stub_frame_unwind); > - frame_unwind_append_unwinder (gdbarch, &s390_sigtramp_frame_unwind); > - frame_unwind_append_unwinder (gdbarch, &s390_frame_unwind); > - frame_base_set_default (gdbarch, &s390_frame_base); The sigtramp unwinder should move to ABI code, but not the others. I note that you now move them back in patch 09/10, but they shouldn't even be moved out in the first place :-) I've also looked at the other patches in this series, and they look good to me. So unless Andreas has any further comments, I think the series is good to go in once the above comments have been addressed. Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain Ulrich.Weigand@de.ibm.com