svn commit: r293724 - in head/sys/boot: arm64/libarm64 common efi/boot1 efi/fdt efi/include efi/include/arm64 efi/libefi efi/loader efi/loader/arch/amd64 efi/loader/arch/arm efi/loader/arch/arm64 i...

[Ian Lepore]

On Wed, 2016-01-13 at 01:17 +0000, Steven Hartland wrote:
> 
> On 13/01/2016 00:54, Ian Lepore wrote:
> > On Wed, 2016-01-13 at 00:43 +0000, Steven Hartland wrote:
> > > On 13/01/2016 00:41, Ian Lepore wrote:
> > > > On Tue, 2016-01-12 at 02:17 +0000, Steven Hartland wrote:
> > > > > Author: smh
> > > > > Date: Tue Jan 12 02:17:39 2016
> > > > > New Revision: 293724
> > > > > URL: https://svnweb.freebsd.org/changeset/base/293724
> > > > > 
> > > > > Log:
> > > > >     Enable warnings in EFI boot code
> > > > >     
> > > > >     Set WARNS if not set for EFI boot code and fix the issues
> > > > > highlighted by
> > > > >     setting it.
> > > > >     
> > > > This appears to break arm builds when gcc is the compiler:
> > > > 
> > > > cc1: warnings being treated as errors
> > > > In file included from
> > > > /local/build/staging/freebsd/rm92/src/sys/boot/efi/fdt/../inclu
> > > > de/e
> > > > fi.h:52,
> > > >                    from
> > > > /local/build/staging/freebsd/rm92/src/sys/boot/efi/fdt/efi_fdt.
> > > > c:35
> > > > :
> > > > /local/build/staging/freebsd/rm92/src/sys/boot/efi/fdt/../inclu
> > > > de/e
> > > > fiapi.h:535: warning: function declaration isn't a prototype
> > > > In file included from
> > > > /local/build/staging/freebsd/rm92/src/sys/boot/efi/fdt/efi_fdt.
> > > > c:39
> > > > :
> > > > /local/build/staging/freebsd/rm92/src/sys/boot/efi/fdt/../../co
> > > > mmon
> > > > /bootstrap.h:332: warning: redundant redeclaration of 'delay'
> > > > /local/build/staging/freebsd/rm92/src/sys/boot/efi/fdt/../inclu
> > > > de/e
> > > > filib.h:53: warning: previous declaration of 'delay' was here
> > > > /local/build/staging/freebsd/rm92/src/sys/boot/efi/fdt/../../co
> > > > mmon
> > > > /bootstrap.h:336: warning: redundant redeclaration of 'time'
> > > > /bsdstg/rm92/obj/arm.arm/local/build/staging/freebsd/rm92/src/t
> > > > mp/u
> > > > sr/include/time.h:154: warning: previous declaration of 'time'
> > > > was
> > > > here
> > > > 
> > > > -- Ian
> > > Passes a full tinderbox so I assume your forcing gcc for some
> > > reason?
> > For several reasons.  The fact that gcc isn't the default compiler
> > doesn't mean that it's okay for code to not compile with gcc; it's
> > still a supported compiler for arm.
> > 
> > -- Ian
> Not disagreeing with that, was just curious that's all ;-)
> 
> The warnings you list seem to be detail, typical gcc, specifically:
> 
> sys/boot/efi/fdt/../include/efiapi.h:535: warning: function
> declaration isn't a prototype
> 
> I'm guessing its being picky and wants EFI_RESERVED_SERVICE to have
> void in there due to no params.
> 
> Does the following help:
> 
> Index: sys/boot/efi/fdt/Makefile
> ===================================================================
> --- sys/boot/efi/fdt/Makefile   (revision 293796)
> +++ sys/boot/efi/fdt/Makefile   (working copy)
> @@ -7,6 +7,8 @@
>   LIB=           efi_fdt
>   INTERNALLIB=
>   WARNS?=                6
> +CWARNFLAGS.gcc+=       -Wno-strict-prototypes
> +CWARNFLAGS.gcc+=       -Wno-redundant-decls
>   
>   SRCS=          efi_fdt.c
>   
> @@ -34,4 +36,6 @@ CLEANFILES+=  machine
>   
>   .include <bsd.lib.mk>
>   
> +CFLAGS+=       ${CWARNFLAGS.${COMPILER_TYPE}}
> +
>   beforedepend ${OBJS}: machine
> 
> Could you detail detail how you're switching to gcc so I an run a
> full pass on that too?
> 
> 	Regards
> 	Steve
> 

Yep, but then I had to do this because ef->off is 64 bits even on 32
bit arches, so I got a pointer/int size mismatch warning...

Index: common/load_elf.c
===================================================================
--- common/load_elf.c	(revision 293796)
+++ common/load_elf.c	(working copy)
@@ -886,7 +886,7 @@ __elfN(parse_modmetadata)(struct preloaded_file *f
 	error = __elfN(reloc_ptr)(fp, ef, v, &md, sizeof(md));
 	if (error == EOPNOTSUPP) {
 	    md.md_cval += ef->off;
-	    md.md_data = (void *)((uintptr_t)md.md_data + ef->off);
+	    md.md_data = (void *)(uintptr_t)((uintptr_t)md.md_data +
ef->off);
 	} else if (error != 0)
 	    return (error);
 #endif


That is just some special kind of ugly.  Fixing warnings is supposed to
lead to better code, but all this casting isn't better, it's just an
unreadable mess.  Man I miss the days when C was just a really powerful
macro assembler. :)

-- Ian