NetBSD Problem Report #53146

From dholland@netbsd.org  Mon Apr  2 04:53:36 2018
Return-Path: <dholland@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id CEF2B7A1CC
	for <gnats-bugs@gnats.NetBSD.org>; Mon,  2 Apr 2018 04:53:35 +0000 (UTC)
Message-Id: <20180402045335.8255684D7B@mail.netbsd.org>
Date: Mon,  2 Apr 2018 04:53:35 +0000 (UTC)
From: dholland@NetBSD.org
Reply-To: dholland@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: make: .for loop binder names aren't expanded
X-Send-Pr-Version: 3.95

>Number:         53146
>Category:       toolchain
>Synopsis:       make: .for loop binder names aren't expanded
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    rillig
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Apr 02 04:55:00 +0000 2018
>Closed-Date:    Mon May 08 10:30:31 +0000 2023
>Last-Modified:  Mon May 08 10:30:31 +0000 2023
>Originator:     David A. Holland
>Release:        NetBSD 8.99.8 (20171205)
>Organization:
>Environment:
System: NetBSD valkyrie 8.99.8 NetBSD 8.99.8 (VALKYRIE) #24: Tue Dec  5 17:30:57 EST 2017  dholland@valkyrie:/usr/src/sys/arch/amd64/compile/VALKYRIE amd64
Architecture: x86_64
Machine: amd64
>Description:

In ordinary assignments the variable name can be supplied as a
variable expansion:

        BAR=bar
        $(BAR)=baz
        meh:
                echo $(bar)

prints "baz".

However, this is not accepted in a .for loop. The name becomes the
string formed by the variable reference itself.

This:
        FOO=foo
        meh:
        .for $(FOO) in a b
                echo "$(FOO)" , "$(foo)" , "$($(FOO))"
        .endfor
prints (with make -n)
        echo "foo" , "" , "a"
        echo "foo" , "" , "b"

What seems to be happening is that the loop binds a variable named
"$(FOO)", so no variable "foo" is bound; then when make goes to
evaluate $($(FOO)) it finds that "FOO" is a variable but not a loop
variable, so it doesn't expand it yet; but then it sees that "$(FOO)"
is a loop variable and expands that, so we get the loop values in the
third print even though one might expect $($(FOO)) to evaluate to the
same thing as $(foo).

>How-To-Repeat:

as above.

>Fix:

Probably for-loop binders should be allowed to be named by variable
expansions like regular assignments; that is, the for code should do
variable expansion on the binder names. This seems like a wild thing
to do, but there doesn't seem to be any obvious reason to forbid it.

Otherwise we'd want to look for variable expansions in there and error
on them. The current behavior is clearly undesirable.

(also variable names in general shouldn't be allowed to contain "$",
but that's a broader issue)

>Release-Note:

>Audit-Trail:

From: Roland Illig <roland.illig@gmx.de>
To: gnats-bugs@NetBSD.org, Christos Zoulas <christos@netbsd.org>,
 Simon Gerraty <sjg@crufty.net>
Cc: 
Subject: Re: toolchain/53146
Date: Sun, 13 Mar 2022 22:10:15 +0100

 Hi David,

 https://gnats.netbsd.org/53146

 Your interpretation of what happens inside make is completely accurate.
 As of 2022-03-13, make allows all characters in the name of a .for
 variable, see the implementation of ForLoop_ParseVarnames in
 usr.bin/make/for.c.

 This is an unfortunate situation, and the tests in
 usr.bin/make/unit-tests do not yet cover all details of the current
 behavior.

 I suggest to forbid any variable expressions in the names of .for loop
 variables, because, as you say, it seems like a wild thing to do.  At
 least in make's "strict mode" (-dL), it should complain, probably in
 default mode as well.

 A simple search for '^\.\s*for.*?\$.*?\bin\b' didn't find any matches in
 pkgsrc.  The same search in the NetBSD tree found a false positive in
 heimdal/Makefile.rules.inc and the few test cases in
 usr.bin/make/unit-tests.  Based on these search results and the general
 notion that using this feature in a productive way is a brain twister
 anyway, I think that this "feature" is not used anywhere and that it can
 be removed.

 Roland

From: Roland Illig <roland.illig@gmx.de>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: Re: toolchain/53146
Date: Sun, 13 Mar 2022 22:28:40 +0100

 Hi David,

 https://gnats.netbsd.org/53146

 Your interpretation of what happens inside make is completely accurate.
 As of 2022-03-13, make allows all characters in the name of a .for
 variable, see the implementation of ForLoop_ParseVarnames in
 usr.bin/make/for.c.

 This is an unfortunate situation, and the tests in
 usr.bin/make/unit-tests do not yet cover all details of the current
 behavior.

 I suggest to forbid any variable expressions in the names of .for loop
 variables, because, as you say, it seems like a wild thing to do.  At
 least in make's "strict mode" (-dL), it should complain, probably in
 default mode as well.

 A simple search for '^\.\s*for.*?\$.*?\bin\b' didn't find any matches in
 pkgsrc.  The same search in the NetBSD tree found a false positive in
 heimdal/Makefile.rules.inc and the few test cases in
 usr.bin/make/unit-tests.  Based on these search results and the general
 notion that using this feature in a productive way is a brain twister
 anyway, I think that this "feature" is not used anywhere and that it can
 be removed.

 Roland

From: Roland Illig <roland.illig@gmx.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: toolchain/53146
Date: Sun, 8 May 2022 10:22:01 +0200

 A similar situation is the modifier ':@var@...@'.  Since
 usr.bin/make/var.c 1.907 from 2022-04-04, variable names containing
 dollars are invalid there.

 See usr.bin/make/unit-tests/varmod-loop-varname.mk for a few examples of
 strange variable names in the ':@var@...@' modifier.

 See usr.bin/make/unit-tests/directive-for-escape.mk for a few examples
 of strange variable names in .for loops.

From: "Roland Illig" <rillig@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53146 CVS commit: src/usr.bin/make
Date: Mon, 8 May 2023 10:24:08 +0000

 Module Name:	src
 Committed By:	rillig
 Date:		Mon May  8 10:24:07 UTC 2023

 Modified Files:
 	src/usr.bin/make: for.c
 	src/usr.bin/make/unit-tests: directive-for-errors.exp
 	    directive-for-errors.mk directive-for-escape.exp
 	    directive-for-escape.mk directive-for.exp directive-for.mk

 Log Message:
 make: disallow characters like '$' in variable names in .for loops

 Fixes PR 53146.


 To generate a diff of this commit:
 cvs rdiff -u -r1.172 -r1.173 src/usr.bin/make/for.c
 cvs rdiff -u -r1.2 -r1.3 src/usr.bin/make/unit-tests/directive-for-errors.exp
 cvs rdiff -u -r1.3 -r1.4 src/usr.bin/make/unit-tests/directive-for-errors.mk
 cvs rdiff -u -r1.17 -r1.18 \
     src/usr.bin/make/unit-tests/directive-for-escape.exp \
     src/usr.bin/make/unit-tests/directive-for.mk
 cvs rdiff -u -r1.16 -r1.17 \
     src/usr.bin/make/unit-tests/directive-for-escape.mk
 cvs rdiff -u -r1.14 -r1.15 src/usr.bin/make/unit-tests/directive-for.exp

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

Responsible-Changed-From-To: toolchain-manager->rillig
Responsible-Changed-By: rillig@NetBSD.org
Responsible-Changed-When: Mon, 08 May 2023 10:30:31 +0000
Responsible-Changed-Why:
Fixed for NetBSD 11 by disallowing strange characters in the variable
names in .for loops.  There is no practical use case for having indirect
variable names in .for loops.
Thanks for the PR.


State-Changed-From-To: open->closed
State-Changed-By: rillig@NetBSD.org
State-Changed-When: Mon, 08 May 2023 10:30:31 +0000
State-Changed-Why:
Fixed for NetBSD 11.


>Unformatted:

Home	PR Database Search