Bug 223564

Comment 1 Alan Somers 2017-12-01 17:55:33 UTC

I can confirm the bug.  I ran "make && sudo make -dA install" in both the working and non-working case and compared the output.  What I found was very surprising.  In the working (file name is not named "null") case, I see this:

Applying[.MAKE.DEPENDFILE] :T to "/dev/null"
  Result[.MAKE.DEPENDFILE] of :T is "null"
  Searching for null ...
     failed.
  Searching for null ...
     /usr/home/alans/freebsd/head/share/mk ...
     /usr/share/mk ...
     failed.


In the non-working (file name is "null") case, I see this:

Applying[.MAKE.DEPENDFILE] :T to "/dev/null"
  Result[.MAKE.DEPENDFILE] of :T is "null"
  Global:.MAKE.MAKEFILES = /usr/home/alans/freebsd/head/share/mk/sys.mk /usr/home/alans/freebsd/head/share/mk/local.sys.env.mk /usr/home/alans/freebsd/head/share/mk/src.sys.env.mk /usr/home/alans/freebsd/head/s
  Parse_SetInput: file /usr/obj/usr/home/alans/freebsd/head/amd64.amd64/tests/bug/testing/null, line 0, fd -1, nextbuf 0x414cc0, arg 0x800d647c0
  Global:.PARSEDIR = /usr/obj/usr/home/alans/freebsd/head/amd64.amd64/tests/bug/testing
  Global:.PARSEFILE = null
  ParseSetParseFile: ${.PARSEDIR} = `/usr/obj/usr/home/alans/freebsd/head/amd64.amd64/tests/bug/testing' ${.PARSEFILE} = `null'
  ParseReadLine (3): 'atf_test_case "dummy" "cleanup"'
  ParseDoDependency(atf_test_case "dummy" "cleanup")

So it looks like in every case make thinks that /dev/null is a depend file (the thing that is normally named Makefile.depend) for the target, and tries to read commands from it.  Also, for some reason it strips off the dirname.  Most of the time, the file can't be found.  But if a file named "null" exists, then make will try to read commands from it.  The target doesn't even need to be named "null".  The bug can be reproduced as long as a file named "null" is present that contains invalid make syntax.  For example:

$ cd /usr/src/tests/etc/rc.d
$ echo "a b c" > null
$ make && sudo make install
make: "/usr/src/tests/etc/rc.d/null" line 1: Need an operator
make: Fatal errors encountered -- cannot continue
make: stopped in /usr/src/tests/etc/rc.d

Comment 2 Bryan Drewery 2017-12-01 18:06:17 UTC

It's an optimization to avoid reading the depend files. I'll fix this later today.

Comment 3 Bryan Drewery 2017-12-02 00:07:25 UTC

So for 'make install' I am setting .MAKE.DEPENDFILE=/dev/null to avoid reading
and parsing all of the large .depend.*.o files.

There's some problems with this.

bmake's main.c:
 /* In particular suppress .depend for '-r -V .OBJDIR -f /dev/null' */
 if (!noBuiltins || !printVars) {
     makeDependfile = Var_Subst(NULL, "${.MAKE.DEPENDFILE:T}",
         VAR_CMD, VARF_WANTRES);
     doing_depend = TRUE;
     (void)ReadMakefile(makeDependfile, NULL);
     doing_depend = FALSE;
 }


It is stripping away all path information for .MAKE.DEPENDFILE even
though ReadMakefile() has explicit support for absolute pathed
files...

/* if we've chdir'd, rebuild the path name */
if (strcmp(curdir, objdir) && *fname != '/') {
...
} else {
        fd = open(fname, O_RDONLY);
        if (fd != -1)
                goto found;
}


I'm not sure why :T is used on .MAKE.DEPENDFILE there.

More importantly though I would like some value to set .MAKE.DEPENDFILE
to avoid attempting any open(2) at all.  Currently it is actually trying
to open .CURDIR/null and .OBJDIR/null when no attempts are wanted.

~/git/freebsd/bin/sh # truss env MK_TESTS=no make -n install 2>&1|grep null
openat(AT_FDCWD,"/root/git/freebsd/bin/sh/null",O_RDONLY,00) ERR#2 'No such file or directory'
openat(AT_FDCWD,"/scratch/obj/root/git/freebsd/amd64.amd64/bin/sh/null",O_RDONLY,00) = 3 (0x3)

Comment 4 Simon J. Gerraty 2017-12-04 01:53:18 UTC

(In reply to Bryan Drewery from comment #3)

BTW the .MAKE.DEPENDFILE=/dev/null should be guarded by .if ${MK_DIRDEPS_BUILD} == "no"
otherwise it was causing us problems.

As to :T I vaguely recall that was done for consistency, but it was a long time ago ;-)

Comment 5 Bryan Drewery 2018-01-24 18:04:28 UTC

(In reply to Simon J. Gerraty from comment #4)
> (In reply to Bryan Drewery from comment #3)
> 
> BTW the .MAKE.DEPENDFILE=/dev/null should be guarded by .if
> ${MK_DIRDEPS_BUILD} == "no"
> otherwise it was causing us problems.

I've built DIRDEPS_BUILD locally quite a few times since adding that and haven't observed any problems. What have you seen?

> 
> As to :T I vaguely recall that was done for consistency, but it was a long
> time ago ;-)

Can we change it, or give me a facility to make bmake NOT read .MAKE.DEPENDFILE if a variable is set? Like .MAKE.NOREADDEPEND

Comment 6 Mateusz Guzik 2020-10-25 09:12:44 UTC

Is this getting anywhere? Said "null" lookups are avoidably creating a lot of negative entries during for example tinderbox. Not the end of the world but would be good to sort this out.

Comment 7 Simon J. Gerraty 2020-10-25 17:29:38 UTC

(In reply to Mateusz Guzik from comment #6)
Sorry, slipped off my radar.  I have a patch that should help.
FYI

diff -r cda9e4e14273 bmake/main.c
--- a/bmake/main.c      Thu Oct 22 11:18:04 2020 -0700
+++ b/bmake/main.c      Sun Oct 25 10:28:14 2020 -0700
@@ -1449,12 +1449,15 @@
 
        /* In particular suppress .depend for '-r -V .OBJDIR -f /dev/null' */
        if (!noBuiltins || !printVars) {
-           (void)Var_Subst("${.MAKE.DEPENDFILE:T}",
+           /* ignore /dev/null and anything starting with "no" */
+           (void)Var_Subst("${.MAKE.DEPENDFILE:N/dev/null:Nno*:T}",
                VAR_CMD, VARE_WANTRES, &makeDependfile);
-           /* TODO: handle errors */
-           doing_depend = TRUE;
-           (void)ReadMakefile(makeDependfile);
-           doing_depend = FALSE;
+           if (makeDependfile[0] != '\0') {
+               /* TODO: handle errors */
+               doing_depend = TRUE;
+               (void)ReadMakefile(makeDependfile);
+               doing_depend = FALSE;
+           }
        }
 
        if (enterFlagObj)

Comment 8 Mateusz Guzik 2020-10-25 17:45:05 UTC

Thanks.

I had to patch it a little as the local variant seems to be older. I can confirm the null lookups are gone and at least "make kernel" did not run into trouble.

diff --git a/contrib/bmake/main.c b/contrib/bmake/main.c
index f27320c5745..e567166a593 100644
--- a/contrib/bmake/main.c
+++ b/contrib/bmake/main.c
@@ -1390,11 +1390,14 @@ main(int argc, char **argv)
 
        /* In particular suppress .depend for '-r -V .OBJDIR -f /dev/null' */
        if (!noBuiltins || !printVars) {
-           makeDependfile = Var_Subst("${.MAKE.DEPENDFILE:T}",
+           /* ignore /dev/null and anything starting with "no" */
+           makeDependfile = Var_Subst("${.MAKE.DEPENDFILE:N/dev/null:Nno*:T}",
                VAR_CMD, VARE_WANTRES);
-           doing_depend = TRUE;
-           (void)ReadMakefile(makeDependfile);
-           doing_depend = FALSE;
+           if (makeDependfile[0] != '\0') {
+                   doing_depend = TRUE;
+                   (void)ReadMakefile(makeDependfile);
+                   doing_depend = FALSE;
+           }
        }
 
        if (enterFlagObj)

Comment 9 Simon J. Gerraty 2020-11-07 21:52:00 UTC

Fixed in bmake-20201101