NetBSD Problem Report #53034

From dholland@macaran.eecs.harvard.edu  Sat Feb 17 06:20:42 2018
Return-Path: <dholland@macaran.eecs.harvard.edu>
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 12F9E7A1BA
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 17 Feb 2018 06:20:42 +0000 (UTC)
Message-Id: <20180217050507.E94606E294@macaran.eecs.harvard.edu>
Date: Sat, 17 Feb 2018 00:05:07 -0500 (EST)
From: dholland@eecs.harvard.edu
Reply-To: dholland@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: make crash from archive member handling
X-Send-Pr-Version: 3.95

>Number:         53034
>Category:       toolchain
>Synopsis:       make crash from archive member handling
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    toolchain-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Feb 17 06:25:00 +0000 2018
>Last-Modified:  Mon Apr 02 03:15:01 +0000 2018
>Originator:     David A. Holland
>Release:        NetBSD 8.99.9 (20181207)
>Organization:
>Environment:
System: NetBSD macaran 8.99.9 NetBSD 8.99.9 (MACARAN) #45: Thu Dec 7 18:18:48 EST 2017 dholland@macaran:/usr/src/sys/arch/amd64/compile/MACARAN amd64
Architecture: x86_64
Machine: amd64
>Description:

Certain makefiles involving archive member specifications cause SIGSEGV
when in compat mode. This came up via a FreeBSD pr: 
   https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=225946

The makefile below is sufficient to reproduce the problem. Clearing
the builtin suffixes makes investigation easier.

The problem that causes the crash is that $(.TARGET) (aka $@) never
gets set for foo.c, so when make goes to set $(.IMPSRC) (aka $<) it
crashes. This only happens with both a suffix rule and an explicit
rule for foo.o, because it only crashes when $(.IMPSRC) is set the
second time; owing I guess to internal peculiarities of var.c the
first time the null value is converted to "" and only when trying to
*change* the value is there a crash.

However, that's unimportant; the question is why $(.TARGET) doesn't
get set on foo.c.

This is considerably more complicated and relies on the presence of a
dependency chain that goes through an archive member.

In particular, what happens is as follows:

* Make_ExpandUse in make.c examines the entire tree that's going to
get built. The primary purpose of this is to implement the .USE
(mis?)feature, but one of the other things it does is make sure
$(.TARGET) is set on every target that's going to be made.

* Under normal circumstances, without the archive member on the RHS of
the lib.a rule (but instead just "foo.o") it will examine all, then
lib.a, then foo.o, and then because foo.c depends on foo.o, at the
bottom of the loop when examining foo.o it will add foo.c to the list
of targets to process and then process it as well, so $(.TARGET) on
foo.c gets set.

* When the dependence chain goes through the archive member spec,
however, the archive member case of the suffix rule mechanism is what
creates the GNode for foo.o (in SuffFindArchiveDeps) and this marks it
OP_MEMBER | OP_JOIN | OP_MADE.

* Because it's OP_MADE it won't be built, apparently. (I'm not sure
how this logic is supposed to work in the wild when actually working
with real archive members.) It still depends on foo.c, but this causes
Make_ExpandUse to not add its children (thus, foo.c) to the list of
targets it examines. This causes $(.TARGET) to not get set on foo.c.

* In spite of this, foo.o gets built, and this causes foo.c to be
looked at, and that's the point at which trying to set $(.IMPSRC)
crashes.

* It seems that in Compat_Make, when foo.o is tagged OP_MADE, this
only prevents the sources of foo.o from being made, not foo.o itself,
which is why we reach Compat_Make trying to make foo.c and crashing.
Or something. It's not clear to me if this is correct or not, but it's
consistent with the documentation for the .MADE tag.

It is not at all clear to me how to fix this properly. Obviously we
can avoid the crash in various ways, and maybe that should be done as
a bandaid, but some piece of the above logic needs to be amended as
well.

>How-To-Repeat:

   --- snip ---
.SUFFIXES:
.SUFFIXES: .c .o

.c.o:
        echo boo

all: lib.a

lib.a: lib.a(foo.o)
        echo aaa
foo.o: foo.c
        echo ccc
   --- snip ---

>Fix:

Please discuss.

>Audit-Trail:
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: toolchain/53034: make crash from archive member handling
Date: Sun, 18 Feb 2018 23:05:15 +0000

 On Sat, Feb 17, 2018 at 06:25:00AM +0000, dholland@eecs.harvard.edu wrote:
  > Please discuss.

 Also see 22868, which seems to have a candidate patch for the
 underlying problem.

 -- 
 David A. Holland
 dholland@netbsd.org

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: toolchain/53034
Date: Mon, 2 Apr 2018 03:14:48 +0000

 This wasn't tagged properly to reach gnats.

    ------

 From: "Simon J. Gerraty" <sjg@netbsd.org>
 To: source-changes@NetBSD.org
 Subject: CVS commit: src/usr.bin/make
 Date: Sun, 18 Feb 2018 00:52:43 +0000
 Mail-Followup-To: source-changes-d@NetBSD.org

 Module Name:	src
 Committed By:	sjg
 Date:		Sun Feb 18 00:52:42 UTC 2018

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

 Log Message:
 Var_Set: avoid SIGSEGV if val is NULL

 A NULL val is handled gracefully (by VarAdd) when
 var is not previously set, so we ought not crash
 the second time.

 PR: 53034


 To generate a diff of this commit:
 cvs rdiff -u -r1.217 -r1.218 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.

Home	PR Database Search