diff mbox

[RFA] Make macOS build warning-free

Message ID 06af645c-3385-7f78-5aa8-d48566c66a45@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi July 3, 2018, 3:57 p.m. UTC 
  On 2018-07-03 09:17 AM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> _INTL_MAY_RETURN_STRING_ARG thing is a gcc/binutils/gdb addition, on top of the
> Simon> v0.12.1 version of gettext.  So we shouldn't have to deal with upstream gettext
> Simon> to fix this, since it's just the gcc/binutils/gdb addition that needs to be fixed.
> 
> It's upstream as well though.  Maybe it was upstreamed from gcc to
> gettext?  I don't really know the history there.
> 
> I'll send a note to the gettext list.

Ah ok I see, it is present in later versions of gettext.  I will follow the discussion on the
gettext mailing to see where it goes.

Grepping and git-blaming the clang source code, it looks like clang has supported the attribute
since at least 2009, and clang has been the default compiler on Mac since about that time too.
So it should be safe to just remove the __APPLE_CC__ #if completely.  If this is fixed upstream,
I think we could just mirror the change in our local copy of libgnuintl.h, without importing a
whole new version of gettext (which would probably be a lot of work).

> Simon> That bit looks good to me, at least to shut up the compiler.  But I'm wondering
> Simon> if we still need that section_offsets structure, or if we could just replace
> Simon> it with std::vector<CORE_ADDR>/gdb:array_view<CORE_ADDR>.
> 
> I think it's a good idea to rewrite section_offsets, but that seems more involved.
> Also the way that this code is using std::vector and then turning it
> into a section_offsets* seems incorrect.

Indeed.  I started a prototype, it uncovered an issue about reread_symbol accessing
objfile::section_offsets out of bounds, so I'm stuck on that.

> I think the symfile.c change isn't enough, btw; the
> -Wno-deprecated-declarations bit is also required.  Otherwise there is at
> least one error (about the use of syscall).  But maybe I could disable that
> one with a #pragma instead - what do you think?

About this part, I would prefer if we disabled the warning at the call sites
instead of disabling the warning across the board, since the warning could be relevant
sometimes.  Or maybe we should look at using a non-deprecated alternative...

Here's the other patch I've been carrying.  If you think it's good, I can finish
it and push it, or you can steal it.


From e458931de993f87702384591701b21edf8299279 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Sun, 25 Jun 2017 15:17:43 -0400
Subject: [PATCH 2/2] Ignore deprecation of sbrk and syscall

---
 gdb/darwin-nat.c      | 3 +++
 gdb/main.c            | 3 +++
 gdb/maint.c           | 6 ++++++
 include/diagnostics.h | 6 ++++++
 4 files changed, 18 insertions(+)

Comments

Tom Tromey July 3, 2018, 5:42 p.m. UTC | #1 
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> Indeed.  I started a prototype, it uncovered an issue about reread_symbol accessing
Simon> objfile::section_offsets out of bounds, so I'm stuck on that.

That sounds bad!

>> I think the symfile.c change isn't enough, btw; the
>> -Wno-deprecated-declarations bit is also required.  Otherwise there is at
>> least one error (about the use of syscall).  But maybe I could disable that
>> one with a #pragma instead - what do you think?

Simon> About this part, I would prefer if we disabled the warning at the call sites
Simon> instead of disabling the warning across the board, since the warning could be relevant
Simon> sometimes.  Or maybe we should look at using a non-deprecated alternative...

Simon> Here's the other patch I've been carrying.  If you think it's good, I can finish
Simon> it and push it, or you can steal it.

Simon> +	  DIAGNOSTIC_PUSH
Simon> +	  DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
Simon>  	  res = syscall (SYS___pthread_kill, thread->gdb_port, nsignal);
Simon> +	  DIAGNOSTIC_POP

I like it but I wonder if it would work with ";" on those lines.
That would make indentation work more nicely in Emacs :)

Simon>  #ifdef HAVE_SBRK
Simon> +DIAGNOSTIC_PUSH
Simon> +DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
Simon>    /* Set this before constructing scoped_command_stats.  */
Simon>    lim_at_start = (char *) sbrk (0);
Simon> +DIAGNOSTIC_POP
Simon>  #endif

Perhaps we need an sbrk wrapper so we can limit the damage to a single spot.

Tom
diff mbox

Patch

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 7dccce7392..b46d8b5632 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -836,7 +836,10 @@  darwin_resume_thread (struct inferior *inf, darwin_thread_t *thread,
 	{
 	  /* Note: ptrace is allowed only if the process is stopped.
 	     Directly send the signal to the thread.  */
+	  DIAGNOSTIC_PUSH
+	  DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
 	  res = syscall (SYS___pthread_kill, thread->gdb_port, nsignal);
+	  DIAGNOSTIC_POP
 	  inferior_debug (4, _("darwin_resume_thread: kill 0x%x %d: %d\n"),
 			  thread->gdb_port, nsignal, res);
 	  thread->signaled = 1;
diff --git a/gdb/main.c b/gdb/main.c
index 9694af2426..0c597685f5 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -488,8 +488,11 @@  captured_main_1 (struct captured_main_args *context)
   struct objfile *objfile;

 #ifdef HAVE_SBRK
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
   /* Set this before constructing scoped_command_stats.  */
   lim_at_start = (char *) sbrk (0);
+DIAGNOSTIC_POP
 #endif

   scoped_command_stats stat_reporter (false);
diff --git a/gdb/maint.c b/gdb/maint.c
index a8a1fcbc29..0b3a3e04e6 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -829,7 +829,10 @@  scoped_command_stats::~scoped_command_stats ()
   if (m_space_enabled && per_command_space)
     {
 #ifdef HAVE_SBRK
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
       char *lim = (char *) sbrk (0);
+DIAGNOSTIC_POP

       long space_now = lim - lim_at_start;
       long space_diff = space_now - m_start_space;
@@ -867,7 +870,10 @@  scoped_command_stats::scoped_command_stats (bool msg_type)
   if (!m_msg_type || per_command_space)
     {
 #ifdef HAVE_SBRK
+DIAGNOSTIC_PUSH
+DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
       char *lim = (char *) sbrk (0);
+DIAGNOSTIC_POP
       m_start_space = lim - lim_at_start;
       m_space_enabled = 1;
 #endif
diff --git a/include/diagnostics.h b/include/diagnostics.h
index 4a674106dc..ecf77969f9 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -35,6 +35,8 @@ 
 #if defined (__clang__) /* clang */

 # define DIAGNOSTIC_IGNORE_SELF_MOVE DIAGNOSTIC_IGNORE ("-Wself-move")
+# define DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS \
+  DIAGNOSTIC_IGNORE ("-Wdeprecated-declarations")
 # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER \
   DIAGNOSTIC_IGNORE ("-Wdeprecated-register")
 # define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
@@ -72,4 +74,8 @@ 
 # define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION
 #endif

+#ifndef DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
+# define DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
+#endif
+
 #endif /* DIAGNOSTICS_H */