| Message ID | 5458D914.1070608@redhat.com |
|---|---|
| State | New |
| Headers | show |
> On Nov 4, 2014, at 5:48 AM, Laszlo Ersek <lersek@redhat.com> wrote: > > diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c > b/MdePkg/Library/BasePrintLib/PrintLibInternal.c > index 8dc5ec7..fbb3726 100644 > --- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c > +++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c > @@ -680,10 +680,20 @@ BasePrintLibSPrintMarker ( > if (TmpGuid == NULL) { > ArgumentString = "<null guid>"; > } else { > + UINTN (EFIAPI * volatile PrintFunction) ( > + OUT CHAR8 *StartOfBuffer, > + IN UINTN BufferSize, > + IN UINTN Flags, > + IN CONST CHAR8 *FormatString, > + ... > + ); > + > + PrintFunction = BasePrintLibSPrint; > + > GuidData1 = ReadUnaligned32 (&(TmpGuid->Data1)); > GuidData2 = ReadUnaligned16 (&(TmpGuid->Data2)); > GuidData3 = ReadUnaligned16 (&(TmpGuid->Data3)); > - BasePrintLibSPrint ( > + PrintFunction ( > ValueBuffer, > MAXIMUM_VALUE_CHARACTERS, > 0, > ---------------- > > With this patch, GUIDs are printed okay with -Os. > > (Of course it's not edk2 that needs to be fixed.) > But pedantically you need change the definition of BasePrintLibSPrint() to include EFIAPI. If you look at BasePrintLibSPrintMarker() (and some of the other routines) you will notice a BASE_LIST and a VA_LIST. We had an API that exposed a VA_LIST (well code that was casting to VA_LIST) in the report status code stack and it forced use to use our own made up BASE_LIST concept to get it to work. I think you are going to hit similar issues mixing calling conventions. So my 1st question is why do you need to mix calling conventions, and depend on EFIAPI for interoperability. Why not just change the ABI on all functions? Problems with the mixed calling convention: 1) All assembly routines must be marked as EFIAPI, or the C code will generate the wrong calling convention. Not an issue in the MdePkg, but potentially an issue in other packages. 2) The var arg chain needs to be constant. I think for i386 you get lucky and the calling conventions are close enough it kind of works, but for X64 they are very different. Even if you force VA_LIST to be the Microsoft one, it is not clear to me that forces the compiler to treat every … the Microsoft way. Thanks, Andrew Fish ------------------------------------------------------------------------------
On 11/04/14 18:28, Andrew Fish wrote: > >> On Nov 4, 2014, at 5:48 AM, Laszlo Ersek <lersek@redhat.com >> <mailto:lersek@redhat.com>> wrote: >> >> diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c >> b/MdePkg/Library/BasePrintLib/PrintLibInternal.c >> index 8dc5ec7..fbb3726 100644 >> --- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c >> +++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c >> @@ -680,10 +680,20 @@ BasePrintLibSPrintMarker ( >> if (TmpGuid == NULL) { >> ArgumentString = "<null guid>"; >> } else { >> + UINTN (EFIAPI * volatile PrintFunction) ( >> + OUT CHAR8 *StartOfBuffer, >> + IN UINTN BufferSize, >> + IN UINTN Flags, >> + IN CONST CHAR8 *FormatString, >> + ... >> + ); >> + >> + PrintFunction = BasePrintLibSPrint; >> + >> GuidData1 = ReadUnaligned32 (&(TmpGuid->Data1)); >> GuidData2 = ReadUnaligned16 (&(TmpGuid->Data2)); >> GuidData3 = ReadUnaligned16 (&(TmpGuid->Data3)); >> - BasePrintLibSPrint ( >> + PrintFunction ( >> ValueBuffer, >> MAXIMUM_VALUE_CHARACTERS, >> 0, >> ---------------- >> >> With this patch, GUIDs are printed okay with -Os. >> >> (Of course it's not edk2 that needs to be fixed.) >> > > But pedantically you need change the definition of BasePrintLibSPrint() > to include EFIAPI. I tried that (without applying above patch, before posting my message); it didn't help. Thanks, Laszlo > > If you look at BasePrintLibSPrintMarker() (and some of the other > routines) you will notice a BASE_LIST and a VA_LIST. We had an API that > exposed a VA_LIST (well code that was casting to VA_LIST) in the report > status code stack and it forced use to use our own made up BASE_LIST > concept to get it to work. I think you are going to hit similar issues > mixing calling conventions. > > So my 1st question is why do you need to mix calling conventions, and > depend on EFIAPI for interoperability. Why not just change the ABI on > all functions? > > Problems with the mixed calling convention: > 1) All assembly routines must be marked as EFIAPI, or the C code will > generate the wrong calling convention. Not an issue in the MdePkg, but > potentially an issue in other packages. > 2) The var arg chain needs to be constant. I think for i386 you get > lucky and the calling conventions are close enough it kind of works, but > for X64 they are very different. Even if you force VA_LIST to be the > Microsoft one, it is not clear to me that forces the compiler to treat > every … the Microsoft way. > > Thanks, > > Andrew Fish > > > > ------------------------------------------------------------------------------ > > > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > ------------------------------------------------------------------------------
> On Nov 4, 2014, at 12:15 PM, Scott Duplichan <scott@notabs.org> wrote: > > But pedantically you need change the definition of BasePrintLibSPrint() to include EFIAPI. > > If you look at BasePrintLibSPrintMarker() (and some of the other routines) you will notice a BASE_LIST and a VA_LIST. We had an API that exposed a VA_LIST (well code that was casting to VA_LIST) in the report status code stack and it forced use to use our own made up BASE_LIST concept to get it to work. I think you are going to hit similar issues mixing calling conventions. > > So my 1st question is why do you need to mix calling conventions, and depend on EFIAPI for interoperability. Why not just change the ABI on all functions? > > If I understand your question "Why not just change the ABI on all functions", you mean use Microsoft ABI throughout the code even when compiled with gcc? The gcc option -mabi=ms makes this easy, and it reduces code size too (8% in one test). Part of that code size reduction is because it removes the requirement to save xmm6-xmm15 when calling msabi code. Gcc doesn't optimize the save/restore of xmm6-xmm15, it just does them all. The problems with ms abi I can think of are: > 1) Linux developers accustomed to stepping through the sysv calling convention would have to adapt to the ms calling convention. Well all the public interfaces (EFI service, protocol member functions, etc.) are EFIAPI, so there will be a mix. > 2) -mabi=ms is probably little used and therefore more likely to have bugs. This might require dropping support for older gcc tool chains. Looks like mixing has bugs too. It looks like this issue is caused by a mismatch in the vararg definitions between the two worlds. You can’t really mix styles in a given call stack. It almost seems like you want to force one var arg scheme every place possible. > 3) According to an email from you in April, ms abi might not support stack trace without debug symbols. > This is not the ABI it is VC++ code generation. There is nothing in the ABI about how to unwind a stack frame, it is about how to call code in C. In my clang examples, in this thread, we have an EFIAPI compatible calling convention with stack unwind. -target x86_64-pc-win32-macho means build an X64 image using EFIAPI, do the standard frame pointer, and we kick out a Mach-O executable for the debugger. We convert the Mach-O to PE/COFF for EFI compatibility. So on clang it as EFIABI, but with stack unwind. We can always unwind a frame without symbols, until we hit code compiled with VC++. > Even if ms abi is never made the default for gcc code, adding an environment variable such as EXTRA_CC_FLAGS would allow its use in custom builds by those who need the code size reduction it brings. > > What about switching EDK2 to sysv abi? I assume that would require dropping support for Microsoft compilers. The EFI calling convention is in the spec, so all things EFI would break. Option ROMs on cards, installed Operating system, etc…. The edk2 is an open source project that implements industry standard, not just a chunk of code. Thanks, Andrew Fish > > Thanks, > Scott ------------------------------------------------------------------------------
On Tue, Nov 4, 2014 at 9:28 AM, Andrew Fish <afish@apple.com> wrote: > So my 1st question is why do you need to mix calling conventions, and depend > on EFIAPI for interoperability. Why not just change the ABI on all > functions? GCC 4.4 doesn't support the command line option to change everything over. So, EFIAPI was the only option then. > Problems with the mixed calling convention: > 1) All assembly routines must be marked as EFIAPI, or the C code will > generate the wrong calling convention. Not an issue in the MdePkg, but > potentially an issue in other packages. I don't see this as a problem. I think this is the rules that we have set up for EDK II. It just so happens that the GCC4X toolchains are the only ones that use EFIAPI, and thus are the only ones that allow us to keep our codebase clean with regards to EFIAPI. For GCC >= 4.5, I actually think we should convert *RELEASE* builds over to using the ms-abi all the time to generate smaller code. I think we should leave DEBUG builds as mixed to help clean up EFIAPI issues. -Jordan ------------------------------------------------------------------------------
Another note (from the archives): vendors used mixed builds for VS2003/VS2005 on 32-bit in order to use __fastcall for internal function calls and then EFIABI for all the various UEFI calls. Tim -----Original Message----- From: Jordan Justen [mailto:jljusten@gmail.com] Sent: Tuesday, November 04, 2014 2:33 PM To: Andrew J. Fish Cc: Paolo Bonzini; edk2-devel@lists.sourceforge.net Subject: Re: [edk2] Enable optimization for gcc x64 builds On Tue, Nov 4, 2014 at 9:28 AM, Andrew Fish <afish@apple.com> wrote: > So my 1st question is why do you need to mix calling conventions, and > depend on EFIAPI for interoperability. Why not just change the ABI on > all functions? GCC 4.4 doesn't support the command line option to change everything over. So, EFIAPI was the only option then. > Problems with the mixed calling convention: > 1) All assembly routines must be marked as EFIAPI, or the C code will > generate the wrong calling convention. Not an issue in the MdePkg, but > potentially an issue in other packages. I don't see this as a problem. I think this is the rules that we have set up for EDK II. It just so happens that the GCC4X toolchains are the only ones that use EFIAPI, and thus are the only ones that allow us to keep our codebase clean with regards to EFIAPI. For GCC >= 4.5, I actually think we should convert *RELEASE* builds over to using the ms-abi all the time to generate smaller code. I think we should leave DEBUG builds as mixed to help clean up EFIAPI issues. -Jordan ------------------------------------------------------------------------------
> On Nov 4, 2014, at 2:32 PM, Jordan Justen <jljusten@gmail.com> wrote: > > On Tue, Nov 4, 2014 at 9:28 AM, Andrew Fish <afish@apple.com> wrote: >> So my 1st question is why do you need to mix calling conventions, and depend >> on EFIAPI for interoperability. Why not just change the ABI on all >> functions? > > GCC 4.4 doesn't support the command line option to change everything > over. So, EFIAPI was the only option then. > >> Problems with the mixed calling convention: >> 1) All assembly routines must be marked as EFIAPI, or the C code will >> generate the wrong calling convention. Not an issue in the MdePkg, but >> potentially an issue in other packages. > > I don't see this as a problem. I think this is the rules that we have > set up for EDK II. It just so happens that the GCC4X toolchains are > the only ones that use EFIAPI, and thus are the only ones that allow > us to keep our codebase clean with regards to EFIAPI. > I agree it is good in keeping the edk2 code clean. I was more concerned about code from 3rd parties. So sorry this was more about a warning when you are porting code on what to look out for. I’m really only concerned about how the VA_LIST stuff is going to work? Does it need to shift for native vs. EFIAPI? If so how you pass the VA_LIST around if the code is not all the same ABI? > For GCC >= 4.5, I actually think we should convert *RELEASE* builds > over to using the ms-abi all the time to generate smaller code. I > think we should leave DEBUG builds as mixed to help clean up EFIAPI > issues. > You guys should figure out if you can have a ms-abi but add the frame pointers. The compiler is open source …. Thanks, Andrew Fish > -Jordan ------------------------------------------------------------------------------
On 11/05/14 00:02, Andrew Fish wrote: > >> On Nov 4, 2014, at 2:32 PM, Jordan Justen <jljusten@gmail.com> wrote: >> >> On Tue, Nov 4, 2014 at 9:28 AM, Andrew Fish <afish@apple.com> wrote: >>> So my 1st question is why do you need to mix calling conventions, and depend >>> on EFIAPI for interoperability. Why not just change the ABI on all >>> functions? >> >> GCC 4.4 doesn't support the command line option to change everything >> over. So, EFIAPI was the only option then. >> >>> Problems with the mixed calling convention: >>> 1) All assembly routines must be marked as EFIAPI, or the C code will >>> generate the wrong calling convention. Not an issue in the MdePkg, but >>> potentially an issue in other packages. >> >> I don't see this as a problem. I think this is the rules that we have >> set up for EDK II. It just so happens that the GCC4X toolchains are >> the only ones that use EFIAPI, and thus are the only ones that allow >> us to keep our codebase clean with regards to EFIAPI. >> > > I agree it is good in keeping the edk2 code clean. I was more concerned about code from 3rd parties. > So sorry this was more about a warning when you are porting code on what to look out for. > > I’m really only concerned about how the VA_LIST stuff is going to > work? Does it need to shift for native vs. EFIAPI? If so how you pass > the VA_LIST around if the code is not all the same ABI? We need to distinguish passing arguments through the ellipsis (...) from passing VA_LIST (as a normal, named parameter). For passing arguments through the ellipsis (...), the called function *must* be EFIAPI (in the current state of the tree). Otherwise VA_START() won't work in the callee. (BTW I have no problem with the above "restriction".) Regarding passing VA_LIST by name (which is identical to passing a simple CHAR8* by name) -- it already works fine, regardless of EFIAPI vs. no-EFIAPI. The problem discussed in this thread is unrelated to EFIAPI. The problem is (apparently) that gcc's -Os optimization corrupts a local variable in a chain of recursive calls. >> For GCC >= 4.5, I actually think we should convert *RELEASE* builds >> over to using the ms-abi all the time to generate smaller code. I >> think we should leave DEBUG builds as mixed to help clean up EFIAPI >> issues. >> > > You guys should figure out if you can have a ms-abi but add the frame pointers. The compiler is open source …. In my experimentation yesterday, one of the first things I tried (on "gcc (GCC) 4.8.2 20140120 (Red Hat 4.8.2-16)") was '-fno-omit-frame-pointer'. Because, '-Os' implies '-fomit-frame-pointer', and at that point I thought that maybe '-fomit-frame-pointer', incurred by '-Os', was causing the issue. So, I added '-fno-omit-frame-pointer' *after* -Os. It triggered a "sorry, unimplemented" bug, which was very similar to <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59927>: sorry, unimplemented: ms_abi attribute requires -maccumulate-outgoing-args or subtarget optimization implying it However, after appending '-maccumulate-outgoing-args' as well, the build resumed. (To clarify, this meant: -Os -fno-omit-frame-pointer -maccumulate-outgoing-args .) Unfortunately, although the tree did build like this, the original issue persisted. Which I took as proof that the bug was unrelated to reserving or not reserving %rbp as frame pointer. I wish I could write a small reproducer for this problem... > > Thanks, > > Andrew Fish > >> -Jordan > > > ------------------------------------------------------------------------------ > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel > ------------------------------------------------------------------------------
On 11/05/14 11:00, Laszlo Ersek wrote:
> I wish I could write a small reproducer for this problem...
I wrote such a reproducer. I'll post it as a separate patch, just for
discussion. If we all agree that the code should work, then I could turn
it into a gcc bug report.
Thanks!
Laszlo
------------------------------------------------------------------------------
> On Nov 5, 2014, at 2:00 AM, Laszlo Ersek <lersek@redhat.com> wrote: > > On 11/05/14 00:02, Andrew Fish wrote: >> >>> On Nov 4, 2014, at 2:32 PM, Jordan Justen <jljusten@gmail.com> wrote: >>> >>> On Tue, Nov 4, 2014 at 9:28 AM, Andrew Fish <afish@apple.com> wrote: >>>> So my 1st question is why do you need to mix calling conventions, and depend >>>> on EFIAPI for interoperability. Why not just change the ABI on all >>>> functions? >>> >>> GCC 4.4 doesn't support the command line option to change everything >>> over. So, EFIAPI was the only option then. >>> >>>> Problems with the mixed calling convention: >>>> 1) All assembly routines must be marked as EFIAPI, or the C code will >>>> generate the wrong calling convention. Not an issue in the MdePkg, but >>>> potentially an issue in other packages. >>> >>> I don't see this as a problem. I think this is the rules that we have >>> set up for EDK II. It just so happens that the GCC4X toolchains are >>> the only ones that use EFIAPI, and thus are the only ones that allow >>> us to keep our codebase clean with regards to EFIAPI. >>> >> >> I agree it is good in keeping the edk2 code clean. I was more concerned about code from 3rd parties. >> So sorry this was more about a warning when you are porting code on what to look out for. >> >> I’m really only concerned about how the VA_LIST stuff is going to >> work? Does it need to shift for native vs. EFIAPI? If so how you pass >> the VA_LIST around if the code is not all the same ABI? > > We need to distinguish passing arguments through the ellipsis (...) from > passing VA_LIST (as a normal, named parameter). > > For passing arguments through the ellipsis (...), the called function > *must* be EFIAPI (in the current state of the tree). Otherwise > VA_START() won't work in the callee. > > (BTW I have no problem with the above "restriction".) > > Regarding passing VA_LIST by name (which is identical to passing a > simple CHAR8* by name) -- it already works fine, regardless of EFIAPI > vs. no-EFIAPI. > OK thanks for the info. For some flavors of GCC the __buildin_va_list is a structure. Since the size of the structure is > 8 bytes it is passed via a pointer per the calling conventions. For X64 EFIAPI VA_LIST is a pointer to the frame where the register based arguments have have been spilled. For clang x86_64 __buildin_va_list is also a structure, so you can’t mix and match. Thanks, Andrew Fish ~/work/Compiler>cat vv.c #include <stdarg.h> #include <stdio.h> int main () { printf ("sizeof __builtin_va_list %lu\n", sizeof (__builtin_va_list)); return 0; } ~/work/Compiler>clang vv.c ~/work/Compiler>./a.out sizeof __builtin_va_list 24 ~/work/Compiler>clang -arch i386 vv.c ~/work/Compiler>./a.out sizeof __builtin_va_list 4 ~/work/Compiler> > The problem discussed in this thread is unrelated to EFIAPI. The problem > is (apparently) that gcc's -Os optimization corrupts a local variable in > a chain of recursive calls. > >>> For GCC >= 4.5, I actually think we should convert *RELEASE* builds >>> over to using the ms-abi all the time to generate smaller code. I >>> think we should leave DEBUG builds as mixed to help clean up EFIAPI >>> issues. >>> >> >> You guys should figure out if you can have a ms-abi but add the frame pointers. The compiler is open source …. > > In my experimentation yesterday, one of the first things I tried (on > "gcc (GCC) 4.8.2 20140120 (Red Hat 4.8.2-16)") was > '-fno-omit-frame-pointer'. > > Because, '-Os' implies '-fomit-frame-pointer', and at that point I > thought that maybe '-fomit-frame-pointer', incurred by '-Os', was > causing the issue. > > So, I added '-fno-omit-frame-pointer' *after* -Os. > > It triggered a "sorry, unimplemented" bug, which was very similar to > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59927 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59927>>: > > sorry, unimplemented: ms_abi attribute requires > -maccumulate-outgoing-args or subtarget optimization implying it > > However, after appending '-maccumulate-outgoing-args' as well, the build > resumed. (To clarify, this meant: > > -Os -fno-omit-frame-pointer -maccumulate-outgoing-args > > .) Unfortunately, although the tree did build like this, the original > issue persisted. Which I took as proof that the bug was unrelated to > reserving or not reserving %rbp as frame pointer. > > I wish I could write a small reproducer for this problem... > >> ------------------------------------------------------------------------------
Andrew Fish [mailto:afish@apple.com] wrote: Sent: Tuesday, November 04, 2014 03:40 PM To: edk2-devel@lists.sourceforge.net Cc: Paolo Bonzini Subject: Re: [edk2] Enable optimization for gcc x64 builds On Nov 4, 2014, at 12:15 PM, Scott Duplichan <scott@notabs.org <mailto:scott@notabs.org> > wrote: But pedantically you need change the definition of BasePrintLibSPrint() to include EFIAPI. If you look at BasePrintLibSPrintMarker() (and some of the other routines) you will notice a BASE_LIST and a VA_LIST. We had an API that exposed a VA_LIST (well code that was casting to VA_LIST) in the report status code stack and it forced use to use our own made up BASE_LIST concept to get it to work. I think you are going to hit similar issues mixing calling conventions. So my 1st question is why do you need to mix calling conventions, and depend on EFIAPI for interoperability. Why not just change the ABI on all functions? If I understand your question "Why not just change the ABI on all functions", you mean use Microsoft ABI throughout the code even when compiled with gcc? The gcc option -mabi=ms makes this easy, and it reduces code size too (8% in one test). Part of that code size reduction is because it removes the requirement to save xmm6-xmm15 when calling msabi code. Gcc doesn't optimize the save/restore of xmm6-xmm15, it just does them all. The problems with ms abi I can think of are: 1) Linux developers accustomed to stepping through the sysv calling convention would have to adapt to the ms calling convention. Well all the public interfaces (EFI service, protocol member functions, etc.) are EFIAPI, so there will be a mix. 2) -mabi=ms is probably little used and therefore more likely to have bugs. This might require dropping support for older gcc tool chains. Looks like mixing has bugs too. It looks like this issue is caused by a mismatch in the vararg definitions between the two worlds. You can’t really mix styles in a given call stack. It almost seems like you want to force one var arg scheme every place possible. 3) According to an email from you in April, ms abi might not support stack trace without debug symbols. This is not the ABI it is VC++ code generation. There is nothing in the ABI about how to unwind a stack frame, it is about how to call code in C. In my clang examples, in this thread, we have an EFIAPI compatible calling convention with stack unwind. -target x86_64-pc-win32-macho means build an X64 image using EFIAPI, do the standard frame pointer, and we kick out a Mach-O executable for the debugger. We convert the Mach-O to PE/COFF for EFI compatibility. So on clang it as EFIABI, but with stack unwind. We can always unwind a frame without symbols, until we hit code compiled with VC++. Even if ms abi is never made the default for gcc code, adding an environment variable such as EXTRA_CC_FLAGS would allow its use in custom builds by those who need the code size reduction it brings. What about switching EDK2 to sysv abi? I assume that would require dropping support for Microsoft compilers. The EFI calling convention is in the spec, so all things EFI would break. Option ROMs on cards, installed Operating system, etc…. The edk2 is an open source project that implements industry standard, not just a chunk of code. Thanks, Andrew Fish These are all good answers. I can't come up with a strong argument for the mixed sysv/ms ABI. Maybe the next step is to test -mabi=ms using several gcc versions (I think -mabi=ms was introduced with gcc 4.5). If that works, I could submit a patch and see what happens.. Thanks, Scott Thanks, Scott ------------------------------------------------------------------------------
On 2014-11-07 08:16:23, Scott Duplichan wrote: > These are all good answers. I can't come up with a strong argument for the > mixed sysv/ms ABI. Maybe the next step is to test -mabi=ms using several gcc > versions (I think -mabi=ms was introduced with gcc 4.5). If that works, I could > submit a patch and see what happens.. I mentioned a reason in this thread a few days back. But, we should look into -mabi=ms for RELEASE builds. -Jordan ------------------------------------------------------------------------------
Jordan Justen [mailto:jordan.l.justen@intel.com] wrote: ]On 2014-11-07 08:16:23, Scott Duplichan wrote: ]> These are all good answers. I can't come up with a strong argument for the ]> mixed sysv/ms ABI. Maybe the next step is to test -mabi=ms using several gcc ]> versions (I think -mabi=ms was introduced with gcc 4.5). If that works, I could ]> submit a patch and see what happens.. ] ]I mentioned a reason in this thread a few days back. But, we should ]look into -mabi=ms for RELEASE builds. ] ]-Jordan I agree, the approach in your previous email is a good one. Prototyping asm functions to enforce calling convention is always a good idea. In theory an IA32 build could be done with a Microsoft compiler with option /Gr (__fastcall calling convention) and it would work. This would not be possible if asm function calling convention information were missing. If I make this patch, I will add the gcc -mabi=ms to the release build. Now for rants... 1) Why do so many developers never want to test release builds? To me, code is not clean until both debug and release builds work smoothly. 2) Why is the NOOPT build missing from virtually every DSC file in EDK2? The EDK NOOPT build is most like what developers call a DEBUG build. It is the only one setup for source level debugging, at least for Microsoft tool chains. The Duet DSC files are missing both RELEASE and NOOPT options. I may submit a patch to allow all 3 builds to every DSC file. Thanks, Scott ------------------------------------------------------------------------------
> > Now for rants... > 1) Why do so many developers never want to test release builds? To me, code > is not clean until both debug and release builds work smoothly. > 2) Why is the NOOPT build missing from virtually every DSC file in EDK2? > The EDK NOOPT build is most like what developers call a DEBUG build. It is > the only one setup for source level debugging, at least for Microsoft tool > chains. The Duet DSC files are missing both RELEASE and NOOPT options. I > may submit a patch to allow all 3 builds to every DSC file. > Scott, Historically, if I remember correctly, the NOOPT builds were added to support Nt32Pkg/EmulatorPkg. “Back in the day” a NOOPT build would not generally fit in a ROM. But given folks could be building an option ROM, shell, OS loader that don’t have size constraints I think you are right in pointing out the NOOPT builds should be in all the supported tool chains. Thanks, Andrew Fish > Thanks, > Scott ------------------------------------------------------------------------------
On 11/07/14 21:29, Scott Duplichan wrote: > Jordan Justen [mailto:jordan.l.justen@intel.com] wrote: > > ]On 2014-11-07 08:16:23, Scott Duplichan wrote: > ]> These are all good answers. I can't come up with a strong argument for the > ]> mixed sysv/ms ABI. Maybe the next step is to test -mabi=ms using several gcc > ]> versions (I think -mabi=ms was introduced with gcc 4.5). If that works, I could > ]> submit a patch and see what happens.. > ] > ]I mentioned a reason in this thread a few days back. But, we should > ]look into -mabi=ms for RELEASE builds. > ] > ]-Jordan > > I agree, the approach in your previous email is a good one. Prototyping > asm functions to enforce calling convention is always a good idea. In theory > an IA32 build could be done with a Microsoft compiler with option /Gr > (__fastcall calling convention) and it would work. This would not be possible > if asm function calling convention information were missing. If I make this > patch, I will add the gcc -mabi=ms to the release build. > > Now for rants... > 1) Why do so many developers never want to test release builds? To me, code > is not clean until both debug and release builds work smoothly. In my experience, release (== optimized) builds are practically unsupportable. Even the Linux kernel disables some optimizations that make the disassembly unreadable. Unless stuff is power and/or performance critical, I prefer if the code does exactly what I tell it to do. (Case in point: the -Os bug with recursion + ellipsis. It works with -O0. Compilers have bugs.) *All* software is chock full of bugs, and having to figure out what goes wrong at a customer's site is a question of "when", not "if". They either won't be able, or willing, to attempt to reproduce the issue with a debug build, or they will try and the bug might disappear. Consequently, since I'm not keen on shipping anything but a debug build, I don't feel like putting many resources into release builds. > 2) Why is the NOOPT build missing from virtually every DSC file in EDK2? I guess in OVMF we never needed it? > The EDK NOOPT build is most like what developers call a DEBUG build. It is > the only one setup for source level debugging, at least for Microsoft tool > chains. I don't use Microsoft tool chains. And source level debugging with gdb is hardly possible even on DEBUG; you have to jump through incredible hoops. NOOPT doesn't solve anything for Linux-based developers & users of OVMF. > The Duet DSC files are missing both RELEASE and NOOPT options. It's an emulator platform, isn't it? You probably won't use it in production. Thanks Laszlo ------------------------------------------------------------------------------ Comprehensive Server Monitoring with Site24x7. Monitor 10 servers for $9/Month. Get alerted through email, SMS, voice calls or mobile push notifications. Take corrective actions from your mobile device. http://pubads.g.doubleclick.net/gampad/clk?id=154624111&iu=/4140/ostg.clktrk
Laszlo Ersek [mailto:lersek@redhat.com] wrote: ]On 11/07/14 21:29, Scott Duplichan wrote: ]> Jordan Justen [mailto:jordan.l.justen@intel.com] wrote: ]> ]> ]On 2014-11-07 08:16:23, Scott Duplichan wrote: ]> ]> These are all good answers. I can't come up with a strong argument for the ]> ]> mixed sysv/ms ABI. Maybe the next step is to test -mabi=ms using several gcc ]> ]> versions (I think -mabi=ms was introduced with gcc 4.5). If that works, I could ]> ]> submit a patch and see what happens.. ]> ] ]> ]I mentioned a reason in this thread a few days back. But, we should ]> ]look into -mabi=ms for RELEASE builds. ]> ] ]> ]-Jordan ]> ]> I agree, the approach in your previous email is a good one. Prototyping ]> asm functions to enforce calling convention is always a good idea. In theory ]> an IA32 build could be done with a Microsoft compiler with option /Gr ]> (__fastcall calling convention) and it would work. This would not be possible ]> if asm function calling convention information were missing. If I make this ]> patch, I will add the gcc -mabi=ms to the release build. ]> ]> Now for rants... ]> 1) Why do so many developers never want to test release builds? To me, code ]> is not clean until both debug and release builds work smoothly. ] ]In my experience, release (== optimized) builds are practically ]unsupportable. Even the Linux kernel disables some optimizations that ]make the disassembly unreadable. Unless stuff is power and/or ]performance critical, I prefer if the code does exactly what I tell it ]to do. (Case in point: the -Os bug with recursion + ellipsis. It works ]with -O0. Compilers have bugs.) ] ]*All* software is chock full of bugs, and having to figure out what goes ]wrong at a customer's site is a question of "when", not "if". They ]either won't be able, or willing, to attempt to reproduce the issue with ]a debug build, or they will try and the bug might disappear. ] ]Consequently, since I'm not keen on shipping anything but a debug build, ]I don't feel like putting many resources into release builds. Release builds are/were shipped out of necessity, at least in the past. This was due to a desire to cut board cost by using the smallest possible flash chip. But times are changing and NOR flash capacity is growing even faster than code size. So the flash size reduction motivation for optimizing code is losing importance I guess. In my experience, getting a release build to work sometimes results in uncovering hidden coding errors. A bigger reason to use release builds is boot time reduction. While UEFI will never boot as fast as coreboot, it can narrow the gap some by minimizing the time spent reading data from the flash chip. Adding -Os and link time optimization can cut the image size in half. That saves significant time when the image is read from the flash chip. ] ]> 2) Why is the NOOPT build missing from virtually every DSC file in EDK2? ] ]I guess in OVMF we never needed it? I got OVMF booting on a real server a few years ago. Adding the NOOPT build was the first thing I did. That let me step through the source code and see all local variables. I couldn't have gotten it working as quickly as I did without source level debugging. Instead of adding NOOPT to every project, adding it to one or two might be a better idea. I don't want to see a lot of code change just for the sake debugger support. ]> The EDK NOOPT build is most like what developers call a DEBUG build. It is ]> the only one setup for source level debugging, at least for Microsoft tool ]> chains. ] ]I don't use Microsoft tool chains. And source level debugging with gdb ]is hardly possible even on DEBUG; you have to jump through incredible ]hoops. NOOPT doesn't solve anything for Linux-based developers & users ]of OVMF. I understand. But Microsoft tool chains are still supported by the EDK2 project. Dropping support for Microsoft tool chains would solve some problems. But clearly that isn't going to happen any time soon. It is surprising to see the strength and weaknesses of different tool chains. Microsoft perfected link time optimization 10+ years ago. Yet they didn't even bother with C99 support until recently. GCC had C99 10+ years ago, yet only recently perfected link time optimization. It is unfortunate there is no nice open source debugger for use with EDK2 and other embedded projects. Some of the OEM and IBV debuggers I used were really nice, though at the time they supported only Microsoft debug symbols. ]> The Duet DSC files are missing both RELEASE and NOOPT options. ] ]It's an emulator platform, isn't it? You probably won't use it in ]production. Of all EDK2 projects, Duet seems to need the fewest changes for use on real hardware. You would be surprised to know how many Duet based systems have shipped from tier one oems. ]Thanks ]Laszlo ------------------------------------------------------------------------------ Comprehensive Server Monitoring with Site24x7. Monitor 10 servers for $9/Month. Get alerted through email, SMS, voice calls or mobile push notifications. Take corrective actions from your mobile device. http://pubads.g.doubleclick.net/gampad/clk?id=154624111&iu=/4140/ostg.clktrk
diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c b/MdePkg/Library/BasePrintLib/PrintLibInternal.c index 8dc5ec7..fbb3726 100644 --- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c +++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c @@ -680,10 +680,20 @@ BasePrintLibSPrintMarker ( if (TmpGuid == NULL) { ArgumentString = "<null guid>"; } else { + UINTN (EFIAPI * volatile PrintFunction) ( + OUT CHAR8 *StartOfBuffer, + IN UINTN BufferSize, + IN UINTN Flags, + IN CONST CHAR8 *FormatString, + ... + ); + + PrintFunction = BasePrintLibSPrint; + GuidData1 = ReadUnaligned32 (&(TmpGuid->Data1)); GuidData2 = ReadUnaligned16 (&(TmpGuid->Data2)); GuidData3 = ReadUnaligned16 (&(TmpGuid->Data3)); - BasePrintLibSPrint ( + PrintFunction ( ValueBuffer, MAXIMUM_VALUE_CHARACTERS, 0,