| Message ID | 1525138292-4526-1-git-send-email-omair.javaid@linaro.org |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 85690 invoked by alias); 1 May 2018 01:32:09 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 85673 invoked by uid 89); 1 May 2018 01:32:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wr0-f194.google.com Received: from mail-wr0-f194.google.com (HELO mail-wr0-f194.google.com) (209.85.128.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 01 May 2018 01:32:07 +0000 Received: by mail-wr0-f194.google.com with SMTP id c14-v6so9658962wrd.4 for <gdb-patches@sourceware.org>; Mon, 30 Apr 2018 18:32:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=GGxXiDXwQHJaNJzmMivK737GfWXWNP1NHbIlHEzs6HA=; b=qcPVcGyeGq6W5YY1BqGANgDkMe0Z74zanxke3EWUSbdCtb0yeoZYTo2Z0KHz0YkQmy TjA3Xp4dtFik2zX06st4xjOM58Pd18QZKBbUXMrYukbDP/qexjrLR9oS/IhiGUd5SDx8 Ad9ENlEcqHpMw4NcfBolI5wtGGwu9q2lUBBXZNwqxBHfCy67DVsvnloPuFIBlWIhR5xf K+VZmyuxCTBvHtZ/RMfn9k5DoWVRErsj82loncg9QoQCYVqx1eSmlo7unotvbFmZnH4t fGvW1qbn3U0qMWj6M5fYm4uuJhKAGFqAlBdqV6zbkW2d/FVbWZ99rM3VYqPwRnU9NiO0 UU5w== X-Gm-Message-State: ALQs6tBG3CEfcxTBMJVHqoh+0y/KGIge9JaUqhlsYJTc0g1zVLkih9MZ Ne3AcCQZRxd19HvCc1B+EG7Xgbuy5gU= X-Google-Smtp-Source: AB8JxZrveIwBos3/6IbJ1HzApXeShC4Qom9i/mylaezVD+6EK9+xKf1RmpajORnWsUpGjdhizCx+hA== X-Received: by 2002:adf:a0ea:: with SMTP id n39-v6mr10304148wrn.87.1525138324567; Mon, 30 Apr 2018 18:32:04 -0700 (PDT) Received: from localhost.localdomain ([182.179.189.44]) by smtp.gmail.com with ESMTPSA id e50-v6sm22743392wre.4.2018.04.30.18.32.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 30 Apr 2018 18:32:03 -0700 (PDT) From: Omair Javaid <omair.javaid@linaro.org> To: gdb-patches@sourceware.org Cc: Omair Javaid <omair.javaid@linaro.org> Subject: [PATCH] [PR gdb/23127] [AArch64] Fix tagged pointer support Date: Tue, 1 May 2018 06:31:32 +0500 Message-Id: <1525138292-4526-1-git-send-email-omair.javaid@linaro.org> X-IsSubscribed: yes |
Commit Message
Omair Javaid
May 1, 2018, 1:31 a.m. UTC
This patch fixes tagged pointer support for AArch64 GDB. Linux kernel debugging failure was reported after tagged pointer support was committed. After a discussion around best path forward to manage tagged pointers on GDB side we are going to disable tagged pointers support for aarch64-none-elf-gdb because for non-linux applications we cant be sure if tagged pointers will be used by MMU or not. Also for aarch64-linux-gdb we are going to sign extend user-space address after clearing tag bits. This will help us debug both kernel and user-space addresses based on information from linux kernel documentation given below: According to AArch64 memory map: https://www.kernel.org/doc/Documentation/arm64/memory.txt "User addresses have bits 63:48 set to 0 while the kernel addresses have the same bits set to 1." According to AArch64 tagged pointers document: https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt The kernel configures the translation tables so that translations made via TTBR0 (i.e. userspace mappings) have the top byte (bits 63:56) of the virtual address ignored by the translation hardware. This frees up this byte for application use. Running gdb testsuite after applying this patch introduces no regressions and tagged pointer test cases still pass. gdb/ChangeLog: 2018-05-01 Omair Javaid <omair.javaid@linaro.org> * aarch64-linux-tdep.c (aarch64_linux_init_abi): Add call to set_gdbarch_significant_addr_bit. * aarch64-tdep.c (aarch64_gdbarch_init): Remove call to set_gdbarch_significant_addr_bit. * utils.c (address_significant): Update to sign extend addr. --- gdb/aarch64-linux-tdep.c | 5 +++++ gdb/aarch64-tdep.c | 5 ----- gdb/utils.c | 14 +++++++++----- 3 files changed, 14 insertions(+), 10 deletions(-)
Comments
On 01/05/18 02:31, Omair Javaid wrote: > This patch fixes tagged pointer support for AArch64 GDB. Linux kernel debugging > failure was reported after tagged pointer support was committed. > > After a discussion around best path forward to manage tagged pointers on GDB > side we are going to disable tagged pointers support for aarch64-none-elf-gdb > because for non-linux applications we cant be sure if tagged pointers will be > used by MMU or not. > > Also for aarch64-linux-gdb we are going to sign extend user-space address after > clearing tag bits. This will help us debug both kernel and user-space addresses > based on information from linux kernel documentation given below: > > According to AArch64 memory map: > https://www.kernel.org/doc/Documentation/arm64/memory.txt > > "User addresses have bits 63:48 set to 0 while the kernel addresses have > the same bits set to 1." > > According to AArch64 tagged pointers document: > https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt > > The kernel configures the translation tables so that translations made > via TTBR0 (i.e. userspace mappings) have the top byte (bits 63:56) of > the virtual address ignored by the translation hardware. This frees up > this byte for application use. > > Running gdb testsuite after applying this patch introduces no regressions and > tagged pointer test cases still pass. ... and I kicked the tyres a little bit using kgdb. print worked as expected, backtrace no longer provokes a gdb panic and breakpoints work (albeit for rather approximate definition of work... and the need for approximation is not gdb's fault). Daniel. > gdb/ChangeLog: > 2018-05-01 Omair Javaid <omair.javaid@linaro.org> > > * aarch64-linux-tdep.c (aarch64_linux_init_abi): Add call to > set_gdbarch_significant_addr_bit. > * aarch64-tdep.c (aarch64_gdbarch_init): Remove call to > set_gdbarch_significant_addr_bit. > * utils.c (address_significant): Update to sign extend addr. > --- > gdb/aarch64-linux-tdep.c | 5 +++++ > gdb/aarch64-tdep.c | 5 ----- > gdb/utils.c | 14 +++++++++----- > 3 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c > index 1f3e888..ba5757d 100644 > --- a/gdb/aarch64-linux-tdep.c > +++ b/gdb/aarch64-linux-tdep.c > @@ -1062,6 +1062,11 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) > /* Syscall record. */ > tdep->aarch64_syscall_record = aarch64_linux_syscall_record; > > + /* The top byte of a user space address known as the "tag", > + is ignored by the kernel and can be regarded as additional > + data associated with the address. */ > + set_gdbarch_significant_addr_bit (gdbarch, 56); > + > /* Initialize the aarch64_linux_record_tdep. */ > /* These values are the size of the type that will be used in a system > call. They are obtained from Linux Kernel source. */ > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index 01566b4..3c1f389 100644 > --- a/gdb/aarch64-tdep.c > +++ b/gdb/aarch64-tdep.c > @@ -2972,11 +2972,6 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > set_tdesc_pseudo_register_reggroup_p (gdbarch, > aarch64_pseudo_register_reggroup_p); > > - /* The top byte of an address is known as the "tag" and is > - ignored by the kernel, the hardware, etc. and can be regarded > - as additional data associated with the address. */ > - set_gdbarch_significant_addr_bit (gdbarch, 56); > - > /* ABI */ > set_gdbarch_short_bit (gdbarch, 16); > set_gdbarch_int_bit (gdbarch, 32); > diff --git a/gdb/utils.c b/gdb/utils.c > index b957b0d..1f9be8f 100644 > --- a/gdb/utils.c > +++ b/gdb/utils.c > @@ -2704,14 +2704,18 @@ When set, debugging messages will be marked with seconds and microseconds."), > CORE_ADDR > address_significant (gdbarch *gdbarch, CORE_ADDR addr) > { > - /* Truncate address to the significant bits of a target address, > - avoiding shifts larger or equal than the width of a CORE_ADDR. > - The local variable ADDR_BIT stops the compiler reporting a shift > - overflow when it won't occur. */ > + /* Clear insignificant bits of a target address and sign extend resulting > + address, avoiding shifts larger or equal than the width of a CORE_ADDR. > + The local variable ADDR_BIT stops the compiler reporting a shift overflow > + when it won't occur. */ > int addr_bit = gdbarch_significant_addr_bit (gdbarch); > > if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT)) > - addr &= ((CORE_ADDR) 1 << addr_bit) - 1; > + { > + CORE_ADDR sign = (CORE_ADDR) 1 << (addr_bit - 1); > + addr &= ((CORE_ADDR) 1 << addr_bit) - 1; > + addr = (addr ^ sign) - sign; > + } > > return addr; > } >
On 1 May 2018 at 20:02, Daniel Thompson <daniel.thompson@linaro.org> wrote: > On 01/05/18 02:31, Omair Javaid wrote: >> >> This patch fixes tagged pointer support for AArch64 GDB. Linux kernel >> debugging >> failure was reported after tagged pointer support was committed. >> >> After a discussion around best path forward to manage tagged pointers on >> GDB >> side we are going to disable tagged pointers support for >> aarch64-none-elf-gdb >> because for non-linux applications we cant be sure if tagged pointers will >> be >> used by MMU or not. >> >> Also for aarch64-linux-gdb we are going to sign extend user-space address >> after >> clearing tag bits. This will help us debug both kernel and user-space >> addresses >> based on information from linux kernel documentation given below: >> >> According to AArch64 memory map: >> https://www.kernel.org/doc/Documentation/arm64/memory.txt >> >> "User addresses have bits 63:48 set to 0 while the kernel addresses have >> the same bits set to 1." >> >> According to AArch64 tagged pointers document: >> https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt >> >> The kernel configures the translation tables so that translations made >> via TTBR0 (i.e. userspace mappings) have the top byte (bits 63:56) of >> the virtual address ignored by the translation hardware. This frees up >> this byte for application use. >> >> Running gdb testsuite after applying this patch introduces no regressions >> and >> tagged pointer test cases still pass. > > > ... and I kicked the tyres a little bit using kgdb. > > print worked as expected, backtrace no longer provokes a gdb panic and > breakpoints work (albeit for rather approximate definition of work... and > the need for approximation is not gdb's fault). > > > Daniel. > > > >> gdb/ChangeLog: >> 2018-05-01 Omair Javaid <omair.javaid@linaro.org> >> >> * aarch64-linux-tdep.c (aarch64_linux_init_abi): Add call to >> set_gdbarch_significant_addr_bit. >> * aarch64-tdep.c (aarch64_gdbarch_init): Remove call to >> set_gdbarch_significant_addr_bit. >> * utils.c (address_significant): Update to sign extend addr. >> --- >> gdb/aarch64-linux-tdep.c | 5 +++++ >> gdb/aarch64-tdep.c | 5 ----- >> gdb/utils.c | 14 +++++++++----- >> 3 files changed, 14 insertions(+), 10 deletions(-) >> >> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c >> index 1f3e888..ba5757d 100644 >> --- a/gdb/aarch64-linux-tdep.c >> +++ b/gdb/aarch64-linux-tdep.c >> @@ -1062,6 +1062,11 @@ aarch64_linux_init_abi (struct gdbarch_info info, >> struct gdbarch *gdbarch) >> /* Syscall record. */ >> tdep->aarch64_syscall_record = aarch64_linux_syscall_record; >> + /* The top byte of a user space address known as the "tag", >> + is ignored by the kernel and can be regarded as additional >> + data associated with the address. */ >> + set_gdbarch_significant_addr_bit (gdbarch, 56); >> + >> /* Initialize the aarch64_linux_record_tdep. */ >> /* These values are the size of the type that will be used in a system >> call. They are obtained from Linux Kernel source. */ >> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c >> index 01566b4..3c1f389 100644 >> --- a/gdb/aarch64-tdep.c >> +++ b/gdb/aarch64-tdep.c >> @@ -2972,11 +2972,6 @@ aarch64_gdbarch_init (struct gdbarch_info info, >> struct gdbarch_list *arches) >> set_tdesc_pseudo_register_reggroup_p (gdbarch, >> >> aarch64_pseudo_register_reggroup_p); >> - /* The top byte of an address is known as the "tag" and is >> - ignored by the kernel, the hardware, etc. and can be regarded >> - as additional data associated with the address. */ >> - set_gdbarch_significant_addr_bit (gdbarch, 56); >> - >> /* ABI */ >> set_gdbarch_short_bit (gdbarch, 16); >> set_gdbarch_int_bit (gdbarch, 32); >> diff --git a/gdb/utils.c b/gdb/utils.c >> index b957b0d..1f9be8f 100644 >> --- a/gdb/utils.c >> +++ b/gdb/utils.c >> @@ -2704,14 +2704,18 @@ When set, debugging messages will be marked with >> seconds and microseconds."), >> CORE_ADDR >> address_significant (gdbarch *gdbarch, CORE_ADDR addr) >> { >> - /* Truncate address to the significant bits of a target address, >> - avoiding shifts larger or equal than the width of a CORE_ADDR. >> - The local variable ADDR_BIT stops the compiler reporting a shift >> - overflow when it won't occur. */ >> + /* Clear insignificant bits of a target address and sign extend >> resulting >> + address, avoiding shifts larger or equal than the width of a >> CORE_ADDR. >> + The local variable ADDR_BIT stops the compiler reporting a shift >> overflow >> + when it won't occur. */ >> int addr_bit = gdbarch_significant_addr_bit (gdbarch); >> if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT)) >> - addr &= ((CORE_ADDR) 1 << addr_bit) - 1; >> + { >> + CORE_ADDR sign = (CORE_ADDR) 1 << (addr_bit - 1); >> + addr &= ((CORE_ADDR) 1 << addr_bit) - 1; >> + addr = (addr ^ sign) - sign; >> + } >> return addr; >> } >> > Ping! Hi Pedro, I was wondering if you can kindly help review this patch. This is a critical bug as it blocks kernel debugging on AArch64. Also can we push this to GDB 8.1.1 once it gets accepted? Thanks!
Hello, Thanks much for all the investigation & discussion, both of you. On 05/08/2018 10:31 AM, Omair Javaid wrote: > I was wondering if you can kindly help review this patch. > This is a critical bug as it blocks kernel debugging on AArch64. > Also can we push this to GDB 8.1.1 once it gets accepted? Yes, this is OK for both master and branch. Please add "PR gdb/23127" to the ChangeLog before pushing. Thanks, Pedro Alves
> > I was wondering if you can kindly help review this patch. > > This is a critical bug as it blocks kernel debugging on AArch64. > > Also can we push this to GDB 8.1.1 once it gets accepted? > > Yes, this is OK for both master and branch. > > Please add "PR gdb/23127" to the ChangeLog before pushing. And I've added 8.1.1 as the target milestone in the GDB PR as well.
On Wed, May 9, 2018 at 7:19 AM, Pedro Alves <palves@redhat.com> wrote: > Hello, > > Thanks much for all the investigation & discussion, both of you. > > On 05/08/2018 10:31 AM, Omair Javaid wrote: > >> I was wondering if you can kindly help review this patch. >> This is a critical bug as it blocks kernel debugging on AArch64. >> Also can we push this to GDB 8.1.1 once it gets accepted? > > Yes, this is OK for both master and branch. > > Please add "PR gdb/23127" to the ChangeLog before pushing. > This breaks gdb on x86-64: https://sourceware.org/bugzilla/show_bug.cgi?id=23210
On Mon, 21 May 2018, 9:06 PM H.J. Lu, <hjl.tools@gmail.com> wrote: > On Wed, May 9, 2018 at 7:19 AM, Pedro Alves <palves@redhat.com> wrote: > > Hello, > > > > Thanks much for all the investigation & discussion, both of you. > > > > On 05/08/2018 10:31 AM, Omair Javaid wrote: > > > >> I was wondering if you can kindly help review this patch. > >> This is a critical bug as it blocks kernel debugging on AArch64. > >> Also can we push this to GDB 8.1.1 once it gets accepted? > > > > Yes, this is OK for both master and branch. > > > > Please add "PR gdb/23127" to the ChangeLog before pushing. > > > > This breaks gdb on x86-64: > > https://sourceware.org/bugzilla/show_bug.cgi?id=23210 > > > Hmm. I ll check it out. Thanks for reporting.
On Mon, May 21, 2018 at 9:23 AM, Omair Javaid <omair.javaid@linaro.org> wrote: > > > On Mon, 21 May 2018, 9:06 PM H.J. Lu, <hjl.tools@gmail.com> wrote: >> >> On Wed, May 9, 2018 at 7:19 AM, Pedro Alves <palves@redhat.com> wrote: >> > Hello, >> > >> > Thanks much for all the investigation & discussion, both of you. >> > >> > On 05/08/2018 10:31 AM, Omair Javaid wrote: >> > >> >> I was wondering if you can kindly help review this patch. >> >> This is a critical bug as it blocks kernel debugging on AArch64. >> >> Also can we push this to GDB 8.1.1 once it gets accepted? >> > >> > Yes, this is OK for both master and branch. >> > >> > Please add "PR gdb/23127" to the ChangeLog before pushing. >> > >> >> This breaks gdb on x86-64: >> >> https://sourceware.org/bugzilla/show_bug.cgi?id=23210 >> >> > > Hmm. I ll check it out. Thanks for reporting. > Please fix both master and gdb-8.1-branch branches Thanks.
On Monday, May 21 2018, Omair Javaid wrote: > On Mon, 21 May 2018, 9:06 PM H.J. Lu, <hjl.tools@gmail.com> wrote: > >> On Wed, May 9, 2018 at 7:19 AM, Pedro Alves <palves@redhat.com> wrote: >> > Hello, >> > >> > Thanks much for all the investigation & discussion, both of you. >> > >> > On 05/08/2018 10:31 AM, Omair Javaid wrote: >> > >> >> I was wondering if you can kindly help review this patch. >> >> This is a critical bug as it blocks kernel debugging on AArch64. >> >> Also can we push this to GDB 8.1.1 once it gets accepted? >> > >> > Yes, this is OK for both master and branch. >> > >> > Please add "PR gdb/23127" to the ChangeLog before pushing. >> > >> >> This breaks gdb on x86-64: >> >> https://sourceware.org/bugzilla/show_bug.cgi?id=23210 >> >> >> > Hmm. I ll check it out. Thanks for reporting. Hi Omair, Any news on this failure? It is causing the testcase to take more than 14 hours to run when using --target_board=unix/-m32, and is impacting some of our builders on BuildBot. Thanks,
On Friday, May 25 2018, I wrote: > On Monday, May 21 2018, Omair Javaid wrote: > >> On Mon, 21 May 2018, 9:06 PM H.J. Lu, <hjl.tools@gmail.com> wrote: >> >>> On Wed, May 9, 2018 at 7:19 AM, Pedro Alves <palves@redhat.com> wrote: >>> > Hello, >>> > >>> > Thanks much for all the investigation & discussion, both of you. >>> > >>> > On 05/08/2018 10:31 AM, Omair Javaid wrote: >>> > >>> >> I was wondering if you can kindly help review this patch. >>> >> This is a critical bug as it blocks kernel debugging on AArch64. >>> >> Also can we push this to GDB 8.1.1 once it gets accepted? >>> > >>> > Yes, this is OK for both master and branch. >>> > >>> > Please add "PR gdb/23127" to the ChangeLog before pushing. >>> > >>> >>> This breaks gdb on x86-64: >>> >>> https://sourceware.org/bugzilla/show_bug.cgi?id=23210 >>> >>> >>> >> Hmm. I ll check it out. Thanks for reporting. > > Hi Omair, > > Any news on this failure? It is causing the testcase to take more than > 14 hours to run when using --target_board=unix/-m32, and is impacting > some of our builders on BuildBot. Apparently you sent a fix already. Sorry about this message.
On Fri, May 25, 2018 at 10:11 AM, Sergio Durigan Junior <sergiodj@redhat.com> wrote: > On Friday, May 25 2018, I wrote: > >> On Monday, May 21 2018, Omair Javaid wrote: >> >>> On Mon, 21 May 2018, 9:06 PM H.J. Lu, <hjl.tools@gmail.com> wrote: >>> >>>> On Wed, May 9, 2018 at 7:19 AM, Pedro Alves <palves@redhat.com> wrote: >>>> > Hello, >>>> > >>>> > Thanks much for all the investigation & discussion, both of you. >>>> > >>>> > On 05/08/2018 10:31 AM, Omair Javaid wrote: >>>> > >>>> >> I was wondering if you can kindly help review this patch. >>>> >> This is a critical bug as it blocks kernel debugging on AArch64. >>>> >> Also can we push this to GDB 8.1.1 once it gets accepted? >>>> > >>>> > Yes, this is OK for both master and branch. >>>> > >>>> > Please add "PR gdb/23127" to the ChangeLog before pushing. >>>> > >>>> >>>> This breaks gdb on x86-64: >>>> >>>> https://sourceware.org/bugzilla/show_bug.cgi?id=23210 >>>> >>>> >>>> >>> Hmm. I ll check it out. Thanks for reporting. >> >> Hi Omair, >> >> Any news on this failure? It is causing the testcase to take more than >> 14 hours to run when using --target_board=unix/-m32, and is impacting >> some of our builders on BuildBot. > > Apparently you sent a fix already. Sorry about this message. > But the fix is wrong.
On Fri, 25 May 2018, 10:12 PM H.J. Lu, <hjl.tools@gmail.com> wrote: > On Fri, May 25, 2018 at 10:11 AM, Sergio Durigan Junior > <sergiodj@redhat.com> wrote: > > On Friday, May 25 2018, I wrote: > > > >> On Monday, May 21 2018, Omair Javaid wrote: > >> > >>> On Mon, 21 May 2018, 9:06 PM H.J. Lu, <hjl.tools@gmail.com> wrote: > >>> > >>>> On Wed, May 9, 2018 at 7:19 AM, Pedro Alves <palves@redhat.com> > wrote: > >>>> > Hello, > >>>> > > >>>> > Thanks much for all the investigation & discussion, both of you. > >>>> > > >>>> > On 05/08/2018 10:31 AM, Omair Javaid wrote: > >>>> > > >>>> >> I was wondering if you can kindly help review this patch. > >>>> >> This is a critical bug as it blocks kernel debugging on AArch64. > >>>> >> Also can we push this to GDB 8.1.1 once it gets accepted? > >>>> > > >>>> > Yes, this is OK for both master and branch. > >>>> > > >>>> > Please add "PR gdb/23127" to the ChangeLog before pushing. > >>>> > > >>>> > >>>> This breaks gdb on x86-64: > >>>> > >>>> https://sourceware.org/bugzilla/show_bug.cgi?id=23210 > >>>> > >>>> > >>>> > >>> Hmm. I ll check it out. Thanks for reporting. > >> > >> Hi Omair, > >> > >> Any news on this failure? It is causing the testcase to take more than > >> 14 hours to run when using --target_board=unix/-m32, and is impacting > >> some of our builders on BuildBot. > > > > Apparently you sent a fix already. Sorry about this message. > > > > But the fix is wrong. > I will update gdbarch as suggested. Are there any other issues in the fix I should address.? > > -- > H.J. >
On Friday, May 25 2018, Omair Javaid wrote: >> But the fix is wrong. >> > > I will update gdbarch as suggested. Are there any other issues in the fix I > should address.? I think we should discuss that on the thread.
diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index 1f3e888..ba5757d 100644 --- a/gdb/aarch64-linux-tdep.c +++ b/gdb/aarch64-linux-tdep.c @@ -1062,6 +1062,11 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) /* Syscall record. */ tdep->aarch64_syscall_record = aarch64_linux_syscall_record; + /* The top byte of a user space address known as the "tag", + is ignored by the kernel and can be regarded as additional + data associated with the address. */ + set_gdbarch_significant_addr_bit (gdbarch, 56); + /* Initialize the aarch64_linux_record_tdep. */ /* These values are the size of the type that will be used in a system call. They are obtained from Linux Kernel source. */ diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 01566b4..3c1f389 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -2972,11 +2972,6 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) set_tdesc_pseudo_register_reggroup_p (gdbarch, aarch64_pseudo_register_reggroup_p); - /* The top byte of an address is known as the "tag" and is - ignored by the kernel, the hardware, etc. and can be regarded - as additional data associated with the address. */ - set_gdbarch_significant_addr_bit (gdbarch, 56); - /* ABI */ set_gdbarch_short_bit (gdbarch, 16); set_gdbarch_int_bit (gdbarch, 32); diff --git a/gdb/utils.c b/gdb/utils.c index b957b0d..1f9be8f 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -2704,14 +2704,18 @@ When set, debugging messages will be marked with seconds and microseconds."), CORE_ADDR address_significant (gdbarch *gdbarch, CORE_ADDR addr) { - /* Truncate address to the significant bits of a target address, - avoiding shifts larger or equal than the width of a CORE_ADDR. - The local variable ADDR_BIT stops the compiler reporting a shift - overflow when it won't occur. */ + /* Clear insignificant bits of a target address and sign extend resulting + address, avoiding shifts larger or equal than the width of a CORE_ADDR. + The local variable ADDR_BIT stops the compiler reporting a shift overflow + when it won't occur. */ int addr_bit = gdbarch_significant_addr_bit (gdbarch); if (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT)) - addr &= ((CORE_ADDR) 1 << addr_bit) - 1; + { + CORE_ADDR sign = (CORE_ADDR) 1 << (addr_bit - 1); + addr &= ((CORE_ADDR) 1 << addr_bit) - 1; + addr = (addr ^ sign) - sign; + } return addr; }