public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] bash vs. dash: Avoid unportable shell feature in gcc/configure.ac
@ 2011-07-13 16:15 Thomas Schwinge
  2011-07-13 16:39 ` Paolo Bonzini
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Schwinge @ 2011-07-13 16:15 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2007 bytes --]

Hallo!

Diffing the make log of a build of GCC with SHELL not explicitly set
(thus /bin/sh, which is bash) and one with SHELL=/bin/dash, I found the
following unexpected difference:

    -checking assembler for eh_frame optimization... yes
    +checking assembler for eh_frame optimization... buggy

This is from gcc/configure; which invokes
acinclude.m4:gcc_GAS_CHECK_FEATURE for the ``eh_frame optimization''
check.

Latter case, gcc/config.log:

    configure:22282: checking assembler for eh_frame optimization
    configure:22327: /usr/bin/as --32  -o conftest.o conftest.s >&5
    conftest.s: Assembler messages:
    conftest.s: Warning: end of file in string; '"' inserted
    conftest.s:13: Warning: unterminated string; newline inserted

There, the following happens:

    $ sh # This is bash.
    sh-4.1$ echo '.ascii "z\0"'
    .ascii "z\0"

This is what GCC expects.  However, with dash:

    $ dash
    $ echo '.ascii "z\0"'
    .ascii "z

The backslash escape and everything after is cut off.

The test in gcc/configure.ac:

    gcc_GAS_CHECK_FEATURE(eh_frame optimization, gcc_cv_as_eh_frame,
      [elf,2,12,0],,
    [	.text
    [...]
    	.byte	0x1
    	.ascii "z\0"
    	.byte	0x1
    [...]

As quickly determined in #gcc with Ian's and Ismail's help, this is
unportable usage of the echo builtin (and also at least questionable for
/bin/echo), so I'm suggesting the following simple fix:

	gcc/
	* configure.ac (eh_frame optimization): Avoid unportable shell feature.

diff --git a/gcc/configure.ac b/gcc/configure.ac
index c2163bf..73f0209 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -2538,7 +2538,7 @@ __FRAME_BEGIN__:
 .LSCIE1:
 	.4byte	0x0
 	.byte	0x1
-	.ascii "z\0"
+	.asciz "z"
 	.byte	0x1
 	.byte	0x78
 	.byte	0x1a

Alternatively, gcc_GAS_CHECK_FEATURE could be changed to emit the
temporary file by using a shell here-doc, which is what AC_TRY_COMPILE is
doing, for example.


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bash vs. dash: Avoid unportable shell feature in gcc/configure.ac
  2011-07-13 16:15 [PATCH] bash vs. dash: Avoid unportable shell feature in gcc/configure.ac Thomas Schwinge
@ 2011-07-13 16:39 ` Paolo Bonzini
  2011-07-13 17:48   ` Thomas Schwinge
  0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2011-07-13 16:39 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: gcc-patches

On 07/13/2011 06:13 PM, Thomas Schwinge wrote:
> Alternatively, gcc_GAS_CHECK_FEATURE could be changed to emit the
> temporary file by using a shell here-doc, which is what AC_TRY_COMPILE is
> doing, for example.

Change instead echo ifelse(...) > conftest.s to

   AS_ECHO([m4_if(...)]) > conftest.s

in gcc_GAS_CHECK_FEATURE.

Paolo

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bash vs. dash: Avoid unportable shell feature in gcc/configure.ac
  2011-07-13 16:39 ` Paolo Bonzini
@ 2011-07-13 17:48   ` Thomas Schwinge
  2011-07-13 19:15     ` Paolo Bonzini
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Schwinge @ 2011-07-13 17:48 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]

Hallo!

On Wed, 13 Jul 2011 18:23:50 +0200, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 07/13/2011 06:13 PM, Thomas Schwinge wrote:
> > Alternatively, gcc_GAS_CHECK_FEATURE could be changed to emit the
> > temporary file by using a shell here-doc, which is what AC_TRY_COMPILE is
> > doing, for example.
> 
> Change instead echo ifelse(...) > conftest.s to
> 
>    AS_ECHO([m4_if(...)]) > conftest.s
> 
> in gcc_GAS_CHECK_FEATURE.

Ah, even better.

	gcc/
	* acinclude.m4 (gcc_GAS_CHECK_FEATURE): Use AS_ECHO instead of echo.
	* configure: Regenerate.

diff --git a/gcc/acinclude.m4 b/gcc/acinclude.m4
index ff38682..f092925 100644
--- a/gcc/acinclude.m4
+++ b/gcc/acinclude.m4
@@ -583,7 +583,7 @@ AC_CACHE_CHECK([assembler for $1], [$2],
   if test $in_tree_gas = yes; then
     gcc_GAS_VERSION_GTE_IFELSE($3, [[$2]=yes])
   el])if test x$gcc_cv_as != x; then
-    echo ifelse(m4_substr([$5],0,1),[$], "[$5]", '[$5]') > conftest.s
+    AS_ECHO([ifelse(m4_substr([$5],0,1),[$], "[$5]", '[$5]')]) > conftest.s
     if AC_TRY_COMMAND([$gcc_cv_as $gcc_cv_as_flags $4 -o conftest.o conftest.s >&AS_MESSAGE_LOG_FD])
     then
 	ifelse([$6],, [$2]=yes, [$6])

The configure differences are strictly s%echo%$as_echo%.


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bash vs. dash: Avoid unportable shell feature in gcc/configure.ac
  2011-07-13 17:48   ` Thomas Schwinge
@ 2011-07-13 19:15     ` Paolo Bonzini
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2011-07-13 19:15 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: gcc-patches

Ok.

Paolo

On Wed, Jul 13, 2011 at 19:17, Thomas Schwinge <thomas@schwinge.name> wrote:
> Hallo!
>
> On Wed, 13 Jul 2011 18:23:50 +0200, Paolo Bonzini <bonzini@gnu.org> wrote:
>> On 07/13/2011 06:13 PM, Thomas Schwinge wrote:
>> > Alternatively, gcc_GAS_CHECK_FEATURE could be changed to emit the
>> > temporary file by using a shell here-doc, which is what AC_TRY_COMPILE is
>> > doing, for example.
>>
>> Change instead echo ifelse(...) > conftest.s to
>>
>>    AS_ECHO([m4_if(...)]) > conftest.s
>>
>> in gcc_GAS_CHECK_FEATURE.
>
> Ah, even better.
>
>        gcc/
>        * acinclude.m4 (gcc_GAS_CHECK_FEATURE): Use AS_ECHO instead of echo.
>        * configure: Regenerate.
>
> diff --git a/gcc/acinclude.m4 b/gcc/acinclude.m4
> index ff38682..f092925 100644
> --- a/gcc/acinclude.m4
> +++ b/gcc/acinclude.m4
> @@ -583,7 +583,7 @@ AC_CACHE_CHECK([assembler for $1], [$2],
>   if test $in_tree_gas = yes; then
>     gcc_GAS_VERSION_GTE_IFELSE($3, [[$2]=yes])
>   el])if test x$gcc_cv_as != x; then
> -    echo ifelse(m4_substr([$5],0,1),[$], "[$5]", '[$5]') > conftest.s
> +    AS_ECHO([ifelse(m4_substr([$5],0,1),[$], "[$5]", '[$5]')]) > conftest.s
>     if AC_TRY_COMMAND([$gcc_cv_as $gcc_cv_as_flags $4 -o conftest.o conftest.s >&AS_MESSAGE_LOG_FD])
>     then
>        ifelse([$6],, [$2]=yes, [$6])
>
> The configure differences are strictly s%echo%$as_echo%.
>
>
> Grüße,
>  Thomas
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-07-13 18:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-13 16:15 [PATCH] bash vs. dash: Avoid unportable shell feature in gcc/configure.ac Thomas Schwinge
2011-07-13 16:39 ` Paolo Bonzini
2011-07-13 17:48   ` Thomas Schwinge
2011-07-13 19:15     ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).