> Zack Weinberg <[email protected]> writes:
>> Please also file a tracking bug for this change at
>> https://savannah.gnu.org/support/?func=additem&group=autoconf so we
>> don't forget about it again.
>>
>> Finally, I haven't reviewed the code in detail but I would like to see
>> more comprehensive tests. Right now you're only testing one word in
>> quotes in CFLAGS; blindly stripping the quotes would do the right
>> thing.  If I understand your goal correctly, you should be testing
>> things like `configure CC="cc -std=c89"` and
>> `CPPFLAGS="-Dfunction_like_macro(with, arguments)=..."` (put something
>> in the ... that actually uses the arguments).

Updated patch with improved test cases attached. 

I provided 4 test cases:

 1. AC_COMPILE_IFELSE with pre-created CPPFLAGS. This test will fail with
    current autoconf code.
 2. Compilation of the same code where CPPFLAGS passed through
    Makefile.in. This one will work with current autoconf code
    correctly.
 3. AC_RUN_IFELSE with function call passed through CPPFLAGS. This test
    will fail with current autoconf code.
 4. Running of the same code where CPPFLAGS passed through
    Makefile.in. This test will work correctly with current autoconf
    code.

I think that the tests pairs 1, 2 and 3, 4 should always provide same
results. Without suggested modification they gives us different
results.

>> Please also try to
>> think of situations where double evaluation would do the *wrong*
>> thing; that will help us understand the potential negative
>> consequences of this change.

I can suggest only one scenario when double evaluation would do wron
thing: Custom _AC_DO_STDERR command that used only through
configuration, but never will be reproduced in Makefile.in. Something
like

> _AC_DO_STDERR([echo \"hello\" >&2]) && test "$(cat conftest.err)" = '"hello"'

Regards,
Nikolai

>From f4ba49a2f68d6640711b7de41390acb7a187ff0c Mon Sep 17 00:00:00 2001
From: Nikolai Merinov <[email protected]>
Date: Thu, 7 Mar 2019 22:33:13 +0500
Subject: [PATCH] _AC_DO _AC_DO_STDERR: Evaluate argument twice

The `AC_COMPILE_IFELSE', `AC_LINK_IFELSE', `AC_PREPROC_IFELSE' macros
use the `_AC_DO_STDERR` macro in the following manner:

> ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
> _AC_DO_STDERR($ac_compile) # Unescape variable only once

Full compilation process works like the following:

> CFLAGS='-DFLAG=\"quoted string\"' ./configure
> # Produce Makefile with CFLAGS = -DFLAG=\"quoted string\"
> make # substitute args in "$(CC) $(CFLAGS) ..." recipe and call shell
> gcc -DFLAG="quoted string" ... # Arg was evaluated twice

In order to have same behavior during a configuration stage and during
a compilation stage we should evaluate a passed argument twice in
_AC_DO_STDERR command: in second time we'll be able to unquote strings
in user-supplied variables as it happens during make rules execution.

The `_AC_DO' macro used through the `_AC_DO_VAR' macro in similar
manner in many places and should work by same rules.

* lib/autoconf/general.m4 (_AC_DO_STDERR): Reuse `_AC_DO_ECHO' trick
to evaluate arguments twice if compile string have no '\"', '`', or
'\\' symbols.
  (_AC_DO): Same changes.
* tests/compile.at: Add test for CPPFLAGS with escaped string in
  autoconf compilation commands and with compilation through Makefile.
---
 NEWS                    |  6 +++
 lib/autoconf/general.m4 | 22 ++++++++--
 tests/compile.at        | 92 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 116 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index dbbea222..b637b2e3 100644
--- a/NEWS
+++ b/NEWS
@@ -229,6 +229,12 @@ GNU Autoconf NEWS - User visible changes.
   subdirectory.  See the documentation of AC_OPENMP for further
   explanation.
 
+*** The _AC_DO_STDERR and _AC_DO macros now evaluate a COMMAND argument twice.
+
+  This change consolidate behavior for user-supplied arguments
+  between the AC_COMPILE_IFELSE and AC_LINK_IFELSE macros and
+  compilation commands used in makefiles.
+
 ** New features
 
 *** Configure scripts now support a ‘--runstatedir’ option.
diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
index 16f0d074..1fecffdb 100644
--- a/lib/autoconf/general.m4
+++ b/lib/autoconf/general.m4
@@ -2607,16 +2607,30 @@ AS_ECHO(["$ac_try_echo"])])
 # Eval COMMAND, save the exit status in ac_status, and log it.
 # For internal use only.
 AC_DEFUN([_AC_DO],
-[_AC_RUN_LOG([eval "$1"],
-	     [_AC_DO_ECHO([$1])])])
+[m4_if([$1], [$ac_do], [], [ac_do="$1"
+])]dnl
+[[case "(($ac_do" in
+  *\"* | *\`* | *\\*) ac_do_command=\$ac_do;;
+  *) ac_do_command=$ac_do;;
+esac
+eval ac_do_command="\"$ac_do_command\""]
+_AC_RUN_LOG([eval "$ac_do_command"],
+		   [_AC_DO_ECHO([$ac_do])])])
 
 
 # _AC_DO_STDERR(COMMAND)
 # ----------------------
 # Like _AC_RUN_LOG_STDERR, but eval (instead of running) COMMAND.
 AC_DEFUN([_AC_DO_STDERR],
-[_AC_RUN_LOG_STDERR([eval "$1"],
-		    [_AC_DO_ECHO([$1])])])
+[m4_if([$1], [$ac_do_stderr], [], [ac_do_stderr="$1"
+])]dnl
+[[case "(($ac_do_stderr" in
+  *\"* | *\`* | *\\*) ac_do_stderr_command=\$ac_do_stderr;;
+  *) ac_do_stderr_command=$ac_do_stderr;;
+esac
+eval ac_do_stderr_command="\"$ac_do_stderr_command\""]
+_AC_RUN_LOG_STDERR([eval "$ac_do_stderr_command"],
+		   [_AC_DO_ECHO([$ac_do_stderr])])])
 
 
 # _AC_DO_VAR(VARIABLE)
diff --git a/tests/compile.at b/tests/compile.at
index 4847f111..ae116a23 100644
--- a/tests/compile.at
+++ b/tests/compile.at
@@ -344,6 +344,98 @@ AT_CHECK_CONFIGURE([-q])
 
 AT_CLEANUP
 
+## -------------------------- ##
+## Quoted string in CPPFLAGS  ##
+## -------------------------- ##
+
+AT_SETUP([Qouted string in CPPFLAGS])
+
+AT_DATA([configure.ac],
+[[AC_INIT
+AC_PROG_CC
+CPPFLAGS='-DSTRING=\"Hello,\ World\"'
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[const char hw[] = STRING;]], [return 0])],
+	      [],
+	      [AC_MSG_ERROR([compiling program with quoted CFLAGS failed])])
+AC_OUTPUT
+]])
+
+AT_CHECK_AUTOCONF
+AT_CHECK_CONFIGURE
+
+AT_DATA([configure.ac],
+[[AC_INIT
+AC_PROG_CC
+CPPFLAGS='-DSTRING=\"Hello,\ World\"'
+AC_CONFIG_FILES([Makefile])
+AC_OUTPUT
+]])
+
+AT_DATA([Makefile.in],
+[[foo@EXEEXT@: foo.@OBJEXT@
+	@CC@ @CFLAGS@ @LDFLAGS@ -o $@ foo.@OBJEXT@
+
+foo.@OBJEXT@: foo.c
+	@CC@ @CPPFLAGS@ @CFLAGS@ -c foo.c
+]])
+
+AT_DATA([foo.c],
+[[const char hw[] = STRING;
+int main (void)
+{
+  return 0;
+}
+]])
+
+AT_CHECK_AUTOCONF
+AT_CHECK_CONFIGURE
+AT_CHECK_MAKE
+
+AT_DATA([configure.ac],
+[[AC_INIT
+AC_PROG_CC
+CPPFLAGS='-DRETURN="exit(0)"'
+AC_RUN_IFELSE([AC_LANG_PROGRAM([@%:@include <stdlib.h>], [RETURN; return 1])],
+	      [],
+	      [AC_MSG_ERROR([Running program with parenthesis in macro failed])])
+AC_OUTPUT
+]])
+
+AT_CHECK_AUTOCONF
+AT_CHECK_CONFIGURE
+
+AT_DATA([configure.ac],
+[[AC_INIT
+AC_PROG_CC
+CPPFLAGS='-DRETURN="exit(0)"'
+AC_CONFIG_FILES([Makefile])
+AC_OUTPUT
+]])
+
+AT_DATA([Makefile.in],
+[[foo@EXEEXT@: foo.@OBJEXT@
+	@CC@ @CFLAGS@ @LDFLAGS@ -o $@ foo.@OBJEXT@
+
+foo.@OBJEXT@: foo.c
+	@CC@ @CPPFLAGS@ @CFLAGS@ -c foo.c
+]])
+
+AT_DATA([foo.c],
+[[#include <stdlib.h>
+int main (void)
+{
+  RETURN;
+  return 1;
+}
+]])
+
+AT_CHECK_AUTOCONF
+AT_CHECK_CONFIGURE
+AT_CHECK_MAKE
+AT_CHECK([./foo])
+
+AT_CLEANUP
+
 ## --------------------------  ##
 ## Order of `rm' and actions.  ##
 ## --------------------------  ##
-- 
2.29.2

Reply via email to