Fix build of `stand/` with base gcc

[dim]

Summary

I tried building world with base gcc, and one of the parts that fails is
stand/, with several different types of errors.

The first type of error is:

cc1: warnings being treated as errors
/usr/src/stand/i386/gptboot/gptboot.c: In function 'main':
/usr/src/stand/i386/gptboot/gptboot.c:258: warning: declaration of 'autoboot' shadows a global declaration
/usr/src/stand/common/bootstrap.h:64: warning: shadowed declaration is here

This is because the autoboot() function is now declared in
bootstrap.h, and several boot loaders use autoboot also as a local
variable. To minimize churn, I propose to make the autoboot()
function static for stand/common/boot.c, since there are no external
callers. Alternatively, it can be renamed to autoboot_actual(), or
something like that.

The second error is:

cc1: warnings being treated as errors
/usr/src/stand/userboot/userboot/main.c: In function 'extract_currdev':
/usr/src/stand/userboot/userboot/main.c:170: warning: dereferencing type-punned pointer will break strict-aliasing rules

gcc is right here, it *is* dereferencing it, so let's just insert a
plain memcpy instead.

The third type of error is:

/usr/bin/gcc  -O2 -pipe   -I/usr/src/stand/i386/btx/lib -nostdinc -I/usr/obj/usr/src/amd64.amd64/stand/libsa32 -I/usr/src/stand/libsa -D_STANDALONE -I/usr/src/sys -Ddouble=jagged-little-pill -Dfloat=floaty-mcfloatface -DLOADER_GELI_SUPPORT -I/usr/src/stand/geli -DLOADER_DISK_SUPPORT -m32 -mcpu=i386 -ffreestanding -mno-mmx -mno-sse  -msoft-float -march=i386 -I. -DBOOTPROG=\"gptboot\"  -O1  -DGPT  -DUFS1_AND_UFS2  -DSIOPRT=0x3f8  -DSIOFMT=0x3  -DSIOSPD=9600  -I/usr/src/stand/common  -I/usr/src/stand/i386/common  -I/usr/src/stand/i386/boot2  -Wall -Waggregate-return -Wbad-function-cast -Wno-cast-align  -Wmissing-declarations -Wmissing-prototypes -Wnested-externs  -Wpointer-arith -Wshadow -Wstrict-prototypes -Wwrite-strings  -Winline -Wno-pointer-sign -g -std=gnu99 -Wsystem-headers -Werror -Wno-pointer-sign   -mpreferred-stack-boundary=2 --param max-inline-insns-single=100  -c /usr/src/stand/i386/gptboot/gptboot.c -o gptboot.o
`-mcpu=' is deprecated. Use `-mtune=' or '-march=' instead.
cc1: warnings being treated as errors
/usr/src/sys/geom/eli/g_eli.h: In function 'geli_taste':
/usr/src/sys/geom/eli/g_eli.h:339: warning: inlining failed in call to 'eli_metadata_decode_v0': --param max-inline-insns-single limit reached
/usr/src/sys/geom/eli/g_eli.h:400: warning: called from here
/usr/src/sys/geom/eli/g_eli.h:365: warning: inlining failed in call to 'eli_metadata_decode_v1v2v3v4v5v6v7': --param max-inline-insns-single limit reached
/usr/src/sys/geom/eli/g_eli.h:409: warning: called from here
/usr/src/sys/geom/eli/g_eli.h:339: warning: inlining failed in call to 'eli_metadata_decode_v0': --param max-inline-insns-single limit reached
/usr/src/sys/geom/eli/g_eli.h:400: warning: called from here
/usr/src/sys/geom/eli/g_eli.h:365: warning: inlining failed in call to 'eli_metadata_decode_v1v2v3v4v5v6v7': --param max-inline-insns-single limit reached
/usr/src/sys/geom/eli/g_eli.h:409: warning: called from here
*** Error code 1

Stop.
make[2]: stopped in /usr/src/stand/i386/gptboot
*** Error code 1

This is gcc warning us that it failed to inline, but I fail to see the
value of this warning. Do we really care, and is it absolutely critical
that this function is inlined? If so, it is better to mark the function
as __forceinline, if old gcc supports that. But I'd like to propose
getting rid of -Winline instead, there is no useful information in
these warnings.

The fourth type of error is:

cc1: warnings being treated as errors
/usr/src/stand/libsa/cd9660read.c: In function 'cd9660_lookup':
/usr/src/stand/libsa/cd9660read.c:176: warning: 'len' may be used uninitialized in this function
/usr/src/stand/libsa/cd9660read.c:176: note: 'len' was declared here

Here base gcc's compile-time analysis is too dumb to realize that len
can never be used, but let's initialize it to zero just in case.

[imp]

stand/userboot/userboot/main.c

170 ↗

(On Diff #43172)

this feels wrong. I thought we'd fixed all the type punning, so there may be a less unsafe way to do that.

[imp]

stand/userboot/userboot/main.c

170 ↗

(On Diff #43172)

dev = zdev.dd;

[imp]

stand/libsa/cd9660read.c

176 ↗

(On Diff #43172)

len is initialized below, so this is a false positive.

[dim]

stand/userboot/userboot/main.c

170 ↗

(On Diff #43172)

That doesn't work:

/usr/src/stand/userboot/userboot/main.c: In function 'extract_currdev':
/usr/src/stand/userboot/userboot/main.c:170: error: incompatible types in assignment

dev is a struct disk_devdesc, while zdev.dd is a struct devdesc. It looks like the construction was meant to force copying incompatible devdesc structs, assuming that they approximately have the same memory layout.

struct disk_devdesc has:

struct disk_devdesc {
        struct devdesc  dd;
        int             d_slice;
        int             d_partition;
        uint64_t        d_offset;
};

while struct zfs_devdesc has:

struct zfs_devdesc {
        struct devdesc  dd;
        uint64_t        pool_guid;
        uint64_t        root_guid;
};

Ignoring any padding, the structs will match in size, though it is probably better to add a __packed attribute to be 100% sure. Or manually copy the individual fields.

[dim]

stand/libsa/cd9660read.c

176 ↗

(On Diff #43172)

Yeah, I remarked in the review that it is due to base gcc's limited compile time analysis. Initializing it is the cheapest way to get rid of the warning. Otherwise, we'll have to add -Wno-uninitialized to the compile flags for gcc <= 4.2.

[imp]

stand/userboot/userboot/main.c

170 ↗

(On Diff #43172)

Then this copying is wrong. Full stop. We need to find the right way to fix it.

[imp]

stand/userboot/userboot/main.c

170 ↗

(On Diff #43172)

https://reviews.freebsd.org/D15629 has this fix

[emaste]

stand/libsa/cd9660read.c

176 ↗

(On Diff #43172)

I'd be fine with -Wno-uninitialized or turn off werror for old gcc or initializing it, but if we do keep this way please add a comment like /* Unnecessary initialization to appease GCC 4.2.1 */

[imp]

wrong review

[imp]

I'm sorry, but it looks like my change replaced this review...

[dim]

Re-uploading, with a few changes:

• Removed the userboot changes, since rS334412 is a better fix
• Use -Wno-unitialized with gcc 4.2.1 for cd9660read.c
• Use -march=i386 instead of -mcpu=i386 in defs.mk. The gcc documentation states that -mcpu is a deprecated synonym for -mtune, but -mtune does not prevent instructions "higher" than i386, it merely tunes for them.

[imp]

In D15628#330215, @dim wrote:

• Use -march=i386 instead of -mcpu=i386 in defs.mk. The gcc documentation states that -mcpu is a deprecated synonym for -mtune, but -mtune does not prevent instructions "higher" than i386, it merely tunes for them.

The current floor is i486. Maybe we should consider using that instead, but I'm unsure what effects that will have on code size.

[imp]

These look good. Sorry for my screw up on the review number.

stand/defs.mk

94 ↗

(On Diff #43199)

This looks good, though I might consider i486. The -mcpu=i386 was cargo-culted through many generations of Makefile, and misinterpreted along the way to mean 32-bit code (as distinct from 64-bit code). I think this is more right having now read the docs.

[emaste]

stand/defs.mk

94 ↗

(On Diff #43199)

Or do this first, then build with -march=i486 and compare (e.g. diffoscope) the result?