NetBSD Problem Report #45269

From gson@gson.org  Fri Aug 19 09:02:32 2011
Return-Path: <gson@gson.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 3268563B89A
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 19 Aug 2011 09:02:32 +0000 (UTC)
Message-Id: <20110819090227.E198075E8F@guava.gson.org>
Date: Fri, 19 Aug 2011 12:02:27 +0300 (EEST)
From: gson@gson.org (Andreas Gustafsson)
Reply-To: gson@gson.org (Andreas Gustafsson)
To: gnats-bugs@gnats.NetBSD.org
Subject: sh fails to strip trailing newline in command substitution, breaking build
X-Send-Pr-Version: 3.95

>Number:         45269
>Category:       bin
>Synopsis:       sh fails to strip trailing newline in command substitution, breaking build
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    bin-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Aug 19 09:05:00 +0000 2011
>Closed-Date:    Sun Mar 11 23:41:40 +0000 2012
>Last-Modified:  Sun Mar 11 23:41:40 +0000 2012
>Originator:     Andreas Gustafsson
>Release:        NetBSD 5.1_STABLE of 2011-04-25
>Organization:
>Environment:
System: NetBSD babylon5.NetBSD.org 5.1_STABLE NetBSD 5.1_STABLE (GENERIC) #4: Mon Apr 25 09:34:00 UTC 2011 root@franklin.NetBSD.org:/home/netbsd/5/amd64/kern-compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:

Since GCC 4.5 was enabled on x86 by mrg's commit of bsd.own.mk 1.678
(CVS date 2011.08.07.08.15.02), every i386 release build on the TNF
automated test server babylon5.netbsd.org has failed, as can be seen at

    http://releng.netbsd.org/b5reports/i386/commits-2011.08.html#2011.08.07.08.15.02

The failures occur during the linking of libmpfr:

    libtool: link: ranlib
     .libs/libmpfr.a
    Usage: ranlib [options] archive

The ranlib comman fails because obj/tools/mpfr/build/config.status contains

    # Commands used to install an old-style archive.
    RANLIB="ranlib
    "

with a newline character between the ranlib command and the trailing
double quote.  In config.status, there is similarly

    RANLIB='ranlib
    '

but in config.log, there is no extra newline:

    RANLIB='ranlib'

The libtool script is generated by configure; it appears that the
shell running the configure script is failing to strip the trailing
newline from the output of a command substitution when generating the
RANLIB line.

A similar problem in a package build was reported in pkg/45052.

I have marked this bug as critical because:

 1) NetBSD can no longer be reliably built on a NetBSD host

 2) it has rendered the TNF automated test infrastructure inoperable

>How-To-Repeat:

The bug can be a bit tricky to reproduce; pkg/45052 reports that
adding some printf or echo commands can make it go away, and my own
tests indicate that doing the build in a directory whose pathname has
a significantly different length from that used by the automated build
scripts also makes it go away.  But the automated build scripts do
reproduce it reliably, and the following also works for me:

1. If you are not a developer, become one :)

2. Ask admins for an account on babylon5.netbsd.org and log in.

4. Create a directory whose name is 38 characers long; let's call this
directory "$d".  For example, I used:

   d=/mnt2/gson/aaaaaaaaaaaaaaaaaaaaaaaaaaa
   mkdir $d

4. Then run:

   cd $d
   CVSROOT="anoncvs@anoncvs.NetBSD.org:/cvsroot" cvs checkout -D 2011.08.13.12.18.54 src
   cd $d/src
   ./build.sh -D $d/destdir -R $d/release -T $d/tools -O $d/obj -m i386 -a i386 -U release

5. Watch the build fail.

>Fix:

>Release-Note:

>Audit-Trail:
From: Andreas Gustafsson <gson@gson.org>
To: gnats-bugs@NetBSD.org
Cc: mrg@netbsd.org, Martin Husemann <martin@mail.netbsd.org>, christos@zoulas.com
Subject: Re: bin/45269: sh fails to strip trailing newline in command substitution, breaking build
Date: Sat, 20 Aug 2011 12:00:18 +0300

 The sparc builds on babylon5 <http://releng.netbsd.org/b5reports/sparc/>
 also started failing because of this bug when gcc 4.5 was enabled for
 the sparc port with bsd.own.mk 1.679, CVS date 2011.08.17.09.18.38,
 just as had previously happened for the i386 port when gcc 4.5 was
 enabled there.

 To restore the usefulness of the babylon5.netbsd.org automated test
 machine, I have now worked around the bug there by renaming the mount
 point of the disk holding the build directories from /bracket to /b.
 With this change, the builds are working again, but the bug is of
 course still there, and the fact that it was successfully worked
 around simply by renaming a directory once again demonstrates its
 fickle nature.  It can still be reproduced at will using the procedure
 given in the initial PR submission.
 -- 
 Andreas Gustafsson, gson@gson.org

From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45269: sh fails to strip trailing newline in command substitution, breaking build
Date: Sat, 20 Aug 2011 21:29:19 +0100

 On Fri, Aug 19, 2011 at 09:05:00AM +0000, Andreas Gustafsson wrote:
 > >Number:         45269
 > >Synopsis:       sh fails to strip trailing newline in command substitution, breaking build

 This sounds similar to a problem I fixed a few years ago where it ran
 off the beginning or the buffer when removing the newlines.
 In that case it was using the wrong buffer - and was looking at
 stale on-stack data.


 	David

 -- 
 David Laight: david@l8s.co.uk

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/45269 CVS commit: src/bin/sh
Date: Tue, 23 Aug 2011 06:04:39 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Tue Aug 23 10:04:39 UTC 2011

 Modified Files:
 	src/bin/sh: expand.c

 Log Message:
 PR/45269: Andreas Gustafsson: Instead of falling off the edge when eating trailing newlines
 if the block has moved, arrange so that trailing newlines are never placed in the string
 in the first place, by accumulating them and adding them only after we've encountered a
 non-newline character. This allows also for more efficient appending since we know how much
 we need beforehand. From FreeBSD.


 To generate a diff of this commit:
 cvs rdiff -u -r1.84 -r1.85 src/bin/sh/expand.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Tue, 23 Aug 2011 10:52:39 +0000
State-Changed-Why:
Christos committed a fix, please test.


From: Andreas Gustafsson <gson@gson.org>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org,
    gson@gson.org (Andreas Gustafsson)
Subject: Re: PR/45269 CVS commit: src/bin/sh
Date: Tue, 23 Aug 2011 13:52:17 +0300

 Christos Zoulas wrote:
 >  PR/45269: Andreas Gustafsson: Instead of falling off the edge when eating trailing newlines
 >  if the block has moved, arrange so that trailing newlines are never placed in the string
 >  in the first place, by accumulating them and adding them only after we've encountered a
 >  non-newline character. This allows also for more efficient appending since we know how much
 >  we need beforehand. From FreeBSD.

 Excellent, thank you!  Will you be submitting a pullup request to releng?
 -- 
 Andreas Gustafsson, gson@gson.org

State-Changed-From-To: feedback->pending-pullups
State-Changed-By: martin@NetBSD.org
State-Changed-When: Tue, 23 Aug 2011 11:22:26 +0000
State-Changed-Why:
Waiting for [pullup-5 #1665]


From: Andreas Gustafsson <gson@gson.org>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org,
    wiz@NetBSD.org,
    Jean-Yves Migeon <jeanyves.migeon@free.fr>
Subject: Re: bin/45269 (sh fails to strip trailing newline in command substitution, breaking build)
Date: Tue, 23 Aug 2011 14:26:38 +0300

 wiz@NetBSD.org wrote:
 > Christos committed a fix, please test.

 Since the failure occurred on a TNF machine, I'm leaving the
 deployment of the fix to admins, and will retest once that's done.

 Alternatively, since Jean-Yves Migeon ran into the same bug, maybe he
 could test the fix on his own system and let us know the results.
 -- 
 Andreas Gustafsson, gson@gson.org

From: christos@zoulas.com (Christos Zoulas)
To: Andreas Gustafsson <gson@gson.org>, gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: PR/45269 CVS commit: src/bin/sh
Date: Tue, 23 Aug 2011 07:45:27 -0400

 On Aug 23,  1:52pm, gson@gson.org (Andreas Gustafsson) wrote:
 -- Subject: Re: PR/45269 CVS commit: src/bin/sh

 | Christos Zoulas wrote:
 | >  PR/45269: Andreas Gustafsson: Instead of falling off the edge when eating trailing newlines
 | >  if the block has moved, arrange so that trailing newlines are never placed in the string
 | >  in the first place, by accumulating them and adding them only after we've encountered a
 | >  non-newline character. This allows also for more efficient appending since we know how much
 | >  we need beforehand. From FreeBSD.
 | 
 | Excellent, thank you!  Will you be submitting a pullup request to releng?

 Thanks for the reproducable bug report. Without it, it would be impossible to
 find! Yes, I've issued a pullup request to -5 already.

 christos

From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: PR/45269 CVS commit: src/bin/sh
Date: Tue, 23 Aug 2011 14:16:41 +0100

 On Tue, Aug 23, 2011 at 11:50:04AM +0000, Christos Zoulas wrote:
 > The following reply was made to PR bin/45269; it has been noted by GNATS.
 > 
 > From: christos@zoulas.com (Christos Zoulas)
 > To: Andreas Gustafsson <gson@gson.org>, gnats-bugs@NetBSD.org
 > Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
 > Subject: Re: PR/45269 CVS commit: src/bin/sh
 > Date: Tue, 23 Aug 2011 07:45:27 -0400
 > 
 >  On Aug 23,  1:52pm, gson@gson.org (Andreas Gustafsson) wrote:
 >  -- Subject: Re: PR/45269 CVS commit: src/bin/sh
 >  
 >  | Christos Zoulas wrote:
 >  | >  PR/45269: Andreas Gustafsson: Instead of falling off the edge when eating trailing newlines
 >  | >  if the block has moved, arrange so that trailing newlines are never placed in the string
 >  | >  in the first place, by accumulating them and adding them only after we've encountered a
 >  | >  non-newline character. This allows also for more efficient appending since we know how much
 >  | >  we need beforehand. From FreeBSD.
 >  | 
 >  | Excellent, thank you!  Will you be submitting a pullup request to releng?
 >  
 >  Thanks for the reproducable bug report. Without it, it would be impossible to
 >  find! Yes, I've issued a pullup request to -5 already.

 Either I'm going blind or something else is wrong.
 I don't believe the netbsd code in rev 1.84 is wrong.
 The code prior to rev 1.68 was wrong, the fix from freebsd
 might have been removing the old version of that code.

 	David

 -- 
 David Laight: david@l8s.co.uk

From: Jean-Yves Migeon <jeanyves.migeon@free.fr>
To: Andreas Gustafsson <gson@gson.org>
Cc: <gnats-bugs@NetBSD.org>, <gnats-admin@netbsd.org>,
 <netbsd-bugs@netbsd.org>, <wiz@NetBSD.org>
Subject: Re: bin/45269 (sh fails to strip trailing newline in command  substitution, breaking build)
Date: Tue, 23 Aug 2011 14:25:52 +0100

  On Tue, 23 Aug 2011 14:26:38 +0300, Andreas Gustafsson wrote:
 > wiz@NetBSD.org wrote:
 >> Christos committed a fix, please test.
 >
 > Since the failure occurred on a TNF machine, I'm leaving the
 > deployment of the fix to admins, and will retest once that's done.
 >
 > Alternatively, since Jean-Yves Migeon ran into the same bug, maybe he
 > could test the fix on his own system and let us know the results.

  Sure; go ahead, I'll test ASAP.

 -- 
  Jean-Yves Migeon
  jeanyves.migeon@free.fr

From: Andreas Gustafsson <gson@gson.org>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org,
    wiz@NetBSD.org
Subject: Re: bin/45269 (sh fails to strip trailing newline in command substitution, breaking build)
Date: Tue, 23 Aug 2011 19:19:39 +0300

 wiz@NetBSD.org wrote:
 > Christos committed a fix, please test.

 Christos suggested I test it on babylon5 by setting HOST_SH, so I did.
 I can confirm that a build with HOST_SH pointing at a freshly built shell
 containing the fix successfully proceeds past the point where it used
 to fail, whereas an otherwise identical build with HOST_SH pointing at
 a shell freshly built from slightly older sources (without the fix) still
 fails.
 -- 
 Andreas Gustafsson, gson@gson.org

From: Jean-Yves Migeon <jeanyves.migeon@free.fr>
To: Andreas Gustafsson <gson@gson.org>
Cc: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, 
 netbsd-bugs@netbsd.org, wiz@NetBSD.org, 
 Christos Zoulas <christos@netbsd.org>
Subject: Re: bin/45269 (sh fails to strip trailing newline in command substitution,
 breaking build)
Date: Wed, 24 Aug 2011 00:23:03 +0200

 On 23.08.2011 13:26, Andreas Gustafsson wrote:
 > wiz@NetBSD.org wrote:
 >> Christos committed a fix, please test.
 > 
 > Since the failure occurred on a TNF machine, I'm leaving the
 > deployment of the fix to admins, and will retest once that's done.
 > 
 > Alternatively, since Jean-Yves Migeon ran into the same bug, maybe he
 > could test the fix on his own system and let us know the results.

 After rebuilding /bin/sh with christos' patch and retrying a tools
 build, I can confirm that it fixes the issue.

 Reverting to the older /bin/sh does bring the problem back, so it's very
 likely to be the root cause.

 Thanks everyone!

 -- 
 Jean-Yves Migeon
 jeanyves.migeon@free.fr

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45269 (sh fails to strip trailing newline in command
 substitution, breaking build)
Date: Wed, 24 Aug 2011 03:21:52 +0000

 On Tue, Aug 23, 2011 at 10:25:02PM +0000, Jean-Yves Migeon wrote:
  >  After rebuilding /bin/sh with christos' patch and retrying a tools
  >  build, I can confirm that it fixes the issue.
  >  
  >  Reverting to the older /bin/sh does bring the problem back, so it's very
  >  likely to be the root cause.

 Given that the problem also goes away if you turn around three times
 and sneeze over your shoulder, I think that's far from guaranteed. I
 haven't looked at this recently (I did at one point a few months ago)
 but we need to at least sort out what dsl@ is concerned about before
 calling it fixed.

 -- 
 David A. Holland
 dholland@netbsd.org

From: David Laight <david@l8s.co.uk>
To: gnats@netbsd.org, Andreas Gustafsson <gson@gson.org>
Cc: 
Subject: Re: PR/45269 CVS commit: src/bin/sh
Date: Sat, 3 Sep 2011 23:08:59 +0100

 On Tue, Aug 23, 2011 at 01:20:03PM +0000, David Laight wrote:
 >  
 >  Either I'm going blind or something else is wrong.
 >  I don't believe the netbsd code in rev 1.84 is wrong.
 >  The code prior to rev 1.68 was wrong, the fix from freebsd
 >  might have been removing the old version of that code.

 Ok, I think this is what happens - but I've not managed to generate
 a short script that exhibits the failure.

 1) expbackq() is called with 'expdest' pointing to the start of
    a chunk of stack (and not the first chunk).
    This also means it is the start of a token.

 2) grabstackstr() doesn't need to update the pointers - so expdest
    is still at the start of the stack.

 3) evalbackcmd() starts building a string on the stack (for something)
    and exceeds the blcok size - so calls growstackblock()

 4) in growstackblock() the condition:
 	if (stacknxt == stackp->space && stackp != &stackbase) {
    is true. So the memory containing the string is realloc()ed.

 All is now lost. The 'dest' pointer within expbackq() - the original
 'expdest' now points into freed memory.

 This all means I'm not sure how it works as well as it does!

 In any case this has nothing to do with how the trailing newlines are
 removed. For instance, if 'qouted' is non-zero the 'recordregion()'
 call will also go wrong.

 I think the simplest correct fix is to pass a byte count to ungrabstackstr()
 and return the new write pointer.

 	David

 -- 
 David Laight: david@l8s.co.uk

From: "Jeff Rizzo" <riz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/45269 CVS commit: [netbsd-5] src/bin/sh
Date: Wed, 2 Nov 2011 19:31:22 +0000

 Module Name:	src
 Committed By:	riz
 Date:		Wed Nov  2 19:31:21 UTC 2011

 Modified Files:
 	src/bin/sh [netbsd-5]: expand.c

 Log Message:
 Pull up following revision(s) (requested by christos in ticket #1665):
 	bin/sh/expand.c: revision 1.85
 PR/45269: Andreas Gustafsson: Instead of falling off the edge when eating
 trailing newlines
 if the block has moved, arrange so that trailing newlines are never placed in
 the string
 in the first place, by accumulating them and adding them only after we've
 encountered a
 non-newline character. This allows also for more efficient appending since we
 know how much
 we need beforehand. From FreeBSD.


 To generate a diff of this commit:
 cvs rdiff -u -r1.79 -r1.79.2.1 src/bin/sh/expand.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: jakllsch@NetBSD.org
State-Changed-When: Sun, 11 Mar 2012 23:41:40 +0000
State-Changed-Why:
pullup-5 #1665 is now complete


>Unformatted:

 Waiting on [pullup-5 #1665]

Home	PR Database Search