mainAutoconf - Support: sr #110530, Missing #include <string.h>...

 
 

sr #110530: Missing #include <string.h> in AC_FUNC_MEMCMP

Submitter:  Ryan Carsten Schmidt <ryandesign>
Submitted:  Thu 19 Aug 2021 11:25:32 PM UTC
   
 
Priority:  * 5 - Unprioritized Severity:  3 - Normal
Status:  Invalid Privacy:  Public
Assigned to:  None Open/Closed:  Closed
Operating System:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 01 Sep 2021 01:06:45 AM UTC, comment #11: 

Thanks, Paul.  I copied your patch and commentary into the bug Ryan already reported: https://github.com/dfu-programmer/dfu-programmer/issues/61#issuecomment-909776423

Zack Weinberg <zackw>
Group administrator
Tue 31 Aug 2021 11:07:19 PM UTC, comment #10: 


> However, none of that should affect AC_FUNC_MEMCMP, because it doesn't use either of those, and string.h is (or should be) automatically included via AC_INCLUDES_DEFAULT.  Something else must be wrong.

Yes, the thing that's wrong is that the configure.ac in question is busted: it has an AC_CHECK_HEADER nested inside a shell "if", and AC_CHECK_HEADERS's prerequisites are thus being skipped which means HAVE_STRING_H isn't set. Here's a simple fix:


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


I'd report this to dfu-programmer except I don't see where the project says how to report bugs and I've spent too much time on this already. There are lots of other problems with that configure.ac: current Autoconf generates a blizzard of warnings about it.

Perhaps somebody else has the time to figure out how to report the fix and further problems to the dfu-programmer developers. In the meantime I'm closing the Autoconf bug report, as this is not an Autoconf bug.


Paul Eggert <eggert>
Group administrator
Tue 31 Aug 2021 10:56:09 PM UTC, comment #9: 

comment #7:

> Your getdtablesize example is inapplicable since a prototype is provided

Oh, good point. As Zack mentioned, the -Wno-error=implicit-function-declaration issue is therefore irrelevant to the current bug report.

At the risk of adding yet more-irrelevant info to this bug report I will reply to some of your other points.

> I'll just explain that on arm64 processors, the compiler generates different machine code depending on whether a function is variadic or not.

Fine, but that's true for many architectures including x86-64. The idea that this incompatibility was forced by machine architecture is a red herring.

> This is in contrast to x86_64 processors where the arguments go in the same place regardless (unless the function has a very large number of arguments which is uncommon).

No, x86-64 is like most other architectures, in that if you omit function prototypes your programs can go way wrong. For example:


int foo (float);
// 'bar' has the same API, but it's omitted here.
int main (void)
{
  foo (0);
  bar (0);
}


The x86-64 machine code for the call to 'foo' is quite different from that of the call to 'bar', and the latter call does not work on x86-64. (It happened to work on some older platforms, but those days are long gone.)

> You are proposing that this helpful compile-time notification should be disabled

Not at all. It's a good thing to generate compile-time notifications for this problem, and GCC regularly does so on GNU/Linux. I'm merely objecting to the notifications breaking the compilations; this is an incompatible change to clang that is likely to cause more real-world problems than it cures.


> However there may be other types of checks performed by some autoconf configure scripts that actually link and run a program in order to make some determination. These programs might crash or misbehave and thus make the wrong determination on arm64 if implicitly declared functions are involved.

Not if they're merely testing for function existence.

> As you point out, function prototypes have been available since C89, and have been required since C99, so everyone has had ample time to modify their code to use them

That's fine if you're writing a single test for a particular function. It's not fine if you're writing a generic Autoconf macro intended for checking arbitrary functions. To cater to this new -Werror business, any such macro would require its user to declare the functions in question, which is a tedious and error-prone process that is likely more trouble than it's worth.

Paul Eggert <eggert>
Group administrator
Tue 31 Aug 2021 12:33:14 PM UTC, comment #8: 

This whole discussion has gotten way off into the weeds.

It's true that everything that uses the C version of AC_LANG_CALL or AC_LANG_FUNC_LINK_TRY is broken by any compiler that defaults to [the equivalent of] -Werror=implicit-function-declaration (or -Werror=strict-prototypes, for that matter).  And fixing that "properly" (by arranging for Autoconf to know which header to include for each library function) is going to be enough work that it might make sense to use -Wno-error=implicit-function-declaration -Wno-error=strict-prototypes as a stopgap.

However, none of that should affect AC_FUNC_MEMCMP, because it doesn't use either of those, and string.h is (or should be) automatically included via AC_INCLUDES_DEFAULT.  Something else must be wrong.  Can we please keep this bug report focused on the originally reported problem?

Zack Weinberg <zackw>
Group administrator
Tue 31 Aug 2021 11:41:06 AM UTC, comment #7: 

I don't really want to continue this discussion, and I'm not going to file bug reports with another company on your behalf arguing for a change that I don't advocate. I'll just explain that on arm64 processors, the compiler generates different machine code depending on whether a function is variadic or not. It affects whether arguments go on the stack or in registers. This is in contrast to x86_64 processors where the arguments go in the same place regardless (unless the function has a very large number of arguments which is uncommon). The compiler must therefore know in advance what type of function it is so it can write the correct machine code, and the way that it knows that is by seeing the function prototype, so it is imperative that one be provided, e.g. by including the right header.

I would not say that Apple "cares" or "doesn't care" about GNU tools; instead, I would say that Apple wished to improve the performance of the computers they sell by switching from Intel processors to in-house arm64 processors, and they made a change to their toolchain that would ensure developers are notified if they use a programming technique that could cause problems on the new architecture. You are proposing that this helpful compile-time notification should be disabled; I don't think that's a good idea since debugging runtime crashes and misbehavior is more difficult than noticing and correcting a compile-time error.

Your getdtablesize example is inapplicable since a prototype is provided and it would not be affected by enabling or disabling the implicit-function-declaration error. However there may be other types of checks performed by some autoconf configure scripts that actually link and run a program in order to make some determination. These programs might crash or misbehave and thus make the wrong determination on arm64 if implicitly declared functions are involved.

Instructing the user to add -Wno-error=implicit-function-declaration to CFLAGS when running configure would be bad because those CFLAGS would be recorded and used during the build where they could cause the aforementioned problem in the software that gets built and installed.

As you point out, function prototypes have been available since C89, and have been required since C99, so everyone has had ample time to modify their code to use them; fighting for the continued right to neglect to specify something that has been required for over 20 years seems fruitless.

Ryan Carsten Schmidt <ryandesign>
Tue 31 Aug 2021 11:19:14 AM UTC, comment #6: 

comment #5:

> I don't know whether there was an alternative that Apple did not pursue in which implicit function declarations could have been allowed.

Sure there was an alternative. They could have made -Wno-error=implicit-function-declaration the default.

> If you disagree with them, you may file a bug report

I don't use Apple software so it's not likely they'll listen to any bug report I might file (possibly they won't even let me file one). If this problem affects you, I suggest you file a bug report; if you're an Apple customer they might listen to you.

> according to Apple, if you allow implicit function declarations, you risk creating a program ... that on Apple Silicon processors either crashes or behaves incorrectly.

Of course. But this has been true on pretty much every platform, ever since people started using function prototypes, introduced in C89 over three decades ago. But this issue does not affect Autoconf's usage. Autoconf is merely trying to find whether the function exists; it's not trying to call the function.

> Do not use -Wno-error=implicit-function-declaration to attempt to circumvent this protection.

This advice sounds incorrect to me.

Please give a specific example of where using -Wno-error=implicit-function-declaration would hurt on macOS, in the sense that it would break Autoconf's use of that flag to detect whether a function exists or is declared.

For example, to test whether getdtablesize exists, 'configure' tries to compile and link a program that is like this:


char getdtablesize ();
int main () { return getdtablesize (); }


It doesn't run the program; it merely links it. If the link succeeds, the function exists. Why would compiling with -Wno-error=implicit-function-declaration break this?

Paul Eggert <eggert>
Group administrator
Tue 31 Aug 2021 09:45:04 AM UTC, comment #5: 

Don't shoot the messenger. Ok, it's true, I don't know whether there was an alternative that Apple did not pursue in which implicit function declarations could have been allowed. All I know is that they determined that this was the best way forward. If you disagree with them, you may file a bug report with them and work with them toward a solution for a future release of clang. But until then, we're left with the current reality, which is that, according to Apple, if you allow implicit function declarations, you risk creating a program (whether it is the main program you're compiling or a configuration test that is trying to determine some system capability) that on Apple Silicon processors either crashes or behaves incorrectly. Apple didn't choose for this behavior to happen; it's just how arm64 processors work. In order to avoid that eventuality and to let you know about the problem in advance, at compile time, implicit declaration of functions is an error. Do not use -Wno-error=implicit-function-declaration to attempt to circumvent this protection.

Ryan Carsten Schmidt <ryandesign>
Mon 30 Aug 2021 06:50:10 PM UTC, comment #4: 


> Apple had to make this change in order to support Apple Silicon processors

No it didn't. At best, Apple just doesn't care much about compatibility with GNU tools.

A simple workaround would be for 'configure' to automatically add -Wno-error=implicit-function-declaration to CFLAGS if it's needed to compile a simple test like "does strlen exist?". Perhaps Autoconf should do that.

In the meantime, the advice "configure with CFLAGS=-Wno-error=implicit-function-declaration" will simply have to become de rigueur among people who want to run GNU tools on macOS despite Apple's increasingly-high portability barriers.

Paul Eggert <eggert>
Group administrator
Mon 30 Aug 2021 11:25:04 AM UTC, comment #3: 

Hi Zack, thanks for the response.

1. Unfortunately you don't have a choice about the matter. Autoconf will experience the effects of -Werror=implicit-function-declaration when using clang from Xcode 12 or later because that is its default behavior. Apple had to make this change in order to support Apple Silicon processors so disabling this is not an option. I manually add -Werror=implicit-function-declaration to CFLAGS when building all software on my system because I have not upgraded to Xcode 12 yet but I want to experience and participate in fixing the problems that result from this.

In any case, using -Werror=implicit-function-declaration is not causing the problem that HAVE_STRING_H is not defined; it's only making it obvious that this problem exists so now we can fix it.

2. I now think this is a bug in how dfu-programmer is using autoconf and have reported it to them:

https://github.com/dfu-programmer/dfu-programmer/issues/61

Something about the way that they are checking for libusb seems to be causing subsequent invocations of AC_HEADER_STDC and AC_FUNC_MEMCMP not to work correctly. A simple fix was to move the invocations of AC_HEADER_STDC and AC_FUNC_MEMCMP above the libusb checks.

Ryan Carsten Schmidt <ryandesign>
Thu 26 Aug 2021 08:24:23 PM UTC, comment #2: 

1. We currently don't support any use of -Werror or -Werror=whatever in configure scripts.  There are lots and lots of places where autoconf's test programs trigger warnings, especially with newer compilers. We want to fix all of those eventually but we have no developers right now, so "eventually" might be a very long time.

The recommended way to use -Werror with autotools is with an AC_ARG_ENABLE construct that optionally adds -Werror to a dedicated AC_SUBST variable that is then added to AM_CFLAGS in Makefile.am (or equivalent if you're not using automake).

2. That said, HAVE_STRING_H should have been defined automatically.  Could you please cut down dfu-programmer's configure.ac to the smallest thing you can manage that still reproduces the issue, and attach that file here?  Please also make it self-contained -- inline any definitions from aclocal.m4 that are needed.

Zack Weinberg <zackw>
Group administrator
Thu 19 Aug 2021 11:39:03 PM UTC, comment #1: 

I am using dfu-programmer 0.7.2 with the configure script regenerated by autoconf 2.71 and automake 1.16.3.

I see that AC_FUNC_MEMCMP uses AC_INCLUDES_DEFAULT. I see in the generated configure script that ac_includes_default (is that the same thing?) contains:


#ifdef HAVE_STRING_H
# include <string.h>
#endif


so I guess HAVE_STRING_H is not defined. Should AC_FUNC_MEMCMP have taken care of defining it or does dfu-programmer's configure.ac need to be calling something else first to get that defined?

Ryan Carsten Schmidt <ryandesign>
Thu 19 Aug 2021 11:25:32 PM UTC, original submission:  

Hi, dfu-programmer's configure.ac uses:


AC_FUNC_MEMCMP


When the configure script is run with "-Werror=implicit-function-declaration" in CFLAGS (or with the clang from Xcode 12 or later where that behavior is the default), configure output contains:


checking for working memcmp... no


and config.log contains:


conftest.c:47: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:47:7: note: include the header <string.h> or explicitly provide a declaration for 'memcmp'
conftest.c:61:2: error: implicitly declaring library function 'strcpy' with type 'char *(char *, const char *)' [-Werror,-Wimplicit-function-declaration]
        strcpy (a, "--------01111111");
        ^
conftest.c:61:2: note: include the header <string.h> or explicitly provide a declaration for 'strcpy'
2 errors generated.


I believe "#include <string.h>" should be added to AC_FUNC_MEMCMP (in lib/autoconf/functions.m4).

Ryan Carsten Schmidt <ryandesign>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by eggert (Posted a comment)
  • -email is unavailable- added by zackw (Posted a comment)
  • -email is unavailable- added by ryandesign (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-08-31 eggert StatusNone Invalid
        Open/ClosedOpen Closed
        Operating SystemMac OS None

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code