From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 59099 invoked by alias); 7 Dec 2016 16:37:06 -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 58858 invoked by uid 89); 7 Dec 2016 16:37:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Richards, richards, henderson, Henderson 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; Wed, 07 Dec 2016 16:36:54 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0E1023B716; Wed, 7 Dec 2016 16:36:53 +0000 (UTC) Received: from [127.0.0.1] (ovpn03.gateway.prod.ext.phx2.redhat.com [10.5.9.3]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uB7GapnQ023837; Wed, 7 Dec 2016 11:36:51 -0500 Subject: Re: [PATCH 2/2] amd64-linux: expose system register FS_BASE and GS_BASE for Linux. To: "Tedeschi, Walfred" , eliz@gnu.org, brobecker@adacore.com References: <1478166445-21370-1-git-send-email-walfred.tedeschi@intel.com> <1478166445-21370-3-git-send-email-walfred.tedeschi@intel.com> <4278085b-7272-47b5-047a-fd9f08a843dd@redhat.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: Date: Wed, 07 Dec 2016 16:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-12/txt/msg00190.txt.bz2 On 11/30/2016 10:29 AM, Tedeschi, Walfred wrote: > On 11/23/2016 01:01 PM, Pedro Alves wrote: >> On 11/03/2016 09:47 AM, Walfred Tedeschi wrote: >>> This patch allows examination of the registers FS_BASE and GS_BASE >>> for Linux Systems running on 64bit. Tests for simple read and write >>> of the new registers is also added with this patch. >>> >>> Tests were performed on: >>> x86_64 RHEL 5.3 - For the older ptrace calls. >>> x86_64 Ubuntu 16.10 - For the new ptrace calls. >>> >>> 2016-04-26 Walfred Tedeschi >>> Richard Henderson >> What changed compared to Richard's original version? > I have added a test, gdbserver support was also added by me. >>> >>> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c >>> index 3f2a92b..a8a0b79 100644 >>> --- a/gdb/amd64-linux-tdep.c >>> +++ b/gdb/amd64-linux-tdep.c >>> @@ -103,7 +103,14 @@ int amd64_linux_gregset_reg_offset[] = >>> -1, -1, -1, -1, -1, -1, -1, -1, >>> -1, -1, -1, -1, -1, -1, -1, -1, >>> -1, -1, -1, -1, -1, -1, -1, -1, >>> + /* System register added at the end. */ >>> +#ifdef HAVE_STRUCT_USER_REGS_STRUCT_FS_BASE >>> + 21 * 8, 22 * 8, /* fs_base and gs_base. */ >>> +#else >>> + -1, -1, /* fs_base and gs_base. */ >>> +#endif >>> 15 * 8 /* "orig_rax" */ >>> + >> Spurious new line? > Yes, fixed for the next version. >> >> How's this meant to work for cross debugging, and core debugging? > I have used the loopback board to test the remote scenario, but it can > be that this is not enough. Right, that doesn't really exercise cross debugging, because in that case, both gdb and gdbserver are running on the same architecture. > You have just pointed out one problem with this setup. > > Core debugging was working automatically, as far as i could see. Core > testes passed and have reported the FS_base register and GS_base registers. > gcore tests report fs_base and gs_base registers while reading the > registers from the core file. That's another manifestation of the same testing environment issue -- what if you load the x86 core on an ARM host? Then HAVE_STRUCT_USER_REGS_STRUCT_FS_BASE won't be defined, while the data is available. Also what happens if you load a core dump generated on an old kernel, into a gdb running/built on a new kernel. And likewise the other way around (dump on new kernel, load on gdb built on old kernel). This is the sort of host-independence concerns that the core reading code should be having. > > For remote: I tested on loop back scenario, it also show good results. > > Not sure if i have answered your questions. Not exactly. > >> I don't think it makes sense to put a host-specific >> "#ifdef HAVE_STRUCT_USER_REGS_STRUCT_FS_BASE" check in a tdep file. > Agreed! I am using values that take into account the newest kernel. > On the other hand for older kernels there is the need to return -1 on > the native file, like the snippet below: See above. > > diff --git a/gdb/amd64-nat.c b/gdb/amd64-nat.c > index ad5df57..e929735 100644 > --- a/gdb/amd64-nat.c > +++ b/gdb/amd64-nat.c > @@ -65,6 +65,11 @@ amd64_native_gregset_reg_offset (struct gdbarch > *gdbarch, int regnum) > if (num_regs > gdbarch_num_regs (gdbarch)) > num_regs = gdbarch_num_regs (gdbarch); > > +#ifndef HAVE_STRUCT_USER_REGS_STRUCT_FS_BASE > + if (regnum == AMD64_FSBASE_REGNUM || regnum == AMD64_GSBASE_REGNUM) > + return -1; > +#endif > > would this be ok? I don't know exactly what this change means. Is HAVE_STRUCT_USER_REGS_STRUCT_FS_BASE really about kernel version, or libc version? E.g., what do older kernels put in those gregset offsets? >>> +++ b/gdb/features/i386/64bit-segments.xml >>> @@ -0,0 +1,12 @@ >>> + >>> + >>> + >>> + >>> + >>> + >>> + >> #1 - Why is "regnum" hard coded? > Removed that, thanks. >> >> #2 - Is bitsize 64 and type "int" really correct? > I suppose you saw something wrong here, that i missed. Well, I'm seeing "int" and thinking "32-bit", since that's what it usually is, but then we have bitsize 64-bit. > My reaoning was the following: I used for gs_base and fs_base the same > type used for orig_rax as they also have the same type the user_reg_struct. Funny, that's the only other case like this: $ grep -rn "type=\"int\"" | grep \"64\" i386/64bit-linux.xml:10: Looks like gdb may be being smart about this: (gdb) ptype $orig_rax type = long Is that the type you get back for the new registers too? >>> --- a/gdb/gdbserver/linux-x86-low.c >>> +++ b/gdb/gdbserver/linux-x86-low.c >>> @@ -133,6 +133,11 @@ static const int x86_64_regmap[] = >>> -1, >>> -1, -1, -1, -1, -1, -1, -1, -1, >>> ORIG_RAX * 8, >>> +#ifdef HAVE_STRUCT_USER_REGS_STRUCT_FS_BASE >>> + 21 * 8, 22 * 8, >>> +#else >>> + -1, -1, >>> +#endif >>> -1, -1, -1, -1, /* MPX registers BND0 ... BND3. */ >> It's curious that above this was put after orig_rax, while >> here: > > This is due to the fact that we usually let orig_rax at last. > In the gdb side we fix it by means of the macros. I'm afraid you lost me. What macros? Thanks, Pedro Alves