[RFA] Make macOS build warning-free

  On 2018-06-29 12:54 PM, Tom Tromey wrote:
> diff --git a/gdb/warning.m4 b/gdb/warning.m4
> index 632cc214ac0..17afc5455a1 100644
> --- a/gdb/warning.m4
> +++ b/gdb/warning.m4
> @@ -58,6 +58,17 @@ case "${host}" in
>      build_warnings="$build_warnings -Wno-unknown-pragmas"
>      # Solaris 11 <unistd.h> marks vfork deprecated.
>      build_warnings="$build_warnings -Wno-deprecated-declarations" ;;
> +  *-*-darwin*)
> +    # macOS deprecates syscall (needed by darwin-nat.c) and
> +    # sbrk (used only for some maint commands).
> +    build_warnings="$build_warnings -Wno-deprecated-declarations"
> +    # -Wformat-nonliteral doesn't work properly on macOS -- apparently
> +    # it does not interact properly with the format_arg attribute.
> +    # (libgnuintl.h disables this attribute for macOS, but re-enabling
> +    # it there did not work.)
> +    build_warnings="$build_warnings -Wno-format-nonliteral"
> +    build_warnings="$build_warnings -Wno-format-security"
> +    ;;
>    *) build_warnings="$build_warnings -Wformat-nonliteral" ;;
>  esac

About the gettext / non-literal warning, this is the patch I carry.  I tried
to dig up why gettext doesn't define _INTL_MAY_RETURN_STRING_ARG for __APPLE_CC__
but couldn't find it.  My guess is that in a distant past, the Apple compiler
did not know about that attribute.  You need another change to get rid of the warning
though.  Either:

1. Apply _INTL_MAY_RETURN_STRING_ARG to the gettext function (see patch below)
2. Remove the other __APPLE_CC__ guard that protects the _INTL_REDIRECT_ASM definition.
   It will make it so the used gettext declaration will be the one at line 138, which
   already has _INTL_MAY_RETURN_STRING_ARG.

Here's the diff I've been carrying for a while:

From c40fdef23fb3c85e94454bb2e3e9559df02cb43c Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Fri, 29 Jun 2018 17:20:06 -0400
Subject: [PATCH 1/2] Fix gettext warnings

---
 intl/libgnuintl.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> About the gettext / non-literal warning, this is the patch I carry.  I tried
Simon> to dig up why gettext doesn't define _INTL_MAY_RETURN_STRING_ARG for __APPLE_CC__
Simon> but couldn't find it.  My guess is that in a distant past, the Apple compiler
Simon> did not know about that attribute.  You need another change to get rid of the warning
Simon> though.

Thanks.

I looked into this a little more.  It seems like a huge effort, because,
first, upstream doesn't have this fix; and second, it seems like it
would have to be imported into gcc first.

Fixing this upstream would probably require the same thing for other
functions, not just gettext.

I think in the meantime I'll just --disable-nls.  Perhaps you wouldn't
mind approving the symfile.c change in isolation.

Tom
  

On 2018-07-02 10:58 AM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> About the gettext / non-literal warning, this is the patch I carry.  I tried
> Simon> to dig up why gettext doesn't define _INTL_MAY_RETURN_STRING_ARG for __APPLE_CC__
> Simon> but couldn't find it.  My guess is that in a distant past, the Apple compiler
> Simon> did not know about that attribute.  You need another change to get rid of the warning
> Simon> though.
> 
> Thanks.
> 
> I looked into this a little more.  It seems like a huge effort, because,
> first, upstream doesn't have this fix; and second, it seems like it
> would have to be imported into gcc first.
> 
> Fixing this upstream would probably require the same thing for other
> functions, not just gettext.

_INTL_MAY_RETURN_STRING_ARG thing is a gcc/binutils/gdb addition, on top of the
v0.12.1 version of gettext.  So we shouldn't have to deal with upstream gettext
to fix this, since it's just the gcc/binutils/gdb addition that needs to be fixed.
Getting the fixed accepted in gcc shouldn't take took long.

> I think in the meantime I'll just --disable-nls.  Perhaps you wouldn't
> mind approving the symfile.c change in isolation.

That bit looks good to me, at least to shut up the compiler.  But I'm wondering
if we still need that section_offsets structure, or if we could just replace
it with std::vector<CORE_ADDR>/gdb:array_view<CORE_ADDR>.

Simon
  

>>>>> "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.

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.

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?

Tom