[Bug build/22206] New: [8.1 regression] SPARC M7 ADI patches break Solaris build

[ro at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=22206

Bug ID: 22206
Summary: [8.1 regression] SPARC M7 ADI patches break Solaris
build
Product: gdb
Version: HEAD
Status: NEW
Severity: normal
Priority: P2
Component: build
Assignee: unassigned at sourceware dot org
Reporter: ro at gcc dot gnu.org
CC: weimin.pan at oracle dot com
Target Milestone: ---
Target: sparc*-sun-solaris2.*

Created attachment 10480
--> https://sourceware.org/bugzilla/attachment.cgi?id=10480&action=edit
Minimal patch

The recent SPARC M7 ADI patches break the gdb build on Solaris/SPARC
(which is a total shame: feels like one side of Oracle doesn't care about what
the
other side is doing ;-(). I'm seeing it on Solaris 10/SPARC, haven't tried
11.3
or 11.4 beta yet:

/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:1876:0: error: "PSR_ICC"
redefined [-Werror]
#define PSR_ICC 0x00f00000
^
In file included from /usr/include/v7/sys/privregs.h:24:0,
from /usr/include/sys/regset.h:420,
from /usr/include/sys/ucontext.h:21,
from /usr/include/sys/signal.h:231,
from /usr/include/sys/procset.h:23,
from /usr/include/sys/wait.h:25,
from /usr/include/stdlib.h:21,
from build-gnulib/import/stdlib.h:36,
from /vol/src/gnu/gdb/gdb/local/gdb/common/common-defs.h:53,
from /vol/src/gnu/gdb/gdb/local/gdb/defs.h:28,
from /vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:20:
/usr/include/v7/sys/psr.h:35:0: note: this is the location of the previous
definition
#define PSR_ICC 0x00F00000 /* integer condition codes */
^
/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:1878:0: error: "PSR_IMPL"
redefined [-Werror]
#define PSR_IMPL 0xf0000000
^
In file included from /usr/include/v7/sys/privregs.h:24:0,
from /usr/include/sys/regset.h:420,
from /usr/include/sys/ucontext.h:21,
from /usr/include/sys/signal.h:231,
from /usr/include/sys/procset.h:23,
from /usr/include/sys/wait.h:25,
from /usr/include/stdlib.h:21,
from build-gnulib/import/stdlib.h:36,
from /vol/src/gnu/gdb/gdb/local/gdb/common/common-defs.h:53,
from /vol/src/gnu/gdb/gdb/local/gdb/defs.h:28,
from /vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:20:
/usr/include/v7/sys/psr.h:41:0: note: this is the location of the previous
definition
#define PSR_IMPL 0xF0000000 /* implementation */
^

/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c: In function ‘int adi_tag_fd()’:
/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:296:63: error: format ‘%d’
expects argument of type ‘int’, but argument 4 has type ‘pid_t {aka long int}’
[-Werror=format=]
snprintf (cl_name, sizeof(cl_name), "/proc/%d/adi/tags", pid);
^
/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c: In function ‘bool
adi_is_addr_mapped(CORE_ADDR, std::size_t)’:
/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:314:64: error: format ‘%d’
expects argument of type ‘int’, but argument 4 has type ‘pid_t {aka long int}’
[-Werror=format=]
snprintf (filename, sizeof filename, "/proc/%d/adi/maps", pid);
^

The attached patch at least allows the build to finish, but there are way
more problems with this code (only from a quick inspection):

* The Solaris/SPARC codes doesn't make use of/support ADI; it's currently
Linux-only.

* Some of the new code in sparc64-tdep.c is Linux-specific, although this is
supposed to be platform-independent code:

** ADI support in procfs is way different from the way used here, which is
Linux-specific.

** Same for ADI support in auxv: none of AT_ADI_BLKSZ, AT_ADI_NBITS, and
AT_ADI_UEONADI exist on Solaris, and the latter is unused beside its
definition.

Rainer

[ro at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=22206

Rainer Orth <ro at gcc dot gnu.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |8.1

[ro at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=22206

Rainer Orth <ro at gcc dot gnu.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
URL| |https://sourceware.org/ml/g
| |db-patches/2017-09/msg00787
| |.html

--- Comment #1 from Rainer Orth <ro at gcc dot gnu.org> ---
Minimal patch posted.

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=22206

--- Comment #2 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Rainer Orth <***@sourceware.org>:

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

commit 39b06c208fb7b7edb98866252cbd05ba0918f666
Author: Rainer Orth <***@CeBiTec.Uni-Bielefeld.DE>
Date: Tue Sep 26 14:58:53 2017 +0200

Fix gdb 8.1 Solaris/SPARC compilation (PR build/22206)

When testing my Solaris < 10 removal patch on Solaris/SPARC, I found
that gdb mainline is currently broken there due to the recent SPARC M7
ADI patches:

/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:1876:0: error: "PSR_ICC"
redefined [-Werror]
#define PSR_ICC 0x00f00000
^
In file included from /usr/include/v7/sys/privregs.h:24:0,
from /usr/include/sys/regset.h:420,
from /usr/include/sys/ucontext.h:21,
from /usr/include/sys/signal.h:231,
from /usr/include/sys/procset.h:23,
from /usr/include/sys/wait.h:25,
from /usr/include/stdlib.h:21,
from build-gnulib/import/stdlib.h:36,
from
/vol/src/gnu/gdb/gdb/local/gdb/common/common-defs.h:53,
from /vol/src/gnu/gdb/gdb/local/gdb/defs.h:28,
from /vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:20:
/usr/include/v7/sys/psr.h:35:0: note: this is the location of the previous
definition
#define PSR_ICC 0x00F00000 /* integer condition codes */
^
/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:1878:0: error: "PSR_IMPL"
redefined [-Werror]
#define PSR_IMPL 0xf0000000
^
In file included from /usr/include/v7/sys/privregs.h:24:0,
from /usr/include/sys/regset.h:420,
from /usr/include/sys/ucontext.h:21,
from /usr/include/sys/signal.h:231,
from /usr/include/sys/procset.h:23,
from /usr/include/sys/wait.h:25,
from /usr/include/stdlib.h:21,
from build-gnulib/import/stdlib.h:36,
from
/vol/src/gnu/gdb/gdb/local/gdb/common/common-defs.h:53,
from /vol/src/gnu/gdb/gdb/local/gdb/defs.h:28,
from /vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:20:
/usr/include/v7/sys/psr.h:41:0: note: this is the location of the previous
definition
#define PSR_IMPL 0xF0000000 /* implementation */
^

Comparing Solaris 11.4 <v7/sys/psr.h> and sparc64-tdep.c, there are more
inconsistencies:

<v7/sys/psr.h>:

#define PSR_S 0x00000080 /* supervisor mode */
#define PSR_ICC 0x00F00000 /* integer condition codes */
#define PSR_VER 0x0F000000 /* mask version */
#define PSR_IMPL 0xF0000000 /* implementation */
#define PSR_RSV 0x000FC000 /* reserved */

sparc64-tdep.c:

#define PSR_S 0x00000080
#define PSR_ICC 0x00f00000
#define PSR_VERS 0x0f000000
#define PSR_IMPL 0xf0000000
#define PSR_V8PLUS 0xff000000
#define PSR_XCC 0x000f0000

Apart from the capitalization differences that trip g++, the names
differ (PSR_VER vs. PSR_VERS), PSR_XCC is included in Solaris' PSR_RSV,
and there's no PSR_V8PLUS on Solaris either.

/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c: In function `int
adi_tag_fd()':
/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:296:63: error: format `%d'
expects argument of type `int', but argument 4 has type `pid_t {aka long int}'
[-Werror=format=]
snprintf (cl_name, sizeof(cl_name), "/proc/%d/adi/tags", pid);
^
/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c: In function `bool
adi_is_addr_mapped(CORE_ADDR, std::size_t)':
/vol/src/gnu/gdb/gdb/local/gdb/sparc64-tdep.c:314:64: error: format `%d'
expects argument of type `int', but argument 4 has type `pid_t {aka long int}'
[-Werror=format=]
snprintf (filename, sizeof filename, "/proc/%d/adi/maps", pid);
^

You cannot always print a pid_t, which can be either int or long on
Solaris, as an int.

Obviously, the ADI patch which modifies code shared between all SPARC
targets, hasn't been tested on anything but Linux/SPARC.

The patch below includes the minimal fixes necessary to unbreak the
Solaris/SPARC build.

However, as detailed in the PR, there's more breakage here: apart from
not bothering to implement ADI support on Solaris, the code contains
several more changes to shared/common SPARC code that are simply wrong
on anything but Linux/SPARC.

The patch was tested on sparcv9-sun-solaris2.10 and
sparcv9-sun-solaris2.11.4 (build and gdb/gdb gdb/gdb smoke test only).

PR build/22206
* sparc64-tdep.c (adi_tag_fd): Print pid as long.
(adi_is_addr_mapped): Likewise.
(PSR_ICC): Don't redefine.
(PSR_IMPL): Likewise.

[ro at CeBiTec dot Uni-Bielefeld.DE]

https://sourceware.org/bugzilla/show_bug.cgi?id=22206

--- Comment #3 from Rainer Orth <ro at CeBiTec dot Uni-Bielefeld.DE> ---
The minimal patch has been installed to unbreak the Solaris/SPARC build.

I'm leaving the PR open for now to figure out how to properly deal with
SPARC M7 ADI support on anything but Linux/SPARC.

Rainer

[brobecker at gnat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=22206

Joel Brobecker <brobecker at gnat dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |brobecker at gnat dot com
Target Milestone|8.1 |---

--- Comment #4 from Joel Brobecker <brobecker at gnat dot com> ---
Hello. First of all, thank you for working on this and the patch your checked
in.

Since the debugger now has your patch and compiled, I removed the target
milestone, as I don't view this as blocking for 8.1 release anymore. If I
missed something, please let me know.

[ro at CeBiTec dot Uni-Bielefeld.DE]

https://sourceware.org/bugzilla/show_bug.cgi?id=22206

--- Comment #5 from Rainer Orth <ro at CeBiTec dot Uni-Bielefeld.DE> ---
No, that's fine. I do have some notes on how to make this work on
Solaris, and even the hardware to test it, but it will probably be some
time.

Rainer

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=22206

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |tromey at sourceware dot org
Component|build |tdep

--- Comment #6 from Tom Tromey <tromey at sourceware dot org> ---
I think this is now a tdep bug rather than a build bug.

[ro at grover dot CeBiTec.Uni-Bielefeld.DE]

https://sourceware.org/bugzilla/show_bug.cgi?id=22206

--- Comment #7 from ro at grover dot CeBiTec.Uni-Bielefeld.DE ---

Post by tromey at sourceware dot org
--- Comment #6 from Tom Tromey <tromey at sourceware dot org> ---
I think this is now a tdep bug rather than a build bug.

It is indeed. I hope to figure out what it takes to make this work
while slowly making my way through a large number of Solaris issues.