NetBSD Problem Report #53023

From paul@whooppee.com  Tue Feb 13 07:18:22 2018
Return-Path: <paul@whooppee.com>
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 BB4367A1EA
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 13 Feb 2018 07:18:22 +0000 (UTC)
Message-Id: <20180213071749.6F17916E44@speedy.whooppee.com>
Date: Tue, 13 Feb 2018 15:17:49 +0800 (+08)
From: paul@whooppee.com
Reply-To: paul@whooppee.com
To: gnats-bugs@NetBSD.org
Subject: config(1) behavior change with new gcc6
X-Send-Pr-Version: 3.95

>Number:         53023
>Category:       toolchain
>Synopsis:       config(1) behavior has changed with new gcc6
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    toolchain-manager
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Feb 13 07:20:00 +0000 2018
>Closed-Date:    Tue Jun 19 11:58:13 +0000 2018
>Last-Modified:  Tue Jun 19 11:58:13 +0000 2018
>Originator:     Paul Goyette
>Release:        NetBSD 8.99.12
>Organization:
+------------------+--------------------------+----------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:          |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org |
+------------------+--------------------------+----------------------------+
>Environment:


System: NetBSD speedy.whooppee.com 8.99.12 NetBSD 8.99.12 (SPEEDY 2018-02-12 00:00:12 UTC) #3: Mon Feb 12 06:57:12 UTC 2018 paul@speedy.whooppee.com:/build/netbsd-local/obj/amd64/sys/arch/amd64/compile/SPEEDY amd64
Architecture: x86_64
Machine: amd64
>Description:
With previous gcc5, if you added "dbcool* at i2c? addr 0xxx" to the amd64
GENERIC config file, it would create an ioconf.c file that includes the
following text:

	static const struct cfiattrdata dbcoolcf_iattrdata = {
		"dbcool", 0, {
			{ NULL, NULL, 0 },
		}
	};

However, with the newly introduced gcc6, these lines do not get generated.
As a result, the eventual compilation of ioconf.c -> .o fails with the
message

	error: 'dbcoolcf_iattrdata' defined but not used

As a work-around, you can restore the correct behavior of config(1) by
removing the line

	define dbcool {}

from src/sys/dev/i2c/files.i2c

Agruably, the define line doesn't really belong there anyway, and is not
present for many other devices.  However, this _does_ trigger a behavior
change in config(1), and who knows what other behavior changes may also
have been triggered by the gcc6 update.

I've files this against config(1), but I've set the category to toolchain
so hopefully this will get the attention of the right maintainers.

>How-To-Repeat:
For now, I have removed the 'define dbcool {}' line from files.i2c since
it shouldn't have been there in the first place.  To reproduce the bug,
simply restore that line, and add

	dbcool* at iic? addr 0x2D

to the amd64 GENERIC config.  Try to build the kernel.

>Fix:
As a workaround, the 'define dbcool {}' line in dev/i2c/files.i2c has
been commented out.


>Release-Note:

>Audit-Trail:
From: Valery Ushakov <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: toolchain/53023: config(1) behavior change with new gcc6
Date: Tue, 13 Feb 2018 16:13:38 +0300

 On Tue, Feb 13, 2018 at 07:20:00 +0000, paul@whooppee.com wrote:

 > With previous gcc5, if you added "dbcool* at i2c? addr 0xxx" to the amd64
 > GENERIC config file, it would create an ioconf.c file that includes the
 > following text:
 > 
 > 	static const struct cfiattrdata dbcoolcf_iattrdata = {
 > 		"dbcool", 0, {
 > 			{ NULL, NULL, 0 },
 > 		}
 > 	};
 > 
 > However, with the newly introduced gcc6, these lines do not get generated.
 > As a result, the eventual compilation of ioconf.c -> .o fails with the
 > message
 > 
 > 	error: 'dbcoolcf_iattrdata' defined but not used

 I can't seem to parse it.  If "these lines do NOT get GENERATED" now
 then how compiling it can fail with "DEFINED but not used"?

 That variable IS generated with both new and old configs, but the new
 gcc fails with (and you have trimmed the relevant part):

 ioconf.c:436:33: error: 'dbcoolcf_iattrdata' defined but not used [-Werror=unused-const-variable=]

 which is new in gcc6, i guess.  A bit of background:

   https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00847.html

 -uwe

From: "Valeriy E. Ushakov" <uwe@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53023 CVS commit: src/sys/dev/i2c
Date: Tue, 13 Feb 2018 13:19:30 +0000

 Module Name:	src
 Committed By:	uwe
 Date:		Tue Feb 13 13:19:30 UTC 2018

 Modified Files:
 	src/sys/dev/i2c: files.i2c

 Log Message:
 Delete 'define dbcool {}' line commented out in previous.  It's bogus
 and the gcc6 correctly complains about that when it compiles ioconf.c

 PR toolchain/53023


 To generate a diff of this commit:
 cvs rdiff -u -r1.83 -r1.84 src/sys/dev/i2c/files.i2c

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

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, toolchain-manager@netbsd.org, 
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, paul@whooppee.com
Cc: 
Subject: Re: toolchain/53023: config(1) behavior change with new gcc6
Date: Tue, 13 Feb 2018 08:24:24 -0500

 On Feb 13,  1:15pm, uwe@stderr.spb.ru (Valery Ushakov) wrote:
 -- Subject: Re: toolchain/53023: config(1) behavior change with new gcc6

 |  That variable IS generated with both new and old configs, but the new
 |  gcc fails with (and you have trimmed the relevant part):
 |  
 |  ioconf.c:436:33: error: 'dbcoolcf_iattrdata' defined but not used [-Werror=unused-const-variable=]
 |  
 |  which is new in gcc6, i guess.  A bit of background:
 |  
 |    https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00847.html
 |  

 Yes, uwe is correct. This is not a gcc bug; it is a new warning. The new
 gcc ioconf.c is the same as the old gcc ioconf.c in that respect. Both
 have the iattr data for dbcool, but the gcc correctly warns because it
 is unused. It get's created because of the extra attribute that you've
 commented out.

From: Valery Ushakov <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: toolchain/53023: config(1) behavior change with new gcc6
Date: Tue, 13 Feb 2018 16:34:12 +0300

 I've just deleted the line completely and requested a pull-up to -8
 just in case.  Not sure if pulling up to -7 is worth the churn, but
 otoh it's trivial.

 -uwe

State-Changed-From-To: open->pending-pullups
State-Changed-By: pgoyette@NetBSD.org
State-Changed-When: Tue, 13 Feb 2018 21:08:16 +0000
State-Changed-Why:
Not a bug, as explained by uwe and christos. The work-around is the fix,
and it h as been committed.  Pullup-8 #548 has been requested.


From: Paul Goyette <paul@whooppee.com>
To: gnats-bugs@NetBSD.org
Cc: toolchain-manager@netbsd.org
Subject: Re: toolchain/53023: config(1) behavior change with new gcc6
Date: Wed, 14 Feb 2018 05:04:33 +0800 (+08)

 On Tue, 13 Feb 2018, Christos Zoulas wrote:

 > |  That variable IS generated with both new and old configs, but the new
 > |  gcc fails with (and you have trimmed the relevant part):
 > |
 > |  ioconf.c:436:33: error: 'dbcoolcf_iattrdata' defined but not used [-Werror=unused-const-variable=]
 > |
 > |  which is new in gcc6, i guess.  A bit of background:
 > |
 > |    https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00847.html
 > |
 >
 > Yes, uwe is correct. This is not a gcc bug; it is a new warning. The new
 > gcc ioconf.c is the same as the old gcc ioconf.c in that respect. Both
 > have the iattr data for dbcool, but the gcc correctly warns because it
 > is unused. It get's created because of the extra attribute that you've
 > commented out.

 Yes, thanks to both of you for the detailed explanation.  I totally 
 missed the fact that the error message was for a new -W in gcc6.

 Since the extraneous attribute has already been deleted, I will go ahead 
 and close this PR.

 Thanks again for the details.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53023 CVS commit: [netbsd-8] src/sys/dev/i2c
Date: Thu, 15 Feb 2018 07:52:17 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Thu Feb 15 07:52:17 UTC 2018

 Modified Files:
 	src/sys/dev/i2c [netbsd-8]: files.i2c

 Log Message:
 Pull up following revision(s) (requested by uwe in ticket #548):

 	sys/dev/i2c/files.i2c: revision 1.84 (via patch)

 Delete 'define dbcool {}' line commented out in previous.  It's bogus
 and the gcc6 correctly complains about that when it compiles ioconf.c
 PR toolchain/53023


 To generate a diff of this commit:
 cvs rdiff -u -r1.73 -r1.73.4.1 src/sys/dev/i2c/files.i2c

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

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: toolchain/53023: config(1) behavior change with new gcc6
Date: Sat, 17 Feb 2018 23:27:34 +0000

 On Tue, Feb 13, 2018 at 01:25:01PM +0000, Christos Zoulas wrote:
  >  Yes, uwe is correct. This is not a gcc bug; it is a new warning. The new
  >  gcc ioconf.c is the same as the old gcc ioconf.c in that respect. Both
  >  have the iattr data for dbcool, but the gcc correctly warns because it
  >  is unused. It get's created because of the extra attribute that you've
  >  commented out.

 Shouldn't config warn about the unused element, rather than generating
 code that then doesn't compile because the compiler notices?

 -- 
 David A. Holland
 dholland@netbsd.org

State-Changed-From-To: pending-pullups->closed
State-Changed-By: maya@NetBSD.org
State-Changed-When: Tue, 19 Jun 2018 11:58:13 +0000
State-Changed-Why:
pulled up to -8, wasn't a visible problem in earlier netbsd and probably not worth rototilling/effort for working releases.


>Unformatted:

Home	PR Database Search