Bug 25981

Summary: Use of short i386 register names breaks compilation on recent Solaris 11.4
Product: gdb Reporter: Rainer Orth <ro>
Component: buildAssignee: Rainer Orth <ro>
Status: RESOLVED FIXED    
Severity: normal CC: brobecker
Priority: P2    
Version: HEAD   
Target Milestone: 9.2   
URL: https://sourceware.org/pipermail/gdb-patches/2020-May/168713.html, https://sourceware.org/pipermail/gdb-patches/2020-May/168714.html
Host: amd64-pc-solaris2.11, i386-pc-solaris2.11 Target: amd64-pc-solaris2.11, i386-pc-solaris2.11
Build: amd64-pc-solaris2.11, i386-pc-solaris2.11 Last reconfirmed:

Description Rainer Orth 2020-05-13 08:42:06 UTC
A future Solaris 11.4 SRU will soon remove the short i386 register names (like
UESP, GS, ...) from <sys/regset.h>.  They can unexpectedly leak into user code
in some circumstances and pollute the user's namespace.

They will only remain in <ucontext.h> since the are prescribed by the i386 psABI
to be present when that header is included.

This change breaks gdb compilation on both 64 and 32-bit Solaris/x86:

* On both amd64 and i386, procfs.c doesn't compile:

/vol/src/gnu/gdb/hg/master/git/gdb/procfs.c: In function 'ssd* procfs_find_LDT_entry(ptid_t)':
/vol/src/gnu/gdb/hg/master/git/gdb/procfs.c:1643:18: error: 'GS' was not declared in this scope
 1643 |   key = (*gregs)[GS] & 0xffff;
      |                  ^~
make[2]: *** [Makefile:1607: procfs.o] Error 1

* On i386 only, i386-sol2-nat.c doesn't compile:

/vol/src/gnu/gdb/hg/master/git/gdb/i386-sol2-nat.c:181:3: error: 'EAX' was not declared in this scope
  181 |   EAX, ECX, EDX, EBX,
      |   ^~~

  and many more.

Both can be easily fixed: procfs_find_LDT_entry has long been obsolete and can
simply go, and i386-sol2-nat.c can just hardcode the register numbers as the
64-bit counterpart already does.

Patches forthcoming; I'm just filing this PR since the changes should also go
into the gdb-9 branch which needs the PR to go forward.
Comment 1 Rainer Orth 2020-05-13 08:43:02 UTC
Mine.
Comment 2 Joel Brobecker 2020-05-18 14:13:43 UTC
OK to backport to 9.2 (simple, can only affect Solaris)
Comment 3 cvs-commit@gcc.gnu.org 2020-05-18 15:57:49 UTC
The master branch has been updated by Rainer Orth <ro@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7f2043399809c0ba5c4819172214371ed820e8c6

commit 7f2043399809c0ba5c4819172214371ed820e8c6
Author: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
Date:   Mon May 18 17:56:00 2020 +0200

    Remove unused ps_lgetLDT etc. on Solaris/x86 [PR25981]
    
    As reported in PR build/25981, a future Solaris 11.4 update will soon
    remove the short i386 register names like SS etc. from <sys/regset.h>.
    They could leak into user code (e.g. via <signal.h> -> <sys/signal.h> ->
    <sys/ucontext.h>) and pollute the user namespace.  Affected code would
    have a hard time avoiding the issue: LLVM is one of those.
    
    While the short names are required to be present by the i386 psABI, that
    document only demands that they exist in <ucontext.h>, which is what the
    upcoming update assures.
    
    With this change, in a 64-bit-default configuration, procfs.c fails to
    compile on Solaris/x86:
    
    /vol/src/gnu/gdb/hg/master/git/gdb/procfs.c: In function 'ssd* procfs_find_LDT_entry(ptid_t)':
    /vol/src/gnu/gdb/hg/master/git/gdb/procfs.c:1643:18: error: 'GS' was not declared in this scope
     1643 |   key = (*gregs)[GS] & 0xffff;
          |                  ^~
    make[2]: *** [Makefile:1607: procfs.o] Error 1
    
    Initially I meant to provide a definition using the planned replacement
    macro, but closer inspection revealed a better way.  procfs_find_LDT_entry
    and its helper proc_get_LDT_entry are only used to implement ps_lgetLDT,
    one of the callback functions required by libthread_db.so.1
    (cf. <proc_service.h>).  While that function is still documented as being
    required even in Solaris 11.4, I found that calls to it had been removed
    long ago in Solaris 9, so just removing the three functions above is the
    easiest fix.
    
    The following patch does just that.  It compiled successfully on
    amd64-pc-solaris2.11, however, as reported in PR gdb/25939, master is
    completely broken on Solaris since the multi-target patch.  The patch
    applies cleanly to the gdb-9 branch and there I could test it
    successfully.
    
            PR build/25981
            * procfs.c [(__i386__ || __x86_64__) && sun] (proc_get_LDT_entry,
            procfs_find_LDT_entry): Remove.
            * procfs.h [(__i386__ || __x86_64__) && sun] (struct ssd,
            procfs_find_LDT_entry): Remove.
            * sol-thread.c [(__i386__ || __x86_64__) && sun] (ps_lgetLDT):
            Remove.
Comment 4 cvs-commit@gcc.gnu.org 2020-05-18 16:01:22 UTC
The master branch has been updated by Rainer Orth <ro@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e52a0f1bd949e1b6b6bcadc284c8f84464d46f2c

commit e52a0f1bd949e1b6b6bcadc284c8f84464d46f2c
Author: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
Date:   Mon May 18 17:59:43 2020 +0200

    Avoid short i386 register names on Solaris/x86 [PR25981]
    
    This is the 32-bit companion to
    
            Remove unused ps_lgetLDT etc. on Solaris/x86 [PR25981]
            https://sourceware.org/pipermail/gdb-patches/2020-May/168713.html
    
    A 32-bit-default gdb fails to compile with the updated <sys/regset.h>.
    While it is also affected by the lack of a GS definition, which the
    compantion patch above fixes, it also fails to compile i386-sol2-nat.c like
    this
    
    /vol/src/gnu/gdb/hg/master/git/gdb/i386-sol2-nat.c:181:3: error: 'EAX' was not declared in this scope
      181 |   EAX, ECX, EDX, EBX,
          |   ^~~
    
    and several more.
    
    While this could be fixed by either including <ucontext.h> here or
    provding fallback definitions of the register macros, I chose to do what
    the 64-bit-default code in the same file
    (amd64_sol2_gregset32_reg_offset[]) does, namely just hardcode the
    numeric values instead.  They are part of the ABI and thus guaranteed
    not to change.
    
    With this patch, a i386-pc-solaris2.11 configuration on master compiles
    again, however, it doesn't work.  However, I could successfully test it
    on the gdb-9 branch.
    
    Compiling and testing proved to be messy, unfortunately:
    
    * For one, Solaris <sys/procfs.h> and largefile support used to be
      mutually exclusive (fixed in Solaris 11.4 and Illumos), which was
      exacerbated by the fact that g++ predefines _FILE_OFFSET_BITS=64 since
      GCC 9.1.0.  For now I've worked around this by adding
      -U_FILE_OFFSET_BITS to CXXFLAGS and configuring with
      --disable-largefile.  I hope to clean this up in a future patch.
    
    * gdb still defaults to startup-with-shell on.  However, /bin/bash is a
      64-bit executable which cannot be debugged by a 32-bit gdb.  I hacked
      around that part by pointing $SHELL at a 32-bit bash before running
      make check.
    
            PR build/25981
            * i386-sol2-nat.c [PR_MODEL_NATIVE != PR_MODEL_LP64] (regmap):
            Hardcode register numbers.
Comment 5 cvs-commit@gcc.gnu.org 2020-05-19 08:04:38 UTC
The gdb-9-branch branch has been updated by Rainer Orth <ro@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ff6da943781d65f7ebc8e2e5ab52fb85590ea60b

commit ff6da943781d65f7ebc8e2e5ab52fb85590ea60b
Author: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
Date:   Tue May 19 10:03:14 2020 +0200

    Remove unused ps_lgetLDT etc. on Solaris/x86 [PR25981]
    
    As reported in PR build/25981, a future Solaris 11.4 update will soon
    remove the short i386 register names like SS etc. from <sys/regset.h>.
    They could leak into user code (e.g. via <signal.h> -> <sys/signal.h> ->
    <sys/ucontext.h>) and pollute the user namespace.  Affected code would
    have a hard time avoiding the issue: LLVM is one of those.
    
    While the short names are required to be present by the i386 psABI, that
    document only demands that they exist in <ucontext.h>, which is what the
    upcoming update assures.
    
    With this change, in a 64-bit-default configuration, procfs.c fails to
    compile on Solaris/x86:
    
    /vol/src/gnu/gdb/hg/master/git/gdb/procfs.c: In function 'ssd* procfs_find_LDT_entry(ptid_t)':
    /vol/src/gnu/gdb/hg/master/git/gdb/procfs.c:1643:18: error: 'GS' was not declared in this scope
     1643 |   key = (*gregs)[GS] & 0xffff;
          |                  ^~
    make[2]: *** [Makefile:1607: procfs.o] Error 1
    
    Initially I meant to provide a definition using the planned replacement
    macro, but closer inspection revealed a better way.  procfs_find_LDT_entry
    and its helper proc_get_LDT_entry are only used to implement ps_lgetLDT,
    one of the callback functions required by libthread_db.so.1
    (cf. <proc_service.h>).  While that function is still documented as being
    required even in Solaris 11.4, I found that calls to it had been removed
    long ago in Solaris 9, so just removing the three functions above is the
    easiest fix.
    
    The following patch does just that.  It compiled successfully on
    amd64-pc-solaris2.11, however, as reported in PR gdb/25939, master is
    completely broken on Solaris since the multi-target patch.  The patch
    applies cleanly to the gdb-9 branch and there I could test it
    successfully.
    
            PR build/25981
            * procfs.c [(__i386__ || __x86_64__) && sun] (proc_get_LDT_entry,
            procfs_find_LDT_entry): Remove.
            * procfs.h [(__i386__ || __x86_64__) && sun] (struct ssd,
            procfs_find_LDT_entry): Remove.
            * sol-thread.c [(__i386__ || __x86_64__) && sun] (ps_lgetLDT):
            Remove.
Comment 6 cvs-commit@gcc.gnu.org 2020-05-19 08:07:15 UTC
The gdb-9-branch branch has been updated by Rainer Orth <ro@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3bfcbaaa2b23824b1584d2782be64820e4a35acb

commit 3bfcbaaa2b23824b1584d2782be64820e4a35acb
Author: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
Date:   Tue May 19 10:06:00 2020 +0200

    Avoid short i386 register names on Solaris/x86 [PR25981]
    
    This is the 32-bit companion to
    
            Remove unused ps_lgetLDT etc. on Solaris/x86 [PR25981]
            https://sourceware.org/pipermail/gdb-patches/2020-May/168713.html
    
    A 32-bit-default gdb fails to compile with the updated <sys/regset.h>.
    While it is also affected by the lack of a GS definition, which the
    compantion patch above fixes, it also fails to compile i386-sol2-nat.c like
    this
    
    /vol/src/gnu/gdb/hg/master/git/gdb/i386-sol2-nat.c:181:3: error: 'EAX' was not declared in this scope
      181 |   EAX, ECX, EDX, EBX,
          |   ^~~
    
    and several more.
    
    While this could be fixed by either including <ucontext.h> here or
    provding fallback definitions of the register macros, I chose to do what
    the 64-bit-default code in the same file
    (amd64_sol2_gregset32_reg_offset[]) does, namely just hardcode the
    numeric values instead.  They are part of the ABI and thus guaranteed
    not to change.
    
    With this patch, a i386-pc-solaris2.11 configuration on master compiles
    again, however, it doesn't work.  However, I could successfully test it
    on the gdb-9 branch.
    
    Compiling and testing proved to be messy, unfortunately:
    
    * For one, Solaris <sys/procfs.h> and largefile support used to be
      mutually exclusive (fixed in Solaris 11.4 and Illumos), which was
      exacerbated by the fact that g++ predefines _FILE_OFFSET_BITS=64 since
      GCC 9.1.0.  For now I've worked around this by adding
      -U_FILE_OFFSET_BITS to CXXFLAGS and configuring with
      --disable-largefile.  I hope to clean this up in a future patch.
    
    * gdb still defaults to startup-with-shell on.  However, /bin/bash is a
      64-bit executable which cannot be debugged by a 32-bit gdb.  I hacked
      around that part by pointing $SHELL at a 32-bit bash before running
      make check.
    
            PR build/25981
            * i386-sol2-nat.c [PR_MODEL_NATIVE != PR_MODEL_LP64] (regmap):
            Hardcode register numbers.
Comment 7 Rainer Orth 2020-05-19 08:15:31 UTC
Fixed for GDB 9.2 and 10.1.