Misuse of autoconf features in configure.ac causes multiple problems

[ryandesign]

Hi, there are multiple problems with the way dfu-programmer's configure.ac uses autoconf features which cause build problems for me on macOS. This was reported to MacPorts here. I reported it as an autoconf bug but now believe it's just a bug with how dfu-programmer is using autoconf.

Symptoms of the problems include this configure output:

checking for ANSI C header files... no

checking stdlib.h usability... yes
checking stdlib.h presence... no
configure: WARNING: stdlib.h: accepted by the compiler, rejected by the preprocessor!
configure: WARNING: stdlib.h: proceeding with the compiler's result
checking for stdlib.h... yes

In the current version of autoconf it is assumed that ANSI C header files exist and the header "presence" checks have been removed, but the reason why these tests are failing here is that they are trying to run $CPP but $CPP is unexpectedly empty (autoconf should have set it to a reasonable value) so the check fails with errors like:

./configure: line 4534: conftest.c: command not found

Because it thinks ANSI C header files don't exist, it doesn't define things like HAVE_STRING_H, resulting in subsequent checks encountering implicit declaration of functions, which hasn't been allowed since C99 was introduced 2 decades ago and which is now forbidden in the version of clang that comes with Xcode 12 and later (in order to accommodate Apple Silicon processors), so subsequent checks fail:

checking for working memcmp... no

because:

configure:4774: checking for working memcmp
configure:4817: cc -o conftest -Werror=implicit-function-declaration -I/opt/local/include/libusb-1.0   conftest.c  -L/opt/local/lib -lusb-1.0 >&5
conftest.c:53:7: error: implicitly declaring library function 'memcmp' with type 'int (const void *, const void *, unsigned long)' [-Werror,-Wimplicit-function-declaration]
  if (memcmp(&c0, &c2, 1) >= 0 || memcmp(&c1, &c2, 1) >= 0)
      ^
conftest.c:53:7: note: include the header <string.h> or explicitly provide a declaration for 'memcmp'
conftest.c:67:2: error: implicitly declaring library function 'strcpy' with type 'char *(char *, const char *)' [-Werror,-Wimplicit-function-declaration]
        strcpy (a, "--------01111111");
        ^
conftest.c:67:2: note: include the header <string.h> or explicitly provide a declaration for 'strcpy'
2 errors generated.

I am manually adding -Werror=implicit-function-declaration to CFLAGS because I am using clang from an older version of Xcode but I want to see the problems that users of Xcode 12 or later would see so that I can try to fix them.

I have libusb 1 installed with MacPorts. If I use the --disable-libusb_1_0 configure flag these problems go away, pointing to a possible problem within the configure.ac code that checks for libusb 1.

Although I don't think it's directly contributing to this problem, it looks suspicious to me that the code is modifying CFLAGS; configure.ac files should never do that. It's explained in the automake documentation:

You should never redefine a user variable ... in Makefile.am

You should not add options to these user variables within configure either

It also looked suspicious to me that autoconf directives that I would have assumed should be done as part of the initial setup (AC_HEADER_STDC, AC_C_CONST, AC_TYPE_SIZE_T, AC_FUNC_MALLOC, AC_FUNC_MEMCMP) are instead being performed near the end of configure.ac. I tried moving AC_HEADER_STDC to near the top of configure.ac (right after AC_PROG_CC). This fixes the problem that $CPP was empty but does not fix the problem that HAVE_STRING_H et al are not defined. Also moving AC_C_CONST, AC_TYPE_SIZE_T, AC_FUNC_MALLOC, AC_FUNC_MEMCMP to the top after AC_HEADER_STDC fixed the second problem. Now HAVE_STRING_H et al are defined, reflected in the additional configure output showing headers being checked, and it can detect that memcmp does exist:

checking for an ANSI C-conforming const... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking for size_t... yes
checking for stdlib.h... (cached) yes
checking for GNU libc compatible malloc... yes
checking for working memcmp... yes

So this diff is enough to solve these problem:

diff --git a/configure.ac b/configure.ac
index 233b7b1..482ead7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,6 +13,17 @@ AM_MAINTAINER_MODE
 # Checks for programs.
 AC_PROG_CC

+# Checks for header files.
+AC_HEADER_STDC
+
+# Checks for typedefs, structures, and compiler characteristics.
+AC_C_CONST
+AC_TYPE_SIZE_T
+
+# Checks for library functions.
+AC_FUNC_MALLOC
+AC_FUNC_MEMCMP
+
 # Checks for libusb - from sane-backends configuration

 dnl Enable libusb-1.0, if available
@@ -64,17 +75,6 @@ if test "$HAVE_USB" = "yes"; then
 fi


-# Checks for header files.
-AC_HEADER_STDC
-
-# Checks for typedefs, structures, and compiler characteristics.
-AC_C_CONST
-AC_TYPE_SIZE_T
-
-# Checks for library functions.
-AC_FUNC_MALLOC
-AC_FUNC_MEMCMP
-#AC_CHECK_FUNC([memset], :, [AC_CHECK_LIB([libc], [libc])])

 AC_CONFIG_FILES(fedora/dfu-programmer.spec Makefile docs/Makefile src/Makefile)
 AC_OUTPUT

A different fix was submitted in PR #22, to change how libusb 1 is detected and to no longer modify CFLAGS, which also works for me to solve the preceding problems. In fact the PR says it was merged:

@swinman merged commit 0daf3d8 into dfu-programmer:master on Feb 18, 2016

I'm confused therefore that the latest commit to master was actually made in Jan 19, 2016 and there is no sign of this commit on master. Perhaps you rewrote history afterward to remove this commit, perhaps because of problems reported on Windows? If so, please don't rewrite history in public repositories; it makes things hard to follow. That PR should be reopened so that whatever the problems were can be fixed and it can be re-merged.

With either my fix above or the one from #22, master then builds. 0.7.2 does not build; it fails with:

atmel.c:285:31: error: implicitly declaring library function 'malloc' with type 'void *(unsigned long)' [-Werror,-Wimplicit-function-declaration]
    bout->data = (uint16_t *) malloc( total_size * sizeof(uint16_t) );
                              ^
atmel.c:285:31: note: include the header <stdlib.h> or explicitly provide a declaration for 'malloc'
1 error generated.

This was already fixed in 26d96f2 by moving the code that calls malloc from atmel.c to intel_hex.c which already includes <stdlib.h>. A new version of dfu-programmer containing this fix and a fix for the other problems reported in this issue should be released.

[zackw]

After further investigation from Autoconf's side we traced the failure to detect standard C header files to use of AC_ macros inside shell if statements. This is one of the backward compatibility issues warned about in the 2.70 release notes:

Many macros no longer AC_REQUIRE as many other macros as they used to.

This can expose several classes of latent bugs. These are the ones
we know about:

• Make sure to explicitly invoke all of the macros that set result
  variables used later in the configure script, or in generated
  Makefiles.

• Autoconf macros that use AC_REQUIRE are not safe to use inside shell
  control-flow constructs that appear outside of macros defined by
  AC_DEFUN. Use AS_IF, AS_CASE, etc. instead. (See the
  “Prerequisite Macros” section of the manual for details.)

  The set of macros that use AC_REQUIRE internally may change from
  release to release. The only macros that are guaranteed not to
  use AC_REQUIRE are the macros for acting on the results of a
  test: AC_DEFINE, AC_SUBST, AC_MSG_*, AC_CACHE_CHECK, etc.

• AC_REQUIRE cannot be applied to macros that need to be used with
  arguments. Instead, invoke the macro normally, with its arguments.

Paul Eggert found that this patch was sufficient to fix this problem:

diff --git a/configure.ac b/configure.ac
index 233b7b1..77c1ac0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -37,7 +37,8 @@ if test "$disable_libusb_1_0" = "no"; then
   fi
fi

-if test "$have_libusb_1_0" = "no"; then
+AS_IF([test "$have_libusb_1_0" = "no"],
+[
   dnl Fallback to the old libusb
   dnl libusb >= 0.1.8 is required, as we need usb_interrupt_read()
   AS_ECHO("using libusb");
@@ -45,7 +46,7 @@ if test "$have_libusb_1_0" = "no"; then
                   AC_CHECK_LIB(usb, usb_interrupt_read,
                                [LIBS="$LIBS -lusb"
                                 HAVE_USB=yes]))
-fi
+])

dnl The following logic is useful for distributions.  If they force
dnl USB support with --enable-libusb=yes then configure will fail

However, in his words, "There are lots of other problems with that configure.ac: current Autoconf generates a blizzard of warnings about it." We advise reading through the complete release notes for 2.70 carefully and then taking some time to revise the configure script thoroughly.

[cinderblock]

I've been updating some of these on #64 but not confident I have them all. I'm new to autoconf and would love a look. I also think the current configure.ac is a little verbose and confusingly written. I'd be happy for a rewrite, as long as build flags don't change.