NetBSD Problem Report #49085

From jarmo.jaakkola@roskakori.fi  Thu Aug  7 16:03:18 2014
Return-Path: <jarmo.jaakkola@roskakori.fi>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 16F95AA9B4
	for <gnats-bugs@gnats.NetBSD.org>; Thu,  7 Aug 2014 16:03:18 +0000 (UTC)
Message-Id: <20140807160314.559375312@roskakori.fi>
Date: Thu,  7 Aug 2014 19:03:14 +0300 (EEST)
From: Jarmo Jaakkola <jarmo.jaakkola@roskakori.fi>
Reply-To: Jarmo Jaakkola <jarmo.jaakkola@roskakori.fi>
To: gnats-bugs@gnats.NetBSD.org
Subject: make(1): several parsing errors
X-Send-Pr-Version: 3.95

>Number:         49085
>Category:       toolchain
>Synopsis:       make: some expansions and line continuations are parsed incorrectly
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    toolchain-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Aug 07 16:05:00 +0000 2014
>Last-Modified:  Mon May 09 22:46:55 +0000 2022
>Originator:     Jarmo Jaakkola <jarmo.jaakkola@roskakori.fi>
>Release:        NetBSD 6.1.2_PATCH
>Organization:
>Environment:
System: NetBSD kotoisa.roskakori.fi 6.1.2_PATCH NetBSD 6.1.2_PATCH (KOTOISA) #5: Mon Jan 20 17:01:44 EET 2014 jammuli@kotoisa.roskakori.fi:/usr/src/sys/arch/amd64/compile/KOTOISA amd64
Architecture: x86_64
Machine: amd64
>Description:

I have found several parsing bugs in make(1).  Here's a rundown of them:

Issue 1: variable modifiers don't work with D and F forms of local variables
Issue 2: $(?D) and $(?F) expand to null string
Issue 3: line continuations in commands are handled incorrectly
Issue 4: lib(member) as the last target before ':' results in syntax error

I have prepared a patch to fix all of these and will submit it
as a follow-up.



Issue 1: variable modifiers don't work with D and F forms of local variables
========

When variable modifiers are used with the POSIX specified D and F forms
of local variables (e.g. $(@D:.c=.y)), the modifier is not processed
and the colon is treated as the closing parentheses/brace.  The only
variable modifier allowed by posix is the string replacement shown in
the example, but it affects all modifiers equally.

--<Start>---------------------------------------------------------------
$ cat >Makefile1 <<EOF
dir.d/target.t:
	@echo 'Expected: dir.d/target.t dir target.T'
	@echo 'Actual:   \$(@) \$(@D:.d=) \${@F:.t=.T}'
EOF
$ make -rf Makefile1
Expected: dir.d/target.t dir target.T
Actual:   dir.d/target.t dir.d.d=) target.t.t=.T}
--<End>-----------------------------------------------------------------

The problem is a premature exit from the var.c:Var_Parse() function after
a D or F modified version of a variable is detected and the following
character (assumed to be the closing parentheses/brace) is eaten.  This
bypasses the modifier processing that occurs later in the function.



Issue 2: $(?D) and $(?F) expand to null string
========

All (POSIX) local variables should be available as D and F variants, but
$(?) isn't.

--<Start>---------------------------------------------------------------
$ cat >Makefile2 <<EOF
target: dir/source1 source2
	@echo 'Expected: dir/source1 source2 dir . source1 source2'
	@echo 'Actual:   \$(?) \$(?D) \$(?F)'

dir/source1 source2:
EOF
$ make -rf Makefile2
Expected: dir/source1 source2 dir . source1 source2
Actual:   dir/source1 source2  
--<End>-----------------------------------------------------------------

Again the problem is with the var.c:Var_Parse() function.  The strchr()
check for local variable names does not include '?'.



Issue 3: line continuations in commands are handled incorrectly
========

All line continuations and the whitespace on the following line
is collapsed into one space.  Command lines should be treated
differently according to POSIX.

Quoting POSIX:
    When an escaped <newline> (one preceded by a backslash) is found
    anywhere in the makefile except in a command line, it shall be
    replaced, along with any leading white space on the following line,
    with a single <space>.  When an escaped <newline> is found in a
    command line in a makefile, the command line shall contain the
    backslash, the <newline>, and the next line, except that the first
    character of the next line shall not be included if it is a <tab>.

--<Start>---------------------------------------------------------------
$ cat >Makefile3 <<EOF
target:
	@printf 'Expected:\na\nb\nc\n'
	@echo 'Actual:'
	@echo 'aXbXc' | sed -e 's/X/\\
	/g'
EOF
$ make -rf Makefile3
Expected:
a
b
c
Actual:
a b c
--<End>-----------------------------------------------------------------

The problem is in parse.c:ParseGetLine().  The handling of escaped
newlines does not take into account command lines.



Issue 4: lib(member) as the last target before ':' results in syntax error
========

When an archive member target ("lib(member)") is specified as the last
target on a dependency line, parsing of the line fails.

--<Start>---------------------------------------------------------------
$ cat >Makefile4 <<EOF
lib: lib(foo.o)
	ar -s \$(@)

# No it isn't missing, its right there
#         v
lib(foo.o): foo.c
	cc -c -o \$(%) foo.c
	ar -rc \$(@) \$(%)
	rm -f \$(%)

foo.c:
	echo 'int foo = 1;' >\$(@)

# Expected:
# echo 'int foo = 1;' >foo.c
# cc -c -o foo.o foo.c
# ar -rc lib foo.o
# rm -f foo.o
# ar -s lib
EOF
$ make -rf Makefile4
make: "/some/dir/Makefile4" line 6: Missing dependency operator
make: Fatal errors encountered -- cannot continue
make: stopped in /some/dir
--<End>-----------------------------------------------------------------

The parsing error goes away when a dummy target is inserted between
the archive member and the dependency operator.  Even after
the workaround this makefile won't work because the value of $(@) is
incorrect for archive member targets.  I have another PR prepared for
that one and will post its number as a follow-up.

The problem is in parse.c:ParseDoDependency().  Arch_ParseArchive()
advances line (start of next target name) instead of cp (current
position).  The rest of the loop is based on cp with line being reset
to cp as the last statement in the loop.  The rest of the function is
based on cp too, assuming that cp points at the dependency operator
after the loop finishes.  One would think that this would result in an
infinite loop, but this does not happen because the loop condition
happens to be based on line instead of cp.


>How-To-Repeat:

Inlined into the description.

>Fix:

I will post a patch as a follow-up.

>Release-Note:

>Audit-Trail:
From: Jarmo Jaakkola <jarmo.jaakkola@roskakori.fi>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: toolchain/49085: make(1): several parsing errors
Date: Thu, 7 Aug 2014 19:17:34 +0300

 The other PR mentioned in issue 4 is #49086 (see issue 1)

     http://gnats.netbsd.org/49086

 -- 
 Jarmo Jaakkola

From: Jarmo Jaakkola <jarmo.jaakkola@roskakori.fi>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: toolchain/49085: make(1): several parsing errors
Date: Thu, 7 Aug 2014 19:29:07 +0300

 The promised patch can be downloaded from

     http://roskakori.fi/software/bugs/NetBSD/PR49085/patch

 -- 
 Jarmo Jaakkola

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49085 CVS commit: src
Date: Sat, 23 Aug 2014 10:50:24 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Sat Aug 23 14:50:24 UTC 2014

 Modified Files:
 	src/tests/usr.bin/make: t_make.sh
 	src/usr.bin/make: make.1 parse.c var.c
 Added Files:
 	src/tests/usr.bin/make: d_posix.mk d_posix.out

 Log Message:
 PR/49085: Jarmo Jaakkola: fix several parsing errors

 Don't exit from var.c:Var_Parse() before possible modifiers are handled
 on D and F modified versions of local variables.  Properly expand $(?D)
 and $(?F) too.

 Make line continuations in rule's commands POSIX compliant.

 Fix the syntax error caused by lib(member) as the last target before
 a dependency operator.

 Document the line continuation change in the manual page.  Also talk
 more about the POSIX style local variables and their modifiers.

 Add tests covering the fixed problems into d_posix.mk.  The test is
 a known failure at the moment because of PR 49086 and PR 49092.

 [XXX: unconverted tests]


 To generate a diff of this commit:
 cvs rdiff -u -r0 -r1.3 src/tests/usr.bin/make/d_posix.mk \
     src/tests/usr.bin/make/d_posix.out
 cvs rdiff -u -r1.2 -r1.3 src/tests/usr.bin/make/t_make.sh
 cvs rdiff -u -r1.230 -r1.231 src/usr.bin/make/make.1
 cvs rdiff -u -r1.198 -r1.199 src/usr.bin/make/parse.c
 cvs rdiff -u -r1.186 -r1.187 src/usr.bin/make/var.c

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

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49085 CVS commit: src/usr.bin/make/unit-tests
Date: Sat, 23 Aug 2014 11:02:04 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Sat Aug 23 15:02:04 UTC 2014

 Modified Files:
 	src/usr.bin/make/unit-tests: Makefile
 Added Files:
 	src/usr.bin/make/unit-tests: posix1.exp posix1.mk

 Log Message:
 Convert test in PR/49085


 To generate a diff of this commit:
 cvs rdiff -u -r1.44 -r1.45 src/usr.bin/make/unit-tests/Makefile
 cvs rdiff -u -r0 -r1.1 src/usr.bin/make/unit-tests/posix1.exp \
     src/usr.bin/make/unit-tests/posix1.mk

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

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49085 CVS commit: src/tests/usr.bin/make
Date: Sat, 23 Aug 2014 11:10:18 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Sat Aug 23 15:10:18 UTC 2014

 Modified Files:
 	src/tests/usr.bin/make: t_make.sh

 Log Message:
 undo previous bogus merge of PR49085


 To generate a diff of this commit:
 cvs rdiff -u -r1.4 -r1.5 src/tests/usr.bin/make/t_make.sh

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

From: "David A. Holland" <dholland@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49085 CVS commit: src/usr.bin/make
Date: Sat, 13 Sep 2014 23:21:01 +0000

 Module Name:	src
 Committed By:	dholland
 Date:		Sat Sep 13 23:21:01 UTC 2014

 Modified Files:
 	src/usr.bin/make: var.c

 Log Message:
 Recognize the crazy POSIXisms $(?D) and $(?F); issue 2 in PR 49085
 from Jarmo Jaakkola.


 To generate a diff of this commit:
 cvs rdiff -u -r1.189 -r1.190 src/usr.bin/make/var.c

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

From: "David A. Holland" <dholland@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49085 CVS commit: src/usr.bin/make
Date: Sun, 14 Sep 2014 02:32:51 +0000

 Module Name:	src
 Committed By:	dholland
 Date:		Sun Sep 14 02:32:51 UTC 2014

 Modified Files:
 	src/usr.bin/make: var.c

 Log Message:
 Fix handling of the silly $(@D) $(@F) etc. vars so modifiers on them
 work. This is issue #1 in PR 49085 from Jarmo Jaakkola, but I've used
 a different and cleaner patch this time.


 To generate a diff of this commit:
 cvs rdiff -u -r1.190 -r1.191 src/usr.bin/make/var.c

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

From: "David A. Holland" <dholland@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49085 CVS commit: src/usr.bin/make
Date: Sun, 14 Sep 2014 03:13:49 +0000

 Module Name:	src
 Committed By:	dholland
 Date:		Sun Sep 14 03:13:49 UTC 2014

 Modified Files:
 	src/usr.bin/make: make.1

 Log Message:
 Improvements pertaining to shell commands and chdir. Some of this
 appeared in the PR 49085 changes, even though it's not actually
 relevant there except tangentially. However, I've reworked most of
 that for clarity and added some more.


 To generate a diff of this commit:
 cvs rdiff -u -r1.238 -r1.239 src/usr.bin/make/make.1

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

From: "David A. Holland" <dholland@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49085 CVS commit: src/usr.bin/make
Date: Sun, 14 Sep 2014 03:50:28 +0000

 Module Name:	src
 Committed By:	dholland
 Date:		Sun Sep 14 03:50:28 UTC 2014

 Modified Files:
 	src/usr.bin/make: make.1

 Log Message:
 Improve the documentation of rule-local variables. Cherry-picked from
 the PR 49085 changes, with some adjustments by me.


 To generate a diff of this commit:
 cvs rdiff -u -r1.239 -r1.240 src/usr.bin/make/make.1

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

From: "David A. Holland" <dholland@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49085 CVS commit: src/usr.bin/make
Date: Sun, 14 Sep 2014 04:32:07 +0000

 Module Name:	src
 Committed By:	dholland
 Date:		Sun Sep 14 04:32:07 UTC 2014

 Modified Files:
 	src/usr.bin/make: make.1

 Log Message:
 Add some notes on compatibility with other make dialects.

 I was originally intending to preserve some of Jarmo Jaakkola's notes
 on POSIX make from the PR 49085 changes... but really there's no point
 wandering into details about $? and such when the big picture is
 "almost everything in this manual works only in BSD make".

 Maybe the exact details can be stuffed into a chapter of the mythical
 make reference manual if that ever gets (re)written.


 To generate a diff of this commit:
 cvs rdiff -u -r1.240 -r1.241 src/usr.bin/make/make.1

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

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49085 CVS commit: src/tests/usr.bin/make
Date: Tue, 27 Jan 2015 12:57:14 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Tue Jan 27 12:57:14 UTC 2015

 Modified Files:
 	src/tests/usr.bin/make: t_make.sh

 Log Message:
 Mark the failing tests as expected failure and point to PR toolchain/49085.


 To generate a diff of this commit:
 cvs rdiff -u -r1.6 -r1.7 src/tests/usr.bin/make/t_make.sh

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

>Unformatted:

Home	PR Database Search