From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 72669 invoked by alias); 24 Nov 2017 19:25:20 -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 72655 invoked by uid 89); 24 Nov 2017 19:25:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=BAYES_00,KB_WAM_FROM_NAME_SINGLEWORD,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=*all*, nasty 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; Fri, 24 Nov 2017 19:25:17 +0000 Received: from pps.filterd (m0098419.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id vAOJOItq058762 for ; Fri, 24 Nov 2017 14:25:16 -0500 Received: from e06smtp15.uk.ibm.com (e06smtp15.uk.ibm.com [195.75.94.111]) by mx0b-001b2d01.pphosted.com with ESMTP id 2eeq485h3r-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 24 Nov 2017 14:25:15 -0500 Received: from localhost by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 24 Nov 2017 19:25:14 -0000 Received: from b06cxnps4074.portsmouth.uk.ibm.com (9.149.109.196) by e06smtp15.uk.ibm.com (192.168.101.145) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Fri, 24 Nov 2017 19:25:12 -0000 Received: from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com [9.149.105.62]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id vAOJPCFs44105824; Fri, 24 Nov 2017 19:25:12 GMT Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E21EEAE04D; Fri, 24 Nov 2017 19:18:24 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id C73CFAE045; Fri, 24 Nov 2017 19:18:24 +0000 (GMT) Received: from oc3748833570.ibm.com (unknown [9.164.187.199]) by d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP; Fri, 24 Nov 2017 19:18:24 +0000 (GMT) Received: by oc3748833570.ibm.com (Postfix, from userid 1000) id A2AE3D80147; Fri, 24 Nov 2017 20:25:11 +0100 (CET) Subject: Re: [PATCH 1/4] Split up s390-linux-tdep.c into two files To: prudo@linux.vnet.ibm.com (Philipp Rudo) Date: Fri, 24 Nov 2017 19:25:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, arnez@linux.vnet.ibm.com (Andreas Arnez) In-Reply-To: <20171124175511.33626ff8@ThinkPad> from "Philipp Rudo" at Nov 24, 2017 05:55:11 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 17112419-0020-0000-0000-000003D04982 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17112419-0021-0000-0000-00004265A535 Message-Id: <20171124192511.A2AE3D80147@oc3748833570.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-11-24_07:,, 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=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1711240261 X-SW-Source: 2017-11/txt/msg00622.txt.bz2 Hi Phillip, > I already feared you will find something. The short answer to most of your > comments is that the split isn't a 100% clean. The new s390-tdep isn't pure > architecture code but mixed with common Linux ELF abi code. If you wanted > it clean we had to split up s390-tdep even further. But I don't know is if > it is worth the work. So in general, for all platforms where there is split between ARCH-tdep.c and ARCH-linux-tdep.c, the former contains code that is applicable to all targets on that architecture, while the latter contains code that is used only on Linux targets. We should really do the same on s390. Now, it is true that in many cases the split isn't 100% clean. For example, on many platforms most OSes use mostly the same calling convention, but with maybe some minor changes. Then, it makes sense to have to bulk of the calling convention stuff in ARCH-tdep.c, but have it check some ABI variant flag in tdep to modify its behavior. So your moving all those parts to s390-tdep.c is perfectly fine with me. However, s390-tdep.c should *not* contain anything that would *break* running GDB on some non-Linux OS (I'm e.g. thinking of BSD or OpenSolaris here, both of which were actually ported to s390 by some people). That's why e.g. moving the orig_r2 stuff to s390-tdep.c is not a good idea. Now, your use case is special. We currently do not support recognizing the Linux kernel as a separate target, different from Linux user-space programs, on *any* architecture. It is well possible that we'll have to add something here. But that is not a s390-specific problem; we'll want to have that capability on *all* Linux architectures in the end. So creating some non-standard s390-tdep.c / s390-linux-tdep.c split just for that reason isn't useful since it wouldn't help other platforms. Whether the Linux kernel target should use (or not use) the ARCH-linux-tdep.c file is an interesting question, but it should be answered the same way on all architectures. > > Not sure I understand your split here. "ORIG_R2" is not a real register > > and only exists on Linux, so it shouldn't move to s390-tdep.c. On the > > other hand, the split between upper halves and lower halves seems more > > generic and could happen elsewhere ... > > This is a little more complicated. > > For the upper/lower split you are right, it is architecture. However i see > it as legacy code and I don't plan to add any support for 31-bit systems on > 64-bit machines. So i decided to leave the definition where it is used. > > For orig_r2 you are right (again), it only exist in user space. However > (again) the kernel uses the same prstatus definitions like the user space, > including a field for orig_r2 (although it is not used). Thats why i moved it > to the common 'Linux ELF abi' s390-tdep. Thinking about this again, all the regsets should probably remain in s390-linux-tdep.c. This stuff is usually different between OSes anyway. For the kernel structure, you can provide your own regset that simply ignores the orig_r2 field in the structure. > > This method is same across all Linux (and really all) targets; there is > > no point for us to try to do it differently, that would just cause confusion. > > I played around with osabi and never got it to work the way I wanted it to. > My problem was that ELF doesn't distinguish the kernel and user space. So the > default ELF sniffer always used the GDB_OSABI_LINUX and never gave my > GDB_OSABI_LINUX_KERNEL a chance. I cannot recall all details and will give it > a second chance. Please give me some time. As said above, this is really a different problem. The first issue is that on s390, we should use the same split between ARCH-tdep.c and ARCH-linux-tdep.c as everybody else, and that means using an OSABI sniffer. The second issue is that we somehow need to distinguish between Linux kernel debugging and Linux user-space debugging. I'm not sure what the best way to do this is; we could have a separate GDB_OSABI_LINUX_KERNEL as you mention and would then have to adapt the (common code) sniffer routines. Or maybe both kernel and user space should use GDB_OSABI_LINUX and then the Linux ABI routine just handles them differently? Hard to say for me at the moment (probably because I haven't seen the actual kernel code yet). > > The Linux specific feature should be handled here, the others in the > > generic part. > > My plan was to take care of this when we change to dynamically creating > the tdesc. Then we could number the registers directly when creating the tdesc > (at least I hope so). To be honest I haven't done it yet as with all those > have_* variables there are some nasty dependencies in the function... It shouldn't be really difficult to split those up. Again, e.g. ppc does the same thing already today. All the have_ flags can stay in the common tdep struct, which makes this easier. Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain Ulrich.Weigand@de.ibm.com