MinGW compilation warnings in libiberty's include/environ.h

MinGW compilation warnings in libiberty's include/environ.h

[Eli Zaretskii]

When compiling libiberty (as part of GDB) with MinGW on MS-Windows, I
see the following warning:

     gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-4 -g3 -D__USE_MINGW_ACCESS  -I. -I./../include   -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic  -D_GNU_SOURCE ./setenv.c -o setenv.o
     In file included from ./setenv.c:64:0:
     ./../include/environ.h:30:1: warning: function declaration isn't a prototype [-Wstrict-prototypes]
      extern char **environ;
      ^

This was already reported 4 years ago, here:

  https://gcc.gnu.org/ml/gcc-patches/2013-03/msg00471.html

and was solved back then.  But it looks like the offending code was
copied to include/environ.h without the fix, and the warning is thus
back.

The problem is with this code in environ.h:

  #ifndef HAVE_ENVIRON_DECL
  #  ifdef __APPLE__
  #     include <crt_externs.h>
  #     define environ (*_NSGetEnviron ())
  #  else
  extern char **environ;
  #  endif
  #  define HAVE_ENVIRON_DECL
  #endif

which causes the MinGW compiler to see the declaration of environ,
whereas MinGW's stdlib.h has this:

  #ifdef __MSVCRT__
  # define _environ  (*__p__environ())
  extern _CRTIMP __cdecl __MINGW_NOTHROW  char ***__p__environ(void);
  # define _wenviron  (*__p__wenviron())
  extern _CRTIMP __cdecl __MINGW_NOTHROW  wchar_t ***__p__wenviron(void);

  #else  /* ! __MSVCRT__ */
  # ifndef __DECLSPEC_SUPPORTED
  # define _environ (*_imp___environ_dll)
  extern char ***_imp___environ_dll;

  # else  /* __DECLSPEC_SUPPORTED */
  # define _environ  _environ_dll
  __MINGW_IMPORT char ** _environ_dll;
  # endif  /* __DECLSPEC_SUPPORTED */
  #endif  /* ! __MSVCRT__ */

  #define environ _environ

Can this be fixed again, please?  The solution, as back then, is this:

  #ifndef HAVE_ENVIRON_DECL
  #  ifdef __APPLE__
  #     include <crt_externs.h>
  #     define environ (*_NSGetEnviron ())
  #  else
  #     ifndef environ
  extern char **environ;
  #     endif
  #  endif
  #  define HAVE_ENVIRON_DECL
  #endif

Thanks.

Re: MinGW compilation warnings in libiberty's include/environ.h

[Pedro Alves]

On 05/08/2017 04:25 PM, Eli Zaretskii wrote:
> When compiling libiberty (as part of GDB) with MinGW on MS-Windows, I
> see the following warning:
> 
>      gcc -c -DHAVE_CONFIG_H -O2 -gdwarf-4 -g3 -D__USE_MINGW_ACCESS  -I. -I./../include   -W -Wall -Wwrite-strings -Wc++-compat -Wstrict-prototypes -pedantic  -D_GNU_SOURCE ./setenv.c -o setenv.o
>      In file included from ./setenv.c:64:0:
>      ./../include/environ.h:30:1: warning: function declaration isn't a prototype [-Wstrict-prototypes]
>       extern char **environ;
>       ^
> 
> This was already reported 4 years ago, here:
> 
>   https://gcc.gnu.org/ml/gcc-patches/2013-03/msg00471.html
> 
> and was solved back then.  But it looks like the offending code was
> copied to include/environ.h without the fix, and the warning is thus
> back.
> 
> The problem is with this code in environ.h:
> 
>   #ifndef HAVE_ENVIRON_DECL
>   #  ifdef __APPLE__
>   #     include <crt_externs.h>
>   #     define environ (*_NSGetEnviron ())
>   #  else
>   extern char **environ;
>   #  endif
>   #  define HAVE_ENVIRON_DECL
>   #endif
> 
> which causes the MinGW compiler to see the declaration of environ,
> whereas MinGW's stdlib.h has this:
> 
>   #ifdef __MSVCRT__
>   # define _environ  (*__p__environ())
>   extern _CRTIMP __cdecl __MINGW_NOTHROW  char ***__p__environ(void);
>   # define _wenviron  (*__p__wenviron())
>   extern _CRTIMP __cdecl __MINGW_NOTHROW  wchar_t ***__p__wenviron(void);
> 
>   #else  /* ! __MSVCRT__ */
>   # ifndef __DECLSPEC_SUPPORTED
>   # define _environ (*_imp___environ_dll)
>   extern char ***_imp___environ_dll;
> 
>   # else  /* __DECLSPEC_SUPPORTED */
>   # define _environ  _environ_dll
>   __MINGW_IMPORT char ** _environ_dll;
>   # endif  /* __DECLSPEC_SUPPORTED */
>   #endif  /* ! __MSVCRT__ */
> 
>   #define environ _environ

So again there's a system header that defines the symbol
but for some reason libiberty still wants to declare/define
it is if it weren't?

That sounds to me like the root issue that should be fixed,
so that these fallback definitions don't come into into play at all.
I.e., why isn't HAVE_ENVIRON_DECL defined on mingw when
setenv.o is built?  Sounds like a decl check is missing
in configure.ac.

Thanks,
Pedro Alves

Re: MinGW compilation warnings in libiberty's include/environ.h

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 19 May 2017 16:51:30 +0100
> 
> So again there's a system header that defines the symbol
> but for some reason libiberty still wants to declare/define
> it is if it weren't?

Yes.  AFAICS, libiberty's configure script doesn't check the
declaration, it only probes the setenv function itself.  You can see
that the cpp directives around the environ declarations are
OS-dependent rather than based on autoconf tests.

> That sounds to me like the root issue that should be fixed,
> so that these fallback definitions don't come into into play at all.
> I.e., why isn't HAVE_ENVIRON_DECL defined on mingw when
> setenv.o is built?  Sounds like a decl check is missing
> in configure.ac.

Most probably, yes.

Re: MinGW compilation warnings in libiberty's include/environ.h

[DJ Delorie]

Fix committed.  As Pedro noted, the correct way to request a change is
to make the change in your local checked out repo, and run "svn diff"
(or "git diff").  That makes sure that the fix is what you intended, and
that you don't have other unexpected changes (especially in regenerated
files, like configure).  It also checks against a patch against a stale
copy of the sources; if your patch doesn't apply that's a hint to the
maintainer that the patch needs to be looked at more closely.

The easier you can make it for the maintainer (me!), the more likely
your patch will get handled quickly.

Re: MinGW compilation warnings in libiberty's include/environ.h

[DJ Delorie]

Pedro Alves <palves@redhat.com> writes:
> That sounds to me like the root issue that should be fixed,
> so that these fallback definitions don't come into into play at all.
> I.e., why isn't HAVE_ENVIRON_DECL defined on mingw when
> setenv.o is built?  Sounds like a decl check is missing
> in configure.ac.

environ is tricky because it's typically messy on platforms, unlike a
standard C function.  You can't use a generic check if the macro expands
to something that interferes with the check.

Re: MinGW compilation warnings in libiberty's include/environ.h

[Eli Zaretskii]

> From: DJ Delorie <dj@redhat.com>
> Cc: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> Date: Fri, 19 May 2017 21:25:56 -0400
> 
> Fix committed.

Thanks!

Pedro, how do we make this propagated to the GDB repository?

> As Pedro noted, the correct way to request a change is
> to make the change in your local checked out repo, and run "svn diff"
> (or "git diff").

My problem is, I don't have a GCC repository, so doing the above means
checking it out, which takes a _very_ long time here.  And the patch I
was thinking about is not the one you eventually committed anyway,
which is to be expected, since I have no idea about the various
dependencies between the projects that use the common configure
infrastructure and about how things are done to keep all the user
projects in sync.

IOW, not everyone who reports a problem can necessarily provide a
patch.  The fact that you know too much about my abilities in other
projects doesn't (or shouldn't) change that ;-)

> The easier you can make it for the maintainer (me!), the more likely
> your patch will get handled quickly.

I know.  I was trying to make it as easy as I possibly could, given
the level of my knowledge in this area.

Re: MinGW compilation warnings in libiberty's include/environ.h

[DJ Delorie]

Eli Zaretskii <eliz@gnu.org> writes:
> My problem is, I don't have a GCC repository, so doing the above means

In this case, a gdb repo would have sufficed.

> IOW, not everyone who reports a problem can necessarily provide a
> patch.  The fact that you know too much about my abilities in other
> projects doesn't (or shouldn't) change that ;-)

:-)

Re: MinGW compilation warnings in libiberty's include/environ.h

[Eli Zaretskii]

> From: DJ Delorie <dj@redhat.com>
> Cc: palves@redhat.com, gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org
> Date: Sat, 20 May 2017 02:16:14 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > My problem is, I don't have a GCC repository, so doing the above means
> 
> In this case, a gdb repo would have sufficed.

Thanks, I didn't know that.

Re: MinGW compilation warnings in libiberty's include/environ.h

[Pedro Alves]

On 05/20/2017 02:27 AM, DJ Delorie wrote:
> 
> Pedro Alves <palves@redhat.com> writes:
>> That sounds to me like the root issue that should be fixed,
>> so that these fallback definitions don't come into into play at all.
>> I.e., why isn't HAVE_ENVIRON_DECL defined on mingw when
>> setenv.o is built?  Sounds like a decl check is missing
>> in configure.ac.
> 
> environ is tricky because it's typically messy on platforms, unlike a
> standard C function.  You can't use a generic check if the macro expands
> to something that interferes with the check.

gnulib has a check, which I assume to be solid:

 http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=m4/environ.m4

we could import just that bit, I suppose, though every client
of libiberty's environ.h would need to gain the same check.
That's be quite doable with the shared libiberty.m4 idea (we'd just
put the check there), but a nuisance if you have to copy the check all
over the place.  At this point it may be more worth it to
invest in finishing the previous use-gnulib-in-gcc effort instead.

Thanks,
Pedro Alves