Date: Mon, 3 Mar 2014 18:49:00 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ian Lepore <ian@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r262696 - head/sys/arm/arm Message-ID: <20140303181837.I859@besplex.bde.org> In-Reply-To: <201403022125.s22LPXgd078485@svn.freebsd.org> References: <201403022125.s22LPXgd078485@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140303181837.I859>