[v3,4/5] sim: Check known getopt definition existence

  Clang generates a warning if there is a function declaration/definition
with zero arguments.  Such declarations/definitions without a prototype (an
argument list) are deprecated forms of indefinite arguments
("-Wdeprecated-non-prototype").  On the default configuration, it causes a
build failure (unless "--disable-werror" is specified).

include/getopt.h defines some getopt function definitions but one of them
has a form "extern int getopt ();".  If this form is selected in
include/getopt.h, Clang generates a warning and the build fails by default.

In really old environments, this getopt definition with no arguments is
necessary (because the definition may change between environments).
However, this definition is now a cause of problems on modern environments.

A good news is, this definition is not always selected (e.g. if used by
binutils/*.c).  This is because configuration scripts of binutils, gas,
gprof and ld tries to find known definition of getopt function is used and
defines HAVE_DECL_GETOPT macro.  If this macro is defined when getopt.h is
included, a good form of getopt is used and Clang won't generate warnings.

This commit adds a modified portion of ld/configure.ac to find the known
getopt definition.  If we could find one (and we *will* in most modern
environments), we don't need to rely on the deprecated definition.
---
 sim/config.h.in  |  3 +++
 sim/configure    | 32 ++++++++++++++++++++++++++++++++
 sim/configure.ac | 10 ++++++++++
 3 files changed, 45 insertions(+)
  

On 10/6/22 08:43, Tsukasa OI via Gdb-patches wrote:
> Clang generates a warning if there is a function declaration/definition
> with zero arguments.  Such declarations/definitions without a prototype (an
> argument list) are deprecated forms of indefinite arguments
> ("-Wdeprecated-non-prototype").  On the default configuration, it causes a
> build failure (unless "--disable-werror" is specified).
> 
> include/getopt.h defines some getopt function definitions but one of them
> has a form "extern int getopt ();".  If this form is selected in
> include/getopt.h, Clang generates a warning and the build fails by default.
> 
> In really old environments, this getopt definition with no arguments is
> necessary (because the definition may change between environments).
> However, this definition is now a cause of problems on modern environments.
> 
> A good news is, this definition is not always selected (e.g. if used by
> binutils/*.c).  This is because configuration scripts of binutils, gas,
> gprof and ld tries to find known definition of getopt function is used and
> defines HAVE_DECL_GETOPT macro.  If this macro is defined when getopt.h is
> included, a good form of getopt is used and Clang won't generate warnings.
> 
> This commit adds a modified portion of ld/configure.ac to find the known
> getopt definition.  If we could find one (and we *will* in most modern
> environments), we don't need to rely on the deprecated definition.

I'm guessing this cause the build breakage on buildbot gdb-centos-x86_64 .

https://builder.sourceware.org/buildbot/#/builders/71/builds/1392

Thanks,
- Tom


> ---
>   sim/config.h.in  |  3 +++
>   sim/configure    | 32 ++++++++++++++++++++++++++++++++
>   sim/configure.ac | 10 ++++++++++
>   3 files changed, 45 insertions(+)
> 
> diff --git a/sim/config.h.in b/sim/config.h.in
> index 84c363c0aec..9a94b289e46 100644
> --- a/sim/config.h.in
> +++ b/sim/config.h.in
> @@ -41,6 +41,9 @@
>   /* Define to 1 if you have the `chmod' function. */
>   #undef HAVE_CHMOD
>   
> +/* Is the prototype for getopt in <unistd.h> in the expected format? */
> +#undef HAVE_DECL_GETOPT
> +
>   /* Define to 1 if you have the declaration of `tzname', and to 0 if you don't.
>      */
>   #undef HAVE_DECL_TZNAME
> diff --git a/sim/configure b/sim/configure
> index 75d1935df38..dac7f085be1 100755
> --- a/sim/configure
> +++ b/sim/configure
> @@ -16428,6 +16428,38 @@ $as_echo "${WARN_CFLAGS} ${WERROR_CFLAGS}" >&6; }
>   fi
>   
>   
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for a known getopt prototype in unistd.h" >&5
> +$as_echo_n "checking for a known getopt prototype in unistd.h... " >&6; }
> +if ${sim_cv_decl_getopt_unistd_h+:} false; then :
> +  $as_echo_n "(cached) " >&6
> +else
> +  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h.  */
> +#include <unistd.h>
> +int
> +main ()
> +{
> +extern int getopt (int, char *const*, const char *);
> +  ;
> +  return 0;
> +}
> +_ACEOF
> +if ac_fn_c_try_compile "$LINENO"; then :
> +  sim_cv_decl_getopt_unistd_h=yes
> +else
> +  sim_cv_decl_getopt_unistd_h=no
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> +fi
> +
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $sim_cv_decl_getopt_unistd_h" >&5
> +$as_echo "$sim_cv_decl_getopt_unistd_h" >&6; }
> +if test $sim_cv_decl_getopt_unistd_h = yes; then
> +
> +$as_echo "#define HAVE_DECL_GETOPT 1" >>confdefs.h
> +
> +fi
> +
>   
>   
>   
> diff --git a/sim/configure.ac b/sim/configure.ac
> index 66a1020efe0..be0cfdbea32 100644
> --- a/sim/configure.ac
> +++ b/sim/configure.ac
> @@ -177,6 +177,16 @@ SIM_AC_OPTION_STDIO
>   SIM_AC_OPTION_TRACE
>   SIM_AC_OPTION_WARNINGS
>   
> +AC_MSG_CHECKING(for a known getopt prototype in unistd.h)
> +AC_CACHE_VAL(sim_cv_decl_getopt_unistd_h,
> +[AC_COMPILE_IFELSE([AC_LANG_PROGRAM([#include <unistd.h>], [extern int getopt (int, char *const*, const char *);])],
> +sim_cv_decl_getopt_unistd_h=yes, sim_cv_decl_getopt_unistd_h=no)])
> +AC_MSG_RESULT($sim_cv_decl_getopt_unistd_h)
> +if test $sim_cv_decl_getopt_unistd_h = yes; then
> +  AC_DEFINE([HAVE_DECL_GETOPT], 1,
> +	    [Is the prototype for getopt in <unistd.h> in the expected format?])
> +fi
> +
>   dnl These are unfortunate.  They are conditionally called by other sim macros
>   dnl but always used by common/Make-common.in.  So we have to subst here even
>   dnl when the rest of the code is in the respective macros.  Once we merge the
  

Hi Tom,

On 2022/10/13 1:28, Tom de Vries wrote:
> On 10/6/22 08:43, Tsukasa OI via Gdb-patches wrote:
>> Clang generates a warning if there is a function declaration/definition
>> with zero arguments.  Such declarations/definitions without a
>> prototype (an
>> argument list) are deprecated forms of indefinite arguments
>> ("-Wdeprecated-non-prototype").  On the default configuration, it
>> causes a
>> build failure (unless "--disable-werror" is specified).
>>
>> include/getopt.h defines some getopt function definitions but one of them
>> has a form "extern int getopt ();".  If this form is selected in
>> include/getopt.h, Clang generates a warning and the build fails by
>> default.
>>
>> In really old environments, this getopt definition with no arguments is
>> necessary (because the definition may change between environments).
>> However, this definition is now a cause of problems on modern
>> environments.
>>
>> A good news is, this definition is not always selected (e.g. if used by
>> binutils/*.c).  This is because configuration scripts of binutils, gas,
>> gprof and ld tries to find known definition of getopt function is used
>> and
>> defines HAVE_DECL_GETOPT macro.  If this macro is defined when
>> getopt.h is
>> included, a good form of getopt is used and Clang won't generate
>> warnings.
>>
>> This commit adds a modified portion of ld/configure.ac to find the known
>> getopt definition.  If we could find one (and we *will* in most modern
>> environments), we don't need to rely on the deprecated definition.
> 
> I'm guessing this cause the build breakage on buildbot gdb-centos-x86_64 .
> 
> https://builder.sourceware.org/buildbot/#/builders/71/builds/1392
> 
> Thanks,
> - Tom

I didn't see that error and it seems this error is very strange
considering sim/m32c/main.c is the **only** file which should not be
affected by my change as long as <unistd.h> correctly declares getopt.

Adding
#include <getopt.h>
after
#include <sys/types.h>
might change something but I cannot guarantee whether this change would
fix the issue.

Thanks,
Tsukasa

> 
> 
>> ---
>>   sim/config.h.in  |  3 +++
>>   sim/configure    | 32 ++++++++++++++++++++++++++++++++
>>   sim/configure.ac | 10 ++++++++++
>>   3 files changed, 45 insertions(+)
>>
>> diff --git a/sim/config.h.in b/sim/config.h.in
>> index 84c363c0aec..9a94b289e46 100644
>> --- a/sim/config.h.in
>> +++ b/sim/config.h.in
>> @@ -41,6 +41,9 @@
>>   /* Define to 1 if you have the `chmod' function. */
>>   #undef HAVE_CHMOD
>>   +/* Is the prototype for getopt in <unistd.h> in the expected
>> format? */
>> +#undef HAVE_DECL_GETOPT
>> +
>>   /* Define to 1 if you have the declaration of `tzname', and to 0 if
>> you don't.
>>      */
>>   #undef HAVE_DECL_TZNAME
>> diff --git a/sim/configure b/sim/configure
>> index 75d1935df38..dac7f085be1 100755
>> --- a/sim/configure
>> +++ b/sim/configure
>> @@ -16428,6 +16428,38 @@ $as_echo "${WARN_CFLAGS} ${WERROR_CFLAGS}"
>> >&6; }
>>   fi
>>     +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for a known
>> getopt prototype in unistd.h" >&5
>> +$as_echo_n "checking for a known getopt prototype in unistd.h... "
>> >&6; }
>> +if ${sim_cv_decl_getopt_unistd_h+:} false; then :
>> +  $as_echo_n "(cached) " >&6
>> +else
>> +  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>> +/* end confdefs.h.  */
>> +#include <unistd.h>
>> +int
>> +main ()
>> +{
>> +extern int getopt (int, char *const*, const char *);
>> +  ;
>> +  return 0;
>> +}
>> +_ACEOF
>> +if ac_fn_c_try_compile "$LINENO"; then :
>> +  sim_cv_decl_getopt_unistd_h=yes
>> +else
>> +  sim_cv_decl_getopt_unistd_h=no
>> +fi
>> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>> +fi
>> +
>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
>> $sim_cv_decl_getopt_unistd_h" >&5
>> +$as_echo "$sim_cv_decl_getopt_unistd_h" >&6; }
>> +if test $sim_cv_decl_getopt_unistd_h = yes; then
>> +
>> +$as_echo "#define HAVE_DECL_GETOPT 1" >>confdefs.h
>> +
>> +fi
>> +
>>       diff --git a/sim/configure.ac b/sim/configure.ac
>> index 66a1020efe0..be0cfdbea32 100644
>> --- a/sim/configure.ac
>> +++ b/sim/configure.ac
>> @@ -177,6 +177,16 @@ SIM_AC_OPTION_STDIO
>>   SIM_AC_OPTION_TRACE
>>   SIM_AC_OPTION_WARNINGS
>>   +AC_MSG_CHECKING(for a known getopt prototype in unistd.h)
>> +AC_CACHE_VAL(sim_cv_decl_getopt_unistd_h,
>> +[AC_COMPILE_IFELSE([AC_LANG_PROGRAM([#include <unistd.h>], [extern
>> int getopt (int, char *const*, const char *);])],
>> +sim_cv_decl_getopt_unistd_h=yes, sim_cv_decl_getopt_unistd_h=no)])
>> +AC_MSG_RESULT($sim_cv_decl_getopt_unistd_h)
>> +if test $sim_cv_decl_getopt_unistd_h = yes; then
>> +  AC_DEFINE([HAVE_DECL_GETOPT], 1,
>> +        [Is the prototype for getopt in <unistd.h> in the expected
>> format?])
>> +fi
>> +
>>   dnl These are unfortunate.  They are conditionally called by other
>> sim macros
>>   dnl but always used by common/Make-common.in.  So we have to subst
>> here even
>>   dnl when the rest of the code is in the respective macros.  Once we
>> merge the
>
  

On 10/12/22 19:03, Tsukasa OI wrote:
> Adding
> #include <getopt.h>
> after
> #include <sys/types.h>
> might change something but I cannot guarantee whether this change would
> fix the issue.

Ack, I'll try attached patch on the try-buildbot.

Thanks,
- Tom
  

On 10/12/22 19:08, Tom de Vries wrote:
> On 10/12/22 19:03, Tsukasa OI wrote:
>> Adding
>> #include <getopt.h>
>> after
>> #include <sys/types.h>
>> might change something but I cannot guarantee whether this change would
>> fix the issue.
> 
> Ack, I'll try attached patch on the try-buildbot.
> 

I've tried it out, and it still fails the same:
...
/srv/buildbot/worker/gdb-try-centos-x86_64/gdb-build/sim/../../binutils-gdb/sim/m32c/main.c: 
In function ‘main’:
/srv/buildbot/worker/gdb-try-centos-x86_64/gdb-build/sim/../../binutils-gdb/sim/m32c/main.c:144:3: 
error: implicit declaration of function ‘getopt’ 
[-Werror=implicit-function-declaration]
    while ((o = getopt (argc, argv, "tc:vdm:C")) != -1)
    ^
cc1: all warnings being treated as errors
make[3]: *** [main.o] Error 1
make[3]: Leaving directory 
`/srv/buildbot/worker/gdb-try-centos-x86_64/gdb-build/sim/m32c'
...

Thanks,
- Tom
  

On 2022/10/13 1:28, Tom de Vries wrote:
> On 10/6/22 08:43, Tsukasa OI via Gdb-patches wrote:
>> Clang generates a warning if there is a function declaration/definition
>> with zero arguments.? Such declarations/definitions without a
>> prototype (an
>> argument list) are deprecated forms of indefinite arguments
>> ("-Wdeprecated-non-prototype").? On the default configuration, it
>> causes a
>> build failure (unless "--disable-werror" is specified).
>>
>> include/getopt.h defines some getopt function definitions but one of them
>> has a form "extern int getopt ();".? If this form is selected in
>> include/getopt.h, Clang generates a warning and the build fails by
>> default.
>>
>> In really old environments, this getopt definition with no arguments is
>> necessary (because the definition may change between environments).
>> However, this definition is now a cause of problems on modern
>> environments.
>>
>> A good news is, this definition is not always selected (e.g. if used by
>> binutils/*.c).? This is because configuration scripts of binutils, gas,
>> gprof and ld tries to find known definition of getopt function is used
>> and
>> defines HAVE_DECL_GETOPT macro.? If this macro is defined when
>> getopt.h is
>> included, a good form of getopt is used and Clang won't generate
>> warnings.
>>
>> This commit adds a modified portion of ld/configure.ac to find the known
>> getopt definition.? If we could find one (and we *will* in most modern
>> environments), we don't need to rely on the deprecated definition.
> 
> I'm guessing this cause the build breakage on buildbot gdb-centos-x86_64 .
> 
> https://builder.sourceware.org/buildbot/#/builders/71/builds/1392
> 
> Thanks,
> - Tom

Hi Tom,

I finally found a cause.  First of all, this is reproduced with
following configurations (examples are selected to minimize time to
reproduce):

-   CentOS 7 (x86_64; with GNU libc 2.17)
-   "make all-sim" with following configurations (for example):
    -   --target=m32c-unknown-elf
    -   --target=rl78-unknown-elf

And it was true that my commit (as you pointed out) was the initial bad
commit but the real cause was much complex than I expected.  The reason
I could not initially reproduce the issue was because it required GNU
libc <= 2.25 to reproduce.

An interesting fact is... standard unistd.h on GNU libc <= 2.25 includes
<getopt.h> (with __need_getopt macro defined) but it actually includes
$(srcdir)/include/getopt.h, not getopt.h in GNU libc.  Since my change
defined HAVE_DECL_GETOPT to 1 on GNU libc-based environment and
declaration of the getopt function is suppressed, causing an error.

On GNU libc >= 2.26, unistd.h includes <bits/getopt_posix.h> without
defining __need_getopt and that's why this error is suppressed on newer
GNU libc-based environments.


Possible reason why this bug is missed is, there are not so many getopt
callers (many calls getopt_long or getopt_long_only, not getopt).

True getopt callers on Binutils/GDB/GCC are following:

-   M32C simulator (affected by my change)
-   RL78 simulator (affected by my change)
-   gprofng (getopt declaration not checked by gprofng/configure)

... yes, GCC (entirely) and the most of Binutils components do not
depend on getopt function anymore.  This fact explains why this bug is
not discovered so long.


The true fix to this is going to be applied to include/getopt.h
(Binutils) to detect this include path on GNU libc <= 2.25.

Aside from this, some sim source files need to be changed (to minimize
problems even further).  I'll submit these patchsets soon.

Thanks,
Tsukasa

> 
> 
>> ---
>> ? sim/config.h.in? |? 3 +++
>> ? sim/configure??? | 32 ++++++++++++++++++++++++++++++++
>> ? sim/configure.ac | 10 ++++++++++
>> ? 3 files changed, 45 insertions(+)
>>
>> diff --git a/sim/config.h.in b/sim/config.h.in
>> index 84c363c0aec..9a94b289e46 100644
>> --- a/sim/config.h.in
>> +++ b/sim/config.h.in
>> @@ -41,6 +41,9 @@
>> ? /* Define to 1 if you have the `chmod' function. */
>> ? #undef HAVE_CHMOD
>> ? +/* Is the prototype for getopt in <unistd.h> in the expected
>> format? */
>> +#undef HAVE_DECL_GETOPT
>> +
>> ? /* Define to 1 if you have the declaration of `tzname', and to 0 if
>> you don't.
>> ???? */
>> ? #undef HAVE_DECL_TZNAME
>> diff --git a/sim/configure b/sim/configure
>> index 75d1935df38..dac7f085be1 100755
>> --- a/sim/configure
>> +++ b/sim/configure
>> @@ -16428,6 +16428,38 @@ $as_echo "${WARN_CFLAGS} ${WERROR_CFLAGS}"
>> >&6; }
>> ? fi
>> ? ? +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for a known
>> getopt prototype in unistd.h" >&5
>> +$as_echo_n "checking for a known getopt prototype in unistd.h... "
>> >&6; }
>> +if ${sim_cv_decl_getopt_unistd_h+:} false; then :
>> +? $as_echo_n "(cached) " >&6
>> +else
>> +? cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>> +/* end confdefs.h.? */
>> +#include <unistd.h>
>> +int
>> +main ()
>> +{
>> +extern int getopt (int, char *const*, const char *);
>> +? ;
>> +? return 0;
>> +}
>> +_ACEOF
>> +if ac_fn_c_try_compile "$LINENO"; then :
>> +? sim_cv_decl_getopt_unistd_h=yes
>> +else
>> +? sim_cv_decl_getopt_unistd_h=no
>> +fi
>> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>> +fi
>> +
>> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
>> $sim_cv_decl_getopt_unistd_h" >&5
>> +$as_echo "$sim_cv_decl_getopt_unistd_h" >&6; }
>> +if test $sim_cv_decl_getopt_unistd_h = yes; then
>> +
>> +$as_echo "#define HAVE_DECL_GETOPT 1" >>confdefs.h
>> +
>> +fi
>> +
>> ? ? ? diff --git a/sim/configure.ac b/sim/configure.ac
>> index 66a1020efe0..be0cfdbea32 100644
>> --- a/sim/configure.ac
>> +++ b/sim/configure.ac
>> @@ -177,6 +177,16 @@ SIM_AC_OPTION_STDIO
>> ? SIM_AC_OPTION_TRACE
>> ? SIM_AC_OPTION_WARNINGS
>> ? +AC_MSG_CHECKING(for a known getopt prototype in unistd.h)
>> +AC_CACHE_VAL(sim_cv_decl_getopt_unistd_h,
>> +[AC_COMPILE_IFELSE([AC_LANG_PROGRAM([#include <unistd.h>], [extern
>> int getopt (int, char *const*, const char *);])],
>> +sim_cv_decl_getopt_unistd_h=yes, sim_cv_decl_getopt_unistd_h=no)])
>> +AC_MSG_RESULT($sim_cv_decl_getopt_unistd_h)
>> +if test $sim_cv_decl_getopt_unistd_h = yes; then
>> +? AC_DEFINE([HAVE_DECL_GETOPT], 1,
>> +??????? [Is the prototype for getopt in <unistd.h> in the expected
>> format?])
>> +fi
>> +
>> ? dnl These are unfortunate.? They are conditionally called by other
>> sim macros
>> ? dnl but always used by common/Make-common.in.? So we have to subst
>> here even
>> ? dnl when the rest of the code is in the respective macros.? Once we
>> merge the
>
  

On 06 Oct 2022 06:43, Tsukasa OI wrote:
> Clang generates a warning if there is a function declaration/definition
> with zero arguments.  Such declarations/definitions without a prototype (an
> argument list) are deprecated forms of indefinite arguments
> ("-Wdeprecated-non-prototype").  On the default configuration, it causes a
> build failure (unless "--disable-werror" is specified).
> 
> include/getopt.h defines some getopt function definitions but one of them
> has a form "extern int getopt ();".  If this form is selected in
> include/getopt.h, Clang generates a warning and the build fails by default.
> 
> In really old environments, this getopt definition with no arguments is
> necessary (because the definition may change between environments).
> However, this definition is now a cause of problems on modern environments.
> 
> A good news is, this definition is not always selected (e.g. if used by
> binutils/*.c).  This is because configuration scripts of binutils, gas,
> gprof and ld tries to find known definition of getopt function is used and
> defines HAVE_DECL_GETOPT macro.  If this macro is defined when getopt.h is
> included, a good form of getopt is used and Clang won't generate warnings.
> 
> This commit adds a modified portion of ld/configure.ac to find the known
> getopt definition.  If we could find one (and we *will* in most modern
> environments), we don't need to rely on the deprecated definition.
> ---
>  sim/config.h.in  |  3 +++
>  sim/configure    | 32 ++++++++++++++++++++++++++++++++
>  sim/configure.ac | 10 ++++++++++

this logic belongs in m4/sim_ac_platform.m4, not configure.ac

should leave a comment above the code too indicating that this logic is
purely for local getopt.h usage, and is copied from other dirs (e.g. the
binutils files).
-mike
  

On 2022/10/23 21:16, Mike Frysinger wrote:
> On 06 Oct 2022 06:43, Tsukasa OI wrote:
>> Clang generates a warning if there is a function declaration/definition
>> with zero arguments.  Such declarations/definitions without a prototype (an
>> argument list) are deprecated forms of indefinite arguments
>> ("-Wdeprecated-non-prototype").  On the default configuration, it causes a
>> build failure (unless "--disable-werror" is specified).
>>
>> include/getopt.h defines some getopt function definitions but one of them
>> has a form "extern int getopt ();".  If this form is selected in
>> include/getopt.h, Clang generates a warning and the build fails by default.
>>
>> In really old environments, this getopt definition with no arguments is
>> necessary (because the definition may change between environments).
>> However, this definition is now a cause of problems on modern environments.
>>
>> A good news is, this definition is not always selected (e.g. if used by
>> binutils/*.c).  This is because configuration scripts of binutils, gas,
>> gprof and ld tries to find known definition of getopt function is used and
>> defines HAVE_DECL_GETOPT macro.  If this macro is defined when getopt.h is
>> included, a good form of getopt is used and Clang won't generate warnings.
>>
>> This commit adds a modified portion of ld/configure.ac to find the known
>> getopt definition.  If we could find one (and we *will* in most modern
>> environments), we don't need to rely on the deprecated definition.
>> ---
>>  sim/config.h.in  |  3 +++
>>  sim/configure    | 32 ++++++++++++++++++++++++++++++++
>>  sim/configure.ac | 10 ++++++++++
> 
> this logic belongs in m4/sim_ac_platform.m4, not configure.ac
> 
> should leave a comment above the code too indicating that this logic is
> purely for local getopt.h usage, and is copied from other dirs (e.g. the
> binutils files).
> -mike

OK, I'll submit a patch to move this portion to
sim/m4/sim_ac_platform.m4 with proper commit message.

Thanks,
Tsukasa
  

On 27 Oct 2022 11:02, Tsukasa OI wrote:
> On 2022/10/23 21:16, Mike Frysinger wrote:
> > On 06 Oct 2022 06:43, Tsukasa OI wrote:
> >> Clang generates a warning if there is a function declaration/definition
> >> with zero arguments.  Such declarations/definitions without a prototype (an
> >> argument list) are deprecated forms of indefinite arguments
> >> ("-Wdeprecated-non-prototype").  On the default configuration, it causes a
> >> build failure (unless "--disable-werror" is specified).
> >>
> >> include/getopt.h defines some getopt function definitions but one of them
> >> has a form "extern int getopt ();".  If this form is selected in
> >> include/getopt.h, Clang generates a warning and the build fails by default.
> >>
> >> In really old environments, this getopt definition with no arguments is
> >> necessary (because the definition may change between environments).
> >> However, this definition is now a cause of problems on modern environments.
> >>
> >> A good news is, this definition is not always selected (e.g. if used by
> >> binutils/*.c).  This is because configuration scripts of binutils, gas,
> >> gprof and ld tries to find known definition of getopt function is used and
> >> defines HAVE_DECL_GETOPT macro.  If this macro is defined when getopt.h is
> >> included, a good form of getopt is used and Clang won't generate warnings.
> >>
> >> This commit adds a modified portion of ld/configure.ac to find the known
> >> getopt definition.  If we could find one (and we *will* in most modern
> >> environments), we don't need to rely on the deprecated definition.
> >> ---
> >>  sim/config.h.in  |  3 +++
> >>  sim/configure    | 32 ++++++++++++++++++++++++++++++++
> >>  sim/configure.ac | 10 ++++++++++
> > 
> > this logic belongs in m4/sim_ac_platform.m4, not configure.ac
> > 
> > should leave a comment above the code too indicating that this logic is
> > purely for local getopt.h usage, and is copied from other dirs (e.g. the
> > binutils files).
> 
> OK, I'll submit a patch to move this portion to
> sim/m4/sim_ac_platform.m4 with proper commit message.

have you had a chance to fix this up yet ?
-mike
  

On 2023/01/03 12:12, Mike Frysinger wrote:
> On 27 Oct 2022 11:02, Tsukasa OI wrote:
>> On 2022/10/23 21:16, Mike Frysinger wrote:
>>> On 06 Oct 2022 06:43, Tsukasa OI wrote:
>>>> Clang generates a warning if there is a function declaration/definition
>>>> with zero arguments.  Such declarations/definitions without a prototype (an
>>>> argument list) are deprecated forms of indefinite arguments
>>>> ("-Wdeprecated-non-prototype").  On the default configuration, it causes a
>>>> build failure (unless "--disable-werror" is specified).
>>>>
>>>> include/getopt.h defines some getopt function definitions but one of them
>>>> has a form "extern int getopt ();".  If this form is selected in
>>>> include/getopt.h, Clang generates a warning and the build fails by default.
>>>>
>>>> In really old environments, this getopt definition with no arguments is
>>>> necessary (because the definition may change between environments).
>>>> However, this definition is now a cause of problems on modern environments.
>>>>
>>>> A good news is, this definition is not always selected (e.g. if used by
>>>> binutils/*.c).  This is because configuration scripts of binutils, gas,
>>>> gprof and ld tries to find known definition of getopt function is used and
>>>> defines HAVE_DECL_GETOPT macro.  If this macro is defined when getopt.h is
>>>> included, a good form of getopt is used and Clang won't generate warnings.
>>>>
>>>> This commit adds a modified portion of ld/configure.ac to find the known
>>>> getopt definition.  If we could find one (and we *will* in most modern
>>>> environments), we don't need to rely on the deprecated definition.
>>>> ---
>>>>  sim/config.h.in  |  3 +++
>>>>  sim/configure    | 32 ++++++++++++++++++++++++++++++++
>>>>  sim/configure.ac | 10 ++++++++++
>>>
>>> this logic belongs in m4/sim_ac_platform.m4, not configure.ac
>>>
>>> should leave a comment above the code too indicating that this logic is
>>> purely for local getopt.h usage, and is copied from other dirs (e.g. the
>>> binutils files).
>>
>> OK, I'll submit a patch to move this portion to
>> sim/m4/sim_ac_platform.m4 with proper commit message.
> 
> have you had a chance to fix this up yet ?
> -mike

I'm really sorry.  Due to my poor health condition and upcoming
relocation, it's difficult to keep up with Binutils/GDB until... April
or May.  But this is definitely the one I should have done before I get
so busy (and messy).

I'll submit a patch by Jan 6.

Tsukasa