From owner-svn-src-all@FreeBSD.ORG Mon Mar 3 07:49:15 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 07AB95CF; Mon, 3 Mar 2014 07:49:15 +0000 (UTC) Received: from mail107.syd.optusnet.com.au (mail107.syd.optusnet.com.au [211.29.132.53]) by mx1.freebsd.org (Postfix) with ESMTP id BCE46684; Mon, 3 Mar 2014 07:49:14 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail107.syd.optusnet.com.au (Postfix) with ESMTPS id CAB3CD44EF5; Mon, 3 Mar 2014 18:49:04 +1100 (EST) Date: Mon, 3 Mar 2014 18:49:00 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Ian Lepore Subject: Re: svn commit: r262696 - head/sys/arm/arm In-Reply-To: <201403022125.s22LPXgd078485@svn.freebsd.org> Message-ID: <20140303181837.I859@besplex.bde.org> References: <201403022125.s22LPXgd078485@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=Wu4Sb7vv c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=WPRLij4gDd8A:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=9i1iI9n1qKuvu3n0atMA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Mar 2014 07:49:15 -0000 On Sun, 2 Mar 2014, Ian Lepore wrote: > Log: > Add __attribute__((used)) so that the delay implementation doesn't get > optimized away as unreferenced, causing linker errors when trying to > resolve the weak reference to the missing function. Why not use the standard FreeBSD macro __used? Hard-coded __attribute__(())s are not only unportable; they are gross style bugs. They were all replaced by FreeBSD macros in old versions of FreeBSD. > Modified: head/sys/arm/arm/mpcore_timer.c > ============================================================================== > --- head/sys/arm/arm/mpcore_timer.c Sun Mar 2 19:46:03 2014 (r262695) > +++ head/sys/arm/arm/mpcore_timer.c Sun Mar 2 21:25:32 2014 (r262696) > @@ -370,6 +370,7 @@ DRIVER_MODULE(mp_tmr, simplebus, arm_tmr > * nothing > */ > static void > +__attribute__((used)) /* Must emit function code for the weak ref below. */ > arm_tmr_DELAY(int usec) > { > int32_t counts_per_usec; The bug was really in the weak reference macro. It is hard-coded in the same unportable asm for all arches and compilers (so the unportable gccism's in it are much larger than for __attribute__(())). Thus the compiler cannot see it. It is missing C parts that declare the referred-to identifier as __used. The __weak_reference() macro has dubious support for __STDC__ (it doesn't really suoport !__STDC__, but only cpp -traditional). Some macros in sys/cdefs.h has been broken to pretend to support lint. This actually breaks lint, by defining away the macros so that lint can't detect their unportabilities. One of these is __unused. But the support for lint is so broken that __unused is not one of these. Neither is __weak_reference() or __attribute__(). The last is intentional. All of the simpler macros like __used and __unused are just aliases for __attribute__(()). These should be defined away for lint if and only if they are just optimization hints or other hints not related to the correctness of the program, e.g., __pure2. The others, and __attribute__() itself, should be left undefined so that lint barfs on them if they are are ever used. lint has a -g flag that supports some gcc extensions. These don't include __attribute__(), so -g was almost useless in practice in 1993. It is more useless now. lint even has a hint about unconditionally defining away __attribute__(). It has a line of source to do this, but this line is ifdefed out in FreeBSD. Bruce