[PATCH] Require C99 for bfd

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Require C99 for bfd

cbiesinger
From: Christian Biesinger <[hidden email]>

This allows writing more modern code, such as not having to declare all
variables at the top of a block.

See also discussion at:
https://sourceware.org/ml/binutils/2020-01/threads.html#00334

Unfortunately, because of how the toplevel Makefile works, we have to
override CC for this to work.  gnulib/Makefile.am already has a workaround
for this issue.

My patch to fix this in the toplevel makefile has not gotten any attention
so far:
https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02237.html

bfd/ChangeLog:

2020-01-29  Christian Biesinger  <[hidden email]>

        * Makefile.am: Override CC to what configure detected.
        * Makefile.in: Regenerate.
        * configure: Regenerate.
        * configure.ac: Turn on C99 mode.

Change-Id: I65a69846ad5d18c1f10369889dc65caee63b44ce
---
 bfd/Makefile.am  |   5 ++
 bfd/Makefile.in  |   5 ++
 bfd/configure    | 181 ++++++++++++++++++++++++++++++++++++++++++++++-
 bfd/configure.ac |   1 +
 4 files changed, 190 insertions(+), 2 deletions(-)

diff --git a/bfd/Makefile.am b/bfd/Makefile.am
index d32640a12c..825a092000 100644
--- a/bfd/Makefile.am
+++ b/bfd/Makefile.am
@@ -23,6 +23,11 @@ ACLOCAL_AMFLAGS = -I . -I .. -I ../config
 INCDIR = $(srcdir)/../include
 CSEARCH = -I. -I$(srcdir) -I$(INCDIR)
 
+# This is needed because the toplevel Makefile will always pass CC on the
+# commandline, with the value from the toplevel configure which does not
+# include the C99 flag.
+override CC = @CC@
+
 SUBDIRS = doc po
 
 bfddocdir = doc
diff --git a/bfd/Makefile.in b/bfd/Makefile.in
index 78555ccbbc..6dc349350f 100644
--- a/bfd/Makefile.in
+++ b/bfd/Makefile.in
@@ -1891,6 +1891,11 @@ uninstall-am: uninstall-bfdincludeHEADERS uninstall-bfdlibLTLIBRARIES
 .PRECIOUS: Makefile
 
 
+# This is needed because the toplevel Makefile will always pass CC on the
+# commandline, with the value from the toplevel configure which does not
+# include the C99 flag.
+override CC = @CC@
+
 po/SRC-POTFILES.in: @MAINT@ Makefile $(SRC_POTFILES)
  for file in $(SRC_POTFILES); do echo $$file; done \
   | LC_ALL=C sort > tmp.src \
diff --git a/bfd/configure b/bfd/configure
index bcc1a4eaf2..60792041ec 100755
--- a/bfd/configure
+++ b/bfd/configure
@@ -5684,6 +5684,183 @@ $as_echo "$ac_cv_safe_to_define___extensions__" >&6; }
 
 
 
+   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $CC option to accept ISO C99" >&5
+$as_echo_n "checking for $CC option to accept ISO C99... " >&6; }
+if ${ac_cv_prog_cc_c99+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_cv_prog_cc_c99=no
+ac_save_CC=$CC
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <wchar.h>
+#include <stdio.h>
+
+// Check varargs macros.  These examples are taken from C99 6.10.3.5.
+#define debug(...) fprintf (stderr, __VA_ARGS__)
+#define showlist(...) puts (#__VA_ARGS__)
+#define report(test,...) ((test) ? puts (#test) : printf (__VA_ARGS__))
+static void
+test_varargs_macros (void)
+{
+  int x = 1234;
+  int y = 5678;
+  debug ("Flag");
+  debug ("X = %d\n", x);
+  showlist (The first, second, and third items.);
+  report (x>y, "x is %d but y is %d", x, y);
+}
+
+// Check long long types.
+#define BIG64 18446744073709551615ull
+#define BIG32 4294967295ul
+#define BIG_OK (BIG64 / BIG32 == 4294967297ull && BIG64 % BIG32 == 0)
+#if !BIG_OK
+  your preprocessor is broken;
+#endif
+#if BIG_OK
+#else
+  your preprocessor is broken;
+#endif
+static long long int bignum = -9223372036854775807LL;
+static unsigned long long int ubignum = BIG64;
+
+struct incomplete_array
+{
+  int datasize;
+  double data[];
+};
+
+struct named_init {
+  int number;
+  const wchar_t *name;
+  double average;
+};
+
+typedef const char *ccp;
+
+static inline int
+test_restrict (ccp restrict text)
+{
+  // See if C++-style comments work.
+  // Iterate through items via the restricted pointer.
+  // Also check for declarations in for loops.
+  for (unsigned int i = 0; *(text+i) != '\0'; ++i)
+    continue;
+  return 0;
+}
+
+// Check varargs and va_copy.
+static void
+test_varargs (const char *format, ...)
+{
+  va_list args;
+  va_start (args, format);
+  va_list args_copy;
+  va_copy (args_copy, args);
+
+  const char *str;
+  int number;
+  float fnumber;
+
+  while (*format)
+    {
+      switch (*format++)
+ {
+ case 's': // string
+  str = va_arg (args_copy, const char *);
+  break;
+ case 'd': // int
+  number = va_arg (args_copy, int);
+  break;
+ case 'f': // float
+  fnumber = va_arg (args_copy, double);
+  break;
+ default:
+  break;
+ }
+    }
+  va_end (args_copy);
+  va_end (args);
+}
+
+int
+main ()
+{
+
+  // Check bool.
+  _Bool success = false;
+
+  // Check restrict.
+  if (test_restrict ("String literal") == 0)
+    success = true;
+  char *restrict newvar = "Another string";
+
+  // Check varargs.
+  test_varargs ("s, d' f .", "string", 65, 34.234);
+  test_varargs_macros ();
+
+  // Check flexible array members.
+  struct incomplete_array *ia =
+    malloc (sizeof (struct incomplete_array) + (sizeof (double) * 10));
+  ia->datasize = 10;
+  for (int i = 0; i < ia->datasize; ++i)
+    ia->data[i] = i * 1.234;
+
+  // Check named initializers.
+  struct named_init ni = {
+    .number = 34,
+    .name = L"Test wide string",
+    .average = 543.34343,
+  };
+
+  ni.number = 58;
+
+  int dynamic_array[ni.number];
+  dynamic_array[ni.number - 1] = 543;
+
+  // work around unused variable warnings
+  return (!success || bignum == 0LL || ubignum == 0uLL || newvar[0] == 'x'
+  || dynamic_array[ni.number - 1] != 543);
+
+  ;
+  return 0;
+}
+_ACEOF
+for ac_arg in '' -std=gnu99 -std=c99 -c99 -AC99 -D_STDC_C99= -qlanglvl=extc99
+do
+  CC="$ac_save_CC $ac_arg"
+  if ac_fn_c_try_compile "$LINENO"; then :
+  ac_cv_prog_cc_c99=$ac_arg
+fi
+rm -f core conftest.err conftest.$ac_objext
+  test "x$ac_cv_prog_cc_c99" != "xno" && break
+done
+rm -f conftest.$ac_ext
+CC=$ac_save_CC
+
+fi
+# AC_CACHE_VAL
+case "x$ac_cv_prog_cc_c99" in
+  x)
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: none needed" >&5
+$as_echo "none needed" >&6; } ;;
+  xno)
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: unsupported" >&5
+$as_echo "unsupported" >&6; } ;;
+  *)
+    CC="$CC $ac_cv_prog_cc_c99"
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_prog_cc_c99" >&5
+$as_echo "$ac_cv_prog_cc_c99" >&6; } ;;
+esac
+if test "x$ac_cv_prog_cc_c99" != xno; then :
+
+fi
+
+
 
 case `pwd` in
   *\ * | *\ *)
@@ -11728,7 +11905,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11731 "configure"
+#line 11908 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11834,7 +12011,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11837 "configure"
+#line 12014 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
diff --git a/bfd/configure.ac b/bfd/configure.ac
index c5bfbd5d12..e8c96351cb 100644
--- a/bfd/configure.ac
+++ b/bfd/configure.ac
@@ -38,6 +38,7 @@ AC_DISABLE_SHARED
 AC_PROG_CC
 AC_GNU_SOURCE
 AC_USE_SYSTEM_EXTENSIONS
+AC_PROG_CC_C99
 
 LT_INIT([dlopen])
 
--
2.25.0.341.g760bfbb309-goog

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Require C99 for bfd

Jan Beulich-2
On 29.01.2020 15:08, [hidden email] wrote:

> From: Christian Biesinger <[hidden email]>
>
> This allows writing more modern code, such as not having to declare all
> variables at the top of a block.
>
> See also discussion at:
> https://sourceware.org/ml/binutils/2020-01/threads.html#00334
>
> Unfortunately, because of how the toplevel Makefile works, we have to
> override CC for this to work.  gnulib/Makefile.am already has a workaround
> for this issue.
>
> My patch to fix this in the toplevel makefile has not gotten any attention
> so far:
> https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02237.html
>
> bfd/ChangeLog:
>
> 2020-01-29  Christian Biesinger  <[hidden email]>
>
> * Makefile.am: Override CC to what configure detected.
> * Makefile.in: Regenerate.
> * configure: Regenerate.
> * configure.ac: Turn on C99 mode.

I'm not convinced of the move, but I'm also not in the position to
argue against it. However, I think precautions should be taken to
prevent headers of libraries made available for external consumption
to require use of C99 as well - C89 ought to remain sufficient for
them. In the Xen Project we make a wee attempt at something similar
(https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/include/Makefile;hb=refs/heads/staging),
to (hopefully) avoid gaining dependencies on gcc extensions. Maybe
something similar could be done here (unless there is something
like this already, and I'm simply unaware)?

Jan
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Require C99 for bfd

Christian Groessler-2
In reply to this post by cbiesinger
On 2020-01-29 15:08, [hidden email] wrote:
> From: Christian Biesinger <[hidden email]>
>
> This allows writing more modern code, such as not having to declare all
> variables at the top of a block.
>

For the record, I don't like it. But who am I anyway...

regards,
chris


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Require C99 for bfd

Maciej W. Rozycki
On Wed, 29 Jan 2020, Christian Groessler wrote:

> > This allows writing more modern code, such as not having to declare all
> > variables at the top of a block.
>
> For the record, I don't like it. But who am I anyway...

 Yeah, it makes code messy and actually we could decide in our coding
standard not to allow it despite allowing the general use of C99 (there's
probably a GCC option to warn about it).  There could be other advantages
to switching to C99 at one point though, e.g. <stdint.h> types.

  Maciej
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Require C99 for bfd

Sourceware - binutils list mailing list
In reply to this post by cbiesinger
On Wed, Jan 29, 2020 at 6:08 AM <[hidden email]> wrote:

>
> This allows writing more modern code, such as not having to declare all
> variables at the top of a block.
>
> See also discussion at:
> https://sourceware.org/ml/binutils/2020-01/threads.html#00334
>
> Unfortunately, because of how the toplevel Makefile works, we have to
> override CC for this to work.  gnulib/Makefile.am already has a workaround
> for this issue.

If I understand that correctly, I think it's a non-starter.  A command like

make CC="gcc-tip"

should continue to work.

Iian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Require C99 for bfd

Hans-Peter Nilsson-2
In reply to this post by cbiesinger
On Wed, 29 Jan 2020, [hidden email] wrote:

> diff --git a/bfd/Makefile.am b/bfd/Makefile.am
> index d32640a12c..825a092000 100644
> --- a/bfd/Makefile.am
> +++ b/bfd/Makefile.am
> @@ -23,6 +23,11 @@ ACLOCAL_AMFLAGS = -I . -I .. -I ../config
>  INCDIR = $(srcdir)/../include
>  CSEARCH = -I. -I$(srcdir) -I$(INCDIR)
>
> +# This is needed because the toplevel Makefile will always pass CC on the
> +# commandline, with the value from the toplevel configure which does not
> +# include the C99 flag.
> +override CC = @CC@
> +

Do we require GNU make to build binutils (and gdb) yet?
If not, how compatible is that "override"?

brgds, H-P