[PATCH 0/3] Fix Solaris build with enable-targets=all

[PATCH 0/3] Fix Solaris build with enable-targets=all

[Christian Biesinger via gdb-patches]

I tried compiling gdb on gcc211.fsffrance.org with enable-targets=all
and found a few issues. This patch series fixes the remaining ones.

https://sourceware.org/ml/gdb-patches/2019-12/msg00798.html should be part
of this series too; I apologize for not including it. When I sent that one
I hadn't decided yet to continue working on these build errors and thus
didn't make it a series.

Christian Biesinger (3):
  Undef REG_Y in s12z.h, it may be defined in a system header
  Cast the log10 argument to double to disambiguate it
  Make the literal argument to pow a double, not an integer

 gdb/score-tdep.c      | 4 ++--
 gdb/tui/tui-source.c  | 2 +-
 include/opcode/s12z.h | 4 ++++
 3 files changed, 7 insertions(+), 3 deletions(-)

-- 
2.24.1.735.g03f4e72817-goog

[PATCH 1/3] Undef REG_Y in s12z.h, it may be defined in a system header

[Christian Biesinger via gdb-patches]

This breaks the compile on Solaris with enable-targets=all.

include/ChangeLog:

2019-12-18  Christian Biesinger  <cbiesinger@google.com>

	* opcode/s12z.h: Undef REG_Y.

Change-Id: I028b2663affce50d6ca37ccdc301bde8de80d45c
---
 include/opcode/s12z.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/opcode/s12z.h b/include/opcode/s12z.h
index 7e38ac5e1c..717b4a9cbb 100644
--- a/include/opcode/s12z.h
+++ b/include/opcode/s12z.h
@@ -20,6 +20,10 @@ struct reg
 
 extern const struct reg registers[S12Z_N_REGISTERS];
 
+/* Solaris defines REG_Y in sys/regset.h; undef it here to avoid
+   breaking compilation when this target is enabled.  */
+#undef REG_Y
+
 enum {
     REG_D2 = 0,
     REG_D3,
-- 
2.24.1.735.g03f4e72817-goog

Re: [PATCH 1/3] Undef REG_Y in s12z.h, it may be defined in a system header

[Tom Tromey]

>>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:

Christian> This breaks the compile on Solaris with enable-targets=all.
Christian> include/ChangeLog:

Christian> 2019-12-18  Christian Biesinger  <cbiesinger@google.com>

Christian> 	* opcode/s12z.h: Undef REG_Y.

I think this one should go to the binutils list.

Tom

[PATCH 3/3] Make the literal argument to pow a double, not an integer

[Christian Biesinger via gdb-patches]

Since pow takes doubles, pass 2.0 instead of 2 to pow ().

Conveniently, this fixes the ambiguous call to pow on Solaris 11
with gcc 5.5 (gcc211 on the compile farm), which has a "using std::pow"
directive in a system header, which brings in float/double/long double
overloads.  Fixes the build on Solaris with enable-targets=all.

gdb/ChangeLog:

2019-12-18  Christian Biesinger  <cbiesinger@google.com>

	* score-tdep.c (score7_analyze_prologue): Pass 2.0 instead of
	2 to pow ().

Change-Id: Ib18e7e4749ddcbff0727b72a31198f8cb84d1993
---
 gdb/score-tdep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/score-tdep.c b/gdb/score-tdep.c
index 5ca763c40f..691051729e 100644
--- a/gdb/score-tdep.c
+++ b/gdb/score-tdep.c
@@ -918,13 +918,13 @@ score7_analyze_prologue (CORE_ADDR startaddr, CORE_ADDR pc,
                    && G_FLD (inst->v, 2, 0) == 0x0)
             {
               /* subei! r0, n */
-              sp_offset += (int) pow (2, G_FLD (inst->v, 6, 3));
+              sp_offset += (int) pow (2.0, G_FLD (inst->v, 6, 3));
             }
           else if (G_FLD (inst->v, 14, 7) == 0xc0
                    && G_FLD (inst->v, 2, 0) == 0x0)
             {
               /* addei! r0, n */
-              sp_offset -= (int) pow (2, G_FLD (inst->v, 6, 3));
+              sp_offset -= (int) pow (2.0, G_FLD (inst->v, 6, 3));
             }
         }
       else
-- 
2.24.1.735.g03f4e72817-goog

Re: [PATCH 3/3] Make the literal argument to pow a double, not an integer

[Tom Tromey]

>>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:

Christian> 2019-12-18  Christian Biesinger  <cbiesinger@google.com>

Christian> 	* score-tdep.c (score7_analyze_prologue): Pass 2.0 instead of
Christian> 	2 to pow ().

It probably makes sense to add a comment about Solaris here as well.
It's a bit borderline for me, considering it is a constant, but on the
other hand, a comment is definitely safe.  So, ok with that change.

thanks,
Tom

Re: [PATCH 3/3] Make the literal argument to pow a double, not an integer

[Christian Biesinger via gdb-patches]

On Thu, Dec 19, 2019 at 12:19 PM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:
>
> Christian> 2019-12-18  Christian Biesinger  <cbiesinger@google.com>
>
> Christian>      * score-tdep.c (score7_analyze_prologue): Pass 2.0 instead of
> Christian>      2 to pow ().
>
> It probably makes sense to add a comment about Solaris here as well.
> It's a bit borderline for me, considering it is a constant, but on the
> other hand, a comment is definitely safe.  So, ok with that change.

OK, added this comment:
+             /* Solaris 11+gcc 5.5 has ambiguous overloads of pow, so we
+                pass 2.0 instead of 2 to get the right one.  */

Christian

[PATCH 2/3] Cast the log10 argument to double to disambiguate it

[Christian Biesinger via gdb-patches]

On Solaris 11 with gcc 5.5.0 (gcc211 on the compile farm), math.h has a
using std::log10; directive. This is unfortunate because std::log10 has
overloads for float/double/long double. To disambiguate this call,
cast the argument to double to fix the build.

Change-Id: I6c0c52e9c172b529c899a435d430e5916aeef69f
---
 gdb/tui/tui-source.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
index 32877d7bc8..1274e81674 100644
--- a/gdb/tui/tui-source.c
+++ b/gdb/tui/tui-source.c
@@ -85,7 +85,7 @@ tui_source_window::set_contents (struct gdbarch *arch,
 	  int digits = 0;
 	  if (compact_source)
 	    {
-	      double l = log10 (offsets->size ());
+	      double l = log10 ((double) offsets->size ());
 	      digits = 1 + (int) l;
 	    }
 
-- 
2.24.1.735.g03f4e72817-goog

Re: [PATCH 2/3] Cast the log10 argument to double to disambiguate it

[Eli Zaretskii]

> Date: Wed, 18 Dec 2019 18:01:02 -0600
> From: "Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>
> Cc: Christian Biesinger <cbiesinger@google.com>
> 
> On Solaris 11 with gcc 5.5.0 (gcc211 on the compile farm), math.h has a
> using std::log10; directive. This is unfortunate because std::log10 has
> overloads for float/double/long double. To disambiguate this call,
> cast the argument to double to fix the build.

We may wish to have a comment in the code referring to the original
problem, including perhaps the OS and the compiler versions?
Otherwise the need for this cast is not immediately obvious, IMO.

Thanks.

Re: [PATCH 2/3] Cast the log10 argument to double to disambiguate it

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Wed, 18 Dec 2019 18:01:02 -0600
>> From: "Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>
>> Cc: Christian Biesinger <cbiesinger@google.com>
>> 
>> On Solaris 11 with gcc 5.5.0 (gcc211 on the compile farm), math.h has a
>> using std::log10; directive. This is unfortunate because std::log10 has
>> overloads for float/double/long double. To disambiguate this call,
>> cast the argument to double to fix the build.

Eli> We may wish to have a comment in the code referring to the original
Eli> problem, including perhaps the OS and the compiler versions?
Eli> Otherwise the need for this cast is not immediately obvious, IMO.

Agreed; this is ok with that change.

Tom

Re: [PATCH 2/3] Cast the log10 argument to double to disambiguate it

[Christian Biesinger via gdb-patches]

On Thu, Dec 19, 2019 at 12:17 PM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
>
> >> Date: Wed, 18 Dec 2019 18:01:02 -0600
> >> From: "Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>
> >> Cc: Christian Biesinger <cbiesinger@google.com>
> >>
> >> On Solaris 11 with gcc 5.5.0 (gcc211 on the compile farm), math.h has a
> >> using std::log10; directive. This is unfortunate because std::log10 has
> >> overloads for float/double/long double. To disambiguate this call,
> >> cast the argument to double to fix the build.
>
> Eli> We may wish to have a comment in the code referring to the original
> Eli> problem, including perhaps the OS and the compiler versions?
> Eli> Otherwise the need for this cast is not immediately obvious, IMO.
>
> Agreed; this is ok with that change.

OK, will push with this comment:
      /* Solaris 11+gcc 5.5 has ambiguous overloads of log10, so we
         cast to double to get the right one.  */

Christian

[PATCH] Rename "sun" variable to avoid conflicts on Solaris

[Christian Biesinger via gdb-patches]

A Solaris system header has a #define for "sun".  This renames
that variable to avoid the conflict, fixing a build error with
--enable-targets=all on Solaris.

gdb/ChangeLog:

2019-12-18  Christian Biesinger  <cbiesinger@google.com>

	* fbsd-tdep.c (fbsd_info_proc_files_entry): Rename local var
	"sun" to "saddr_un".

Change-Id: I07a5cd801db1e28ccab8a473ebad74d7afe017c2
---
 gdb/fbsd-tdep.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 937f696f44..d7482d3b58 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -1018,12 +1018,12 @@ fbsd_info_proc_files_entry (int kf_type, int kf_fd, int kf_flags,
 
 	    /* For local sockets, print out the first non-nul path
 	       rather than both paths.  */
-	    const struct fbsd_sockaddr_un *sun
+	    const struct fbsd_sockaddr_un *saddr_un
 	      = reinterpret_cast<const struct fbsd_sockaddr_un *> (kf_sa_local);
-	    if (sun->sun_path[0] == 0)
-	      sun = reinterpret_cast<const struct fbsd_sockaddr_un *>
+	    if (saddr_un->sun_path[0] == 0)
+	      saddr_un = reinterpret_cast<const struct fbsd_sockaddr_un *>
 		(kf_sa_peer);
-	    printf_filtered ("%s", sun->sun_path);
+	    printf_filtered ("%s", saddr_un->sun_path);
 	    break;
 	  }
 	case FBSD_AF_INET:
-- 
2.24.1.735.g03f4e72817-goog

Re: [PATCH 0/3] Fix Solaris build with enable-targets=all

[Christian Biesinger via gdb-patches]

On Wed, Dec 18, 2019 at 6:01 PM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> I tried compiling gdb on gcc211.fsffrance.org with enable-targets=all
> and found a few issues. This patch series fixes the remaining ones.
>
> https://sourceware.org/ml/gdb-patches/2019-12/msg00798.html should be part
> of this series too; I apologize for not including it. When I sent that one
> I hadn't decided yet to continue working on these build errors and thus
> didn't make it a series.
>
> Christian Biesinger (3):
>   Undef REG_Y in s12z.h, it may be defined in a system header
>   Cast the log10 argument to double to disambiguate it
>   Make the literal argument to pow a double, not an integer

Pushed patches 2 and 3 with the comments addressed, and sent patch 1
to binutils@

Christian

>  gdb/score-tdep.c      | 4 ++--
>  gdb/tui/tui-source.c  | 2 +-
>  include/opcode/s12z.h | 4 ++++
>  3 files changed, 7 insertions(+), 3 deletions(-)
>
> --
> 2.24.1.735.g03f4e72817-goog
>

[PATCH] Rename "sun" variable to avoid conflicts on Solaris

[Christian Biesinger via gdb-patches]

A Solaris system header has a #define for "sun".  This renames
that variable to avoid the conflict, fixing a build error with
--enable-targets=all on Solaris.

gdb/ChangeLog:

2019-12-18  Christian Biesinger  <cbiesinger@google.com>

	* fbsd-tdep.c (fbsd_info_proc_files_entry): Rename local var
	"sun" to "saddr_un".

Change-Id: I07a5cd801db1e28ccab8a473ebad74d7afe017c2
---
 gdb/fbsd-tdep.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 937f696f44..d7482d3b58 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -1018,12 +1018,12 @@ fbsd_info_proc_files_entry (int kf_type, int kf_fd, int kf_flags,
 
 	    /* For local sockets, print out the first non-nul path
 	       rather than both paths.  */
-	    const struct fbsd_sockaddr_un *sun
+	    const struct fbsd_sockaddr_un *saddr_un
 	      = reinterpret_cast<const struct fbsd_sockaddr_un *> (kf_sa_local);
-	    if (sun->sun_path[0] == 0)
-	      sun = reinterpret_cast<const struct fbsd_sockaddr_un *>
+	    if (saddr_un->sun_path[0] == 0)
+	      saddr_un = reinterpret_cast<const struct fbsd_sockaddr_un *>
 		(kf_sa_peer);
-	    printf_filtered ("%s", sun->sun_path);
+	    printf_filtered ("%s", saddr_un->sun_path);
 	    break;
 	  }
 	case FBSD_AF_INET:
-- 
2.24.1.735.g03f4e72817-goog

Re: [PATCH] Rename "sun" variable to avoid conflicts on Solaris

[Tom Tromey]

>>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:

Christian> A Solaris system header has a #define for "sun".  This renames
Christian> that variable to avoid the conflict, fixing a build error with
Christian> --enable-targets=all on Solaris.

Christian> gdb/ChangeLog:

Christian> 2019-12-18  Christian Biesinger  <cbiesinger@google.com>

Christian> 	* fbsd-tdep.c (fbsd_info_proc_files_entry): Rename local var
Christian> 	"sun" to "saddr_un".

Thanks, this is ok.

Tom

Re: [PATCH] Rename "sun" variable to avoid conflicts on Solaris

[Christian Biesinger via gdb-patches]

On Thu, Dec 19, 2019 at 12:16 PM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:
>
> Christian> A Solaris system header has a #define for "sun".  This renames
> Christian> that variable to avoid the conflict, fixing a build error with
> Christian> --enable-targets=all on Solaris.
>
> Christian> gdb/ChangeLog:
>
> Christian> 2019-12-18  Christian Biesinger  <cbiesinger@google.com>
>
> Christian>      * fbsd-tdep.c (fbsd_info_proc_files_entry): Rename local var
> Christian>      "sun" to "saddr_un".
>
> Thanks, this is ok.

Thanks, pushed.

To ssh://sourceware.org/git/binutils-gdb.git
   78aa740b76..aa989b27d0  HEAD -> master

Christian