[PATCHv3,0/8] x86/Linux Target Description Changes

  In v3:

  - Rebased.  Nasty merge conflict with 4bb20a6244b7091 which I think
    I've resolved, but am unable to test.  Reposting so the author of
    that other commit can validate.

  - Initial testing looks good.  Full tests are still running.

In v2:

  - Rebase to current upstream/master, no merge conflicts,

  - Retested.

---

Andrew Burgess (8):
  gdbserver: convert have_ptrace_getregset to a tribool
  gdb/x86: move reading of cs and ds state into gdb/nat directory
  gdbserver/x86: move no-xml code earlier in x86_linux_read_description
  gdb/gdbserver: share I386_LINUX_XSAVE_XCR0_OFFSET definition
  gdb/gdbserver: share some code relating to target description creation
  gdb/arch: assert that X86_XSTATE_MPX is not set for x32
  gdbserver: update target description creation for x86/linux
  gdb/gdbserver: share x86/linux tdesc caching

 gdb/Makefile.in              |   1 +
 gdb/amd64-linux-tdep.c       |  33 +--
 gdb/amd64-linux-tdep.h       |   6 -
 gdb/arch/amd64.c             |   8 +-
 gdb/configure.nat            |   4 +-
 gdb/i386-linux-tdep.c        |  32 +--
 gdb/i386-linux-tdep.h        |  23 --
 gdb/nat/x86-linux-tdesc.c    | 411 +++++++++++++++++++++++++++++++++++
 gdb/nat/x86-linux-tdesc.h    | 115 ++++++++++
 gdb/nat/x86-linux.c          |  47 ++++
 gdb/nat/x86-linux.h          |  48 ++++
 gdb/x86-linux-nat.c          | 123 ++---------
 gdbserver/Makefile.in        |   4 +
 gdbserver/configure.srv      |   4 +
 gdbserver/linux-amd64-ipa.cc |  45 +---
 gdbserver/linux-arm-low.cc   |   6 +-
 gdbserver/linux-i386-ipa.cc  |  25 +--
 gdbserver/linux-low.cc       |   2 +-
 gdbserver/linux-low.h        |   2 +-
 gdbserver/linux-x86-low.cc   | 189 +++++-----------
 gdbserver/linux-x86-tdesc.cc | 141 +-----------
 gdbserver/linux-x86-tdesc.h  |  56 -----
 22 files changed, 749 insertions(+), 576 deletions(-)
 create mode 100644 gdb/nat/x86-linux-tdesc.c
 create mode 100644 gdb/nat/x86-linux-tdesc.h
 delete mode 100644 gdbserver/linux-x86-tdesc.h


base-commit: e9315f148d56b3f4c7cfeef469633e85933d412c
  

Andrew Burgess <aburgess@redhat.com> writes:

> In v3:
>
>   - Rebased.  Nasty merge conflict with 4bb20a6244b7091 which I think
>     I've resolved, but am unable to test.  Reposting so the author of
>     that other commit can validate.
>
>   - Initial testing looks good.  Full tests are still running.

Testing completed with no issues.  H.J. Lu confirmed that this versions
didn't break the x32 behaviour.  I've gone ahead and pushed these
patches.

If anything crops up then do let me know.

Thanks,
Andrew
  

On 3/25/24 13:20, Andrew Burgess wrote:
> Andrew Burgess <aburgess@redhat.com> writes:
> 
>> In v3:
>>
>>   - Rebased.  Nasty merge conflict with 4bb20a6244b7091 which I think
>>     I've resolved, but am unable to test.  Reposting so the author of
>>     that other commit can validate.
>>
>>   - Initial testing looks good.  Full tests are still running.
> 
> Testing completed with no issues.  H.J. Lu confirmed that this versions
> didn't break the x32 behaviour.  I've gone ahead and pushed these
> patches.
> 
> If anything crops up then do let me know.
> 
> Thanks,
> Andrew
> 

Hi Andrew,

I think your series introduces some build failures with Clang.  One is
easy, it's a missing `-x c++` in gdbserver/Makefile.in.

The other is:

      CXX    nat/x86-linux-tdesc-ipa.o
    /home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/x86-linux-tdesc.c:167:1: error: unused function 'x86_linux_i386_tdesc_feature_mask' [-Werror,-Wunused-function]
      167 | x86_linux_i386_tdesc_feature_mask ()
          | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It's possible that this function just needs to be moved in the same
"#ifdef" as where it's used, but since I didn't follow your work
closely, I prefer to let you fix it, in case I'm missing something.

Thanks,

Simon
  

Simon Marchi <simark@simark.ca> writes:

> On 3/25/24 13:20, Andrew Burgess wrote:
>> Andrew Burgess <aburgess@redhat.com> writes:
>> 
>>> In v3:
>>>
>>>   - Rebased.  Nasty merge conflict with 4bb20a6244b7091 which I think
>>>     I've resolved, but am unable to test.  Reposting so the author of
>>>     that other commit can validate.
>>>
>>>   - Initial testing looks good.  Full tests are still running.
>> 
>> Testing completed with no issues.  H.J. Lu confirmed that this versions
>> didn't break the x32 behaviour.  I've gone ahead and pushed these
>> patches.
>> 
>> If anything crops up then do let me know.
>> 
>> Thanks,
>> Andrew
>> 
>
> Hi Andrew,
>
> I think your series introduces some build failures with Clang.  One is
> easy, it's a missing `-x c++` in gdbserver/Makefile.in.

Thanks for fixing this one.

>
> The other is:
>
>       CXX    nat/x86-linux-tdesc-ipa.o
>     /home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/x86-linux-tdesc.c:167:1: error: unused function 'x86_linux_i386_tdesc_feature_mask' [-Werror,-Wunused-function]
>       167 | x86_linux_i386_tdesc_feature_mask ()
>           | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> It's possible that this function just needs to be moved in the same
> "#ifdef" as where it's used, but since I didn't follow your work
> closely, I prefer to let you fix it, in case I'm missing something.

Sorry for the breakage.

I pushed the patch below to resolve this issue.

Thanks,
Andrew

--

commit f4c19f89ef43dbce8065532c808e1aeb05d08994
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Mar 26 12:09:27 2024 +0000

    gdb/gdbserver: fix some defined but unused function warnings
    
    This commit:
    
      commit 198ff6ff819c240545f9fc68b39636fd376d4ba9
      Date:   Tue Jan 30 15:37:23 2024 +0000
    
          gdb/gdbserver: share x86/linux tdesc caching
    
    added some functions which are always defined, but their use is
    guarded within various #ifdef blocks.  As a result we were seeing
    errors about defined, but unused, functions.
    
    I've fixed this problem in this commit by wrapping the function
    definitions within #ifdef blocks.
    
    I'm a little worried that there might be too many #ifdef blocks within
    this file, however, I'm going to commit this fix for now as this will
    fix the build, then I'll think about if there's a better way to split
    this file so we might avoid some of these #ifdef blocks.

diff --git a/gdb/nat/x86-linux-tdesc.c b/gdb/nat/x86-linux-tdesc.c
index c438dfae84f..8a02f77fa6a 100644
--- a/gdb/nat/x86-linux-tdesc.c
+++ b/gdb/nat/x86-linux-tdesc.c
@@ -160,6 +160,8 @@ static constexpr x86_tdesc_feature x86_linux_all_tdesc_features[] = {
   { X86_XSTATE_X87,	true,	false, 	false }
 };
 
+#if defined __i386__ || !defined IN_PROCESS_AGENT
+
 /* Return a compile time constant which is a mask of all the cpu features
    that are checked for when building an i386 target description.  */
 
@@ -175,6 +177,10 @@ x86_linux_i386_tdesc_feature_mask ()
   return mask;
 }
 
+#endif /* __i386__ || !IN_PROCESS_AGENT */
+
+#ifdef __x86_64__
+
 /* Return a compile time constant which is a mask of all the cpu features
    that are checked for when building an amd64 target description.  */
 
@@ -205,6 +211,8 @@ x86_linux_x32_tdesc_feature_mask ()
   return mask;
 }
 
+#endif /* __x86_64__ */
+
 /* Return a compile time constant which is a count of the number of cpu
    features that are checked for when building an i386 target description.  */
 
@@ -222,6 +230,8 @@ x86_linux_i386_tdesc_count ()
   return (1 << count);
 }
 
+#if defined __x86_64__ || defined IN_PROCESS_AGENT
+
 /* Return a compile time constant which is a count of the number of cpu
    features that are checked for when building an amd64 target description.  */
 
@@ -256,6 +266,8 @@ x86_linux_x32_tdesc_count ()
   return (1 << count);
 }
 
+#endif /* __x86_64__ || IN_PROCESS_AGENT */
+
 #ifdef IN_PROCESS_AGENT
 
 /* See linux-x86-tdesc.h.  */
  

On Tue, Mar 26, 2024 at 5:16 AM Andrew Burgess <aburgess@redhat.com> wrote:
>
> Simon Marchi <simark@simark.ca> writes:
>
> > On 3/25/24 13:20, Andrew Burgess wrote:
> >> Andrew Burgess <aburgess@redhat.com> writes:
> >>
> >>> In v3:
> >>>
> >>>   - Rebased.  Nasty merge conflict with 4bb20a6244b7091 which I think
> >>>     I've resolved, but am unable to test.  Reposting so the author of
> >>>     that other commit can validate.
> >>>
> >>>   - Initial testing looks good.  Full tests are still running.
> >>
> >> Testing completed with no issues.  H.J. Lu confirmed that this versions
> >> didn't break the x32 behaviour.  I've gone ahead and pushed these
> >> patches.
> >>
> >> If anything crops up then do let me know.
> >>
> >> Thanks,
> >> Andrew
> >>
> >
> > Hi Andrew,
> >
> > I think your series introduces some build failures with Clang.  One is
> > easy, it's a missing `-x c++` in gdbserver/Makefile.in.
>
> Thanks for fixing this one.
>
> >
> > The other is:
> >
> >       CXX    nat/x86-linux-tdesc-ipa.o
> >     /home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/x86-linux-tdesc.c:167:1: error: unused function 'x86_linux_i386_tdesc_feature_mask' [-Werror,-Wunused-function]
> >       167 | x86_linux_i386_tdesc_feature_mask ()
> >           | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > It's possible that this function just needs to be moved in the same
> > "#ifdef" as where it's used, but since I didn't follow your work
> > closely, I prefer to let you fix it, in case I'm missing something.
>
> Sorry for the breakage.
>
> I pushed the patch below to resolve this issue.
>

gdbserver from your old patch:

https://patchwork.sourceware.org/project/gdb/list/?series=32189

works on x32.  But the current master gives me:

/export/gnu/import/git/sources/gdb/gdbserver/regcache.cc:273: A
problem internal to GDBserver has been detected.
Unknown register bnd0raw requested

again.
  

On Tue, Mar 26, 2024 at 6:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Mar 26, 2024 at 5:16 AM Andrew Burgess <aburgess@redhat.com> wrote:
> >
> > Simon Marchi <simark@simark.ca> writes:
> >
> > > On 3/25/24 13:20, Andrew Burgess wrote:
> > >> Andrew Burgess <aburgess@redhat.com> writes:
> > >>
> > >>> In v3:
> > >>>
> > >>>   - Rebased.  Nasty merge conflict with 4bb20a6244b7091 which I think
> > >>>     I've resolved, but am unable to test.  Reposting so the author of
> > >>>     that other commit can validate.
> > >>>
> > >>>   - Initial testing looks good.  Full tests are still running.
> > >>
> > >> Testing completed with no issues.  H.J. Lu confirmed that this versions
> > >> didn't break the x32 behaviour.  I've gone ahead and pushed these
> > >> patches.
> > >>
> > >> If anything crops up then do let me know.
> > >>
> > >> Thanks,
> > >> Andrew
> > >>
> > >
> > > Hi Andrew,
> > >
> > > I think your series introduces some build failures with Clang.  One is
> > > easy, it's a missing `-x c++` in gdbserver/Makefile.in.
> >
> > Thanks for fixing this one.
> >
> > >
> > > The other is:
> > >
> > >       CXX    nat/x86-linux-tdesc-ipa.o
> > >     /home/smarchi/src/binutils-gdb/gdbserver/../gdb/nat/x86-linux-tdesc.c:167:1: error: unused function 'x86_linux_i386_tdesc_feature_mask' [-Werror,-Wunused-function]
> > >       167 | x86_linux_i386_tdesc_feature_mask ()
> > >           | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > It's possible that this function just needs to be moved in the same
> > > "#ifdef" as where it's used, but since I didn't follow your work
> > > closely, I prefer to let you fix it, in case I'm missing something.
> >
> > Sorry for the breakage.
> >
> > I pushed the patch below to resolve this issue.
> >
>
> gdbserver from your old patch:
>
> https://patchwork.sourceware.org/project/gdb/list/?series=32189
>
> works on x32.  But the current master gives me:
>
> /export/gnu/import/git/sources/gdb/gdbserver/regcache.cc:273: A
> problem internal to GDBserver has been detected.
> Unknown register bnd0raw requested
>
> again.

Never mind.  I was using the wrong source.  x32 works.
  

Andrew Burgess <aburgess@redhat.com> writes:

> Andrew Burgess <aburgess@redhat.com> writes:
>
>> In v3:
>>
>>   - Rebased.  Nasty merge conflict with 4bb20a6244b7091 which I think
>>     I've resolved, but am unable to test.  Reposting so the author of
>>     that other commit can validate.
>>
>>   - Initial testing looks good.  Full tests are still running.
>
> Testing completed with no issues.  H.J. Lu confirmed that this versions
> didn't break the x32 behaviour.  I've gone ahead and pushed these
> patches.
>
> If anything crops up then do let me know.

I'm aware that this series broke pretty much anything that was not x86
based when configured with --enable-targets=all.

The problem is that, as part of this commit, I moved generic tdep (arch
specific) code into the nat/ directory as part of an effort to share the
code between GDB and gdbserver.  This was incorrect.

I'm open to reverting this series, however, I'm currently working on a
fix, splitting the nat/ code between nat/ and arch/.  I'm hoping to have
a fix today.  If that doesn't look possible then I'll likely revert this
series before the end of my day.

Apologies for the (serious) breakage.

Thanks,
Andrew
  

Andrew Burgess <aburgess@redhat.com> writes:

> Andrew Burgess <aburgess@redhat.com> writes:
>
>> Andrew Burgess <aburgess@redhat.com> writes:
>>
>>> In v3:
>>>
>>>   - Rebased.  Nasty merge conflict with 4bb20a6244b7091 which I think
>>>     I've resolved, but am unable to test.  Reposting so the author of
>>>     that other commit can validate.
>>>
>>>   - Initial testing looks good.  Full tests are still running.
>>
>> Testing completed with no issues.  H.J. Lu confirmed that this versions
>> didn't break the x32 behaviour.  I've gone ahead and pushed these
>> patches.
>>
>> If anything crops up then do let me know.
>
> I'm aware that this series broke pretty much anything that was not x86
> based when configured with --enable-targets=all.
>
> The problem is that, as part of this commit, I moved generic tdep (arch
> specific) code into the nat/ directory as part of an effort to share the
> code between GDB and gdbserver.  This was incorrect.
>
> I'm open to reverting this series, however, I'm currently working on a
> fix, splitting the nat/ code between nat/ and arch/.  I'm hoping to have
> a fix today.  If that doesn't look possible then I'll likely revert this
> series before the end of my day.
>
> Apologies for the (serious) breakage.

I have now reverted all of the commits from this series, as well as the
small fixes which I pushed earlier today.  I've also reverts the fix
which Simon pushed that was for code from this series.

Splitting the code between nat/ and arch/ mostly worked, but I was still
running into some edge cases which I couldn't get working, so I could
either:

  + push a partial fix, which still left some things broken, or

  + revert everything, take a step back and re-work the series.

I think reverting and coming at this fresh tomorrow is the better
choice.

I apologise for all the churn.  The post-revert branch seemed to build
fine, so hopefully I've not left anything in a broken state.

Let me know if I've left anything broken, I'll check back in a couple of
hours to see if there is more fixing needed.

Thanks,
Andrew