On Fri, 21 Jul 2017, Konstantin Belousov wrote:

On Thu, Jul 20, 2017 at 04:02:02PM -0700, Ryan Libby wrote:
On Thu, Jul 20, 2017 at 3:33 AM, Konstantin Belousov
<kostik...@gmail.com> wrote:
On Thu, Jul 20, 2017 at 02:08:30AM -0700, Ryan Libby wrote:
On Thu, Jul 20, 2017 at 1:01 AM, Bruce Evans <b...@optusnet.com.au> wrote:
[...]
This bug is not very common.  There seem to be no instances of it in
<sys> (only sys/cdefs.h uses __attribute__(()), and it seems to use
underscores for all the attributes).  Grepping sys/include/*.h for
attribute shows the following bugs:

X amd64/include/efi.h:#define   EFIABI_ATTR     __attribute__((ms_abi))
X i386/include/efi.h:#define    EFIABI_ATTR /* __attribute__((ms_abi)) */ /* 
clang fails with this */
X ofed/include/rdma/ib_user_mad.h:typedef unsigned long 
__attribute__((aligned(4))) packed_ulong;
X ofed/include/rdma/ib_smi.h:} __attribute__ ((packed));
X ofed/include/rdma/ib_mad.h:} __attribute__ ((packed));
X ofed/include/rdma/ib_mad.h:} __attribute__ ((packed));

The commented-out ms_abi was only a style bug.  Now it is a larger style
bug -- it is different and worse than amd64.

I'm not sure what to do about i386 there (again beyond fixing up the
spelling in the comment).  Maybe the unsupported architectures should
just not be declaring EFIABI_ATTR at all?  (Thoughts, kib?)

I think i386 should be treated exactly same as amd64, i.e. EFIABI_ATTR
should be not defined if gcc < 4.4. Or I do not understand the scope
of the question.

It depends on whether to comment is correct.

After googling around [1] and a quick check of the spec [2], it now
seems to me that the i386 comment might just be erroneous.  I think the

Testing shows that it is still correct.  Using this attribute generates
the warning that it is ignored.

right solution for sys/i386/include/efi.h may just be to delete the
comment and leave that EFIAPI_ATTR macro definition as empty (always, no
compiler version check) in order to use the native calling convention.

[1] http://wiki.osdev.org/UEFI#Calling_Conventions
[2] http://www.uefi.org/sites/default/files/resources/UEFI_Spec_2_7.pdf

At very least, the UEFI Spec requires 16-byte alignment of the stack.
This is not guaranteed by our i386 ABI.

This UEFI pessimization is guaranteed to be not done by our i386 ABI.  We
use -mpreferred-stack-boundary=2 to prevent it for gcc, and this 4-byte
alignment is the default for clang.  This avoids the pessimization of
wasting time and space by padding the stack to a 16-byte boundary on
(non-null on 3/4 of function calls).

__aligned(16) on local variables is broken by this for gcc-4.2.1.
gcc-4.2.1 doesn't understand its own option, and doesn't generate the
necessary andl of the stack to align it in functions that need it.
gcc by default has the pessimization.

clang handles stack alignment correctly, except it doesn't support
-mpreferred-stack-boundary to control it.  It hard-codes 4-byte alignment
on i386, and does the necessary andl of the stack in functions that need
it.  Since clang on i386 also doesn't support ms_abi to change this, the
hard-coding seems to be perfect.

Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to