Clover Problems and Solutions

[mhaeuser]

I see... Well, aside from that and one removed assignation, the patches are either obsolete, absolute rubbish or they add files that shouldn't be

[Slice]

Hi Zenith432,

Why this happen after I applyed Patches_for_UDK2018 (still lived without them)

Quote

/Users/sergey/src/UDK2018/MdePkg/Library/BaseLib/SwitchStack.c:66:3: error: 'va_start' used in Win64 ABI function
  VA_START (Marker, NewStack);
  ^
/Users/sergey/src/UDK2018/MdePkg/Include/Base.h:740:38: note: expanded from macro 'VA_START'
#define VA_START(Marker, Parameter)  __builtin_va_start (Marker, Parameter)
                                     ^
1 error generated.

 

 

[Zenith432]

Revert MdePkg/Include/Base.h to its repository copy of UDK2018.  It is no longer in the patches.  The macro NO_MSABI_VA_FUNCS is used to inhibit __builtin_ms_va* in XCODE5 toolchain.  XCODE8 toolchain no longer needs -D for this.  It used to be the reverse - that's why you're getting the error becausing using old patched Base.h with new tools_def.txt.

[Slice]

Thanks for the explanation.

I successfully used Base.h and tools.def from previous EDK2. Why this is wrong?

[Zenith432]

53 minutes ago, Slice said:

Thanks for the explanation.

I successfully used Base.h and tools.def from previous EDK2. Why this is wrong?

Previously, MdePkg/Include/Base.h had old patch so that...

GCC didn't need any of it except GCC before v4.8.

-t XCODE5 needed no -D symbol and there was logic in patch to detect whether old versions of clang need __builtin_ms_va* or not.

-t XCODE8 needed -D USE_CLANG_MS_VA_LIST to force use of __builtin_ms_va* because uses -target x86_64_apple_darwin which always need it regardless of version of clang.

 

All this was outdated.

In repository Base.h, __builtin_ms_va* is enabled unless macro NO_MSABI_VA_LIST is defined.

So I reversed logic in order so patch for Base.h not needed anymore.

 

In Clover r4496 I removed Base.h patch.  In tools_def.txt there was no -D for XCODE5, so I added -D NO_MSABI_VA_LIST to tools_def.txt.  So starting r4496 XCODE5 still worked with old patched Base.h, and with reverted Base.h.

In r4496 I left the -D USE_CLANG_MS_VA_LIST for XCODE8 in order to give time to adjust.  (Actually because I forget to remove it :))

So starting r4496 XCODE8 continued to work with both old patched Base.h and with reverted Base.h.

 

Then in Clover r4498 I removed -D USE_CLANG_MS_VA_LIST from XCODE8 so it stopped working with old patched Base.h.

 

So if you got caught with

- old patched Base.h

- copy of tools_def.txt from r4496 or r4497

Then XODE8 still worked.

After change to tools_def.txt in r4498, XCODE8 stopped working old patched Base.h

 

Edited June 7, 2018 by Zenith432

[Slice]

Is it true for Xcode 9 or also for Xcode 7.3.1 which I used in ElCapitan? XCODE8 toolset fine here.

Quote

Dell:Clover sergey$ /usr/bin/clang -v
Apple LLVM version 7.3.0 (clang-703.0.31)
Target: x86_64-apple-darwin15.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
Dell:Clover sergey$

 

 

[Zenith432]

XCODE8 needs __builtin_ms_va*.  If Xcode 7.3.1 has it - should work.  Old versions of clang don't have it.  I added XCODE8 when XCODE 8.something was out, I was using it and didn't bother to check older versions so I named it XCODE8.

[Slice]

I see this comment

Quote

#if CLANG_VERSION >= 0x0703 && CLANG_VERSION < 0x0801
// Use __builtin_ms_va_list if Apple clang ver >= 7.3.0 and < 8.1.0
#define USE_CLANG_BUILTIN_MS_VA_LIST
#endif

 

 

[Zenith432]

That comment was for selectively enabling for XCODE5 depending on version of clang.  For XCODE8 it must always be on.

The code you quoted is surrounded by "#if !defined(__MACH__)".  For XCODE8, -target x86_64-apple-darwin - __MACH__ is defined so this doesn't get included.

 

There are 3 combinations

1) -target x86_64-windows-macho + __builtin_va*.  This is the oldest, the one that's always been used by Apple (for example boot.efi) and is still used by Apple.  All functions are ms_abi.  __builtlin_va_list is ISO-C compliant, but not compatible with Microsoft.  It is the one used by XCODE5 today.

 

2) -target x86_64-apple-darwin + __builtin_ms_va*.  This works for versions of clang that support __builtin_ms_va*.  It is used by XCODE8.  EFIAPI are ms_abi.  Other funcs are sysv_abi.  __builtin_ms_va_list is Microsoft compatible.

 

3) -target x86_64-windows-macho + __builtin_ms_va*.  This was never officially supported by clang, but worked on a limited range of versions which is what the patch was for.  It's like 1), but __builtin_ms_va_list is Microsoft compatible.  Nowadays it generates compile-time errors and can't be used.  It was used with XCODE5 for the versions of clang it worked in.

 

Some occasional old releases of Xcode had bugs in some of these that lasted for one release and then fixed.  Today options 1) works (XCODE5) and 2) works (XCODE8).

 

Edited June 8, 2018 by Zenith432

[Slice]

I want to propose that 2) will work with clang 7.3.1 so I can use XCODE8 toolset.

[Zenith432]

If your proposal is to change XCODE8 to XCODE7 go ahead.  Nobody is stopping you.  I never tested 2) on versions prior to 8 which is why I used 8 in the name.

 

Setup 2) also needs (and uses) -mno-red-zone (like GCC mixed ABI).  Because timer interrupts overrun sysv redzone.

 

If wanting to write varargs sysv (=non-EFIAPI) functions in setup 2) they can use __builtin_va_list, which is sysv_va_list.

 

For Setup 1), all functions are ms_abi so all varargs functions can use the same clang-specific __builtin_va_list.

Edited June 8, 2018 by Zenith432

[Slice]

Is it essencial to have Microsoft compatibility? Booting Windows 10 from legacy Clover, for example?

[Zenith432]

I don't think so, because public UEFI APIs don't pass VA_LIST arguments.  Some of them have varargs, but it's passing incompatible VA_LIST that would crash it.  (such as between  bootmgfw.efi and CloverEFI).

 

This began with bug that was in DevicePathToText where function taking varargs did this

VA_START(ap, XX)

call subfunction to go over ap.

call another function to go over ap a 2nd time.

VA_END(ap)

 

This is invalid according to ISO C.  With Microsoft va_list this doesn't cause any bugs so nobody noticed it.  The VA_LIST is a pointer to the first variable argument and can be used many times over and over.

In clang different implementation VA_LIST is something more complicated and a pointer to it is passed to the subfunction, so it can't be used a 2nd time without doing VA_END/VA_START again.

 

If this seems simple - it was the only reason XCODE5 was not working at the time, and you were using GCC.  It caused a memory corruption deep down the call chain that required a lot of work to find in the absence of a kernel debugger for Clover.

 

After this bug was patched (later resolved in EDK2), XCODE5 started working for Clover.  Because this clang va_list was a potential for bugs, I looked for way to get the normal Microsoft va_list, and __builtin_ms_va_list accomplished this from some version of clang - which is why that patch in Base.h started.  Before XCODE5 there was a patch in Base.h, but just for GCC, since GCC < 4.8 didn't have __builtin_ms_va_list.

 

The XCODE8 I did not start because of a bug - but because it was a challenge to see if mixed-ABI can be made to work in clang like they use GCC mixed-ABI in EDK2 (in Clover we use -mabi=ms for GCC to avoid problems.)

So XCODE8 helped reveal all the errors in usage of EFIAPI in function declarations or definitions in Clover - I fixed all those.  I also found there were va_list issues in the code imported from Unix (grub and openssl).  Because sysv functions were trying to using ms_va_list or vice versa.  I fixed those as well.  After fixing the linkage problems XCODE8 worked - so I published it.  Then later it turned out there is that redzone problem with timer interrupts (-mno-red-zone is in GCC, but I overlooked it.)  After the -mno-red-zone XCODE8 became stable so you decided to use it.  Today XCODE5 and XCODE8 both work with no outstanding known bugs and it's a matter of personal choice which you prefer.

 

 

[Pavo]

Just wanted to give you guys a heads up but Clover v4522 can not boot Mojave. 

 

Well guess it was just a glitch, now its booting. Even with only apfs.efi, AptioMemoryFix.efi and HFS+.efi

Edited June 8, 2018 by Pavo

[Slice]

Yes, XCODE5 and XCODE8 both working and XCODE5 produces smalles codes. (why?)

I just have a problem with compilation UDK2018+Patches_for_UDK2018+XCODE8. Reverting Base.h causes more errors. I will look later what is happen. Base.h+tools_def from EDK2 helps.

[Zenith432]

@Slice:

If you want to add the patch back for Xcode 7.3.1 so you can use XCODE5 + __builtin_ms_va*, it can be done in simpler way than was before...

#if <check version of clang>
#undef NO_MSABI_VA_FUNCS
#endif

Check doesn't need to make distinction between XCODE5 or XCODE8.  Just check version clang.  For XCODE8 this macro is not defined.  For XCODE5 it is defined, but with right versions of clang can undefine it to enable the __builtin_ms_va*.  No need to modify tools_def.txt for this.

 

BTW, tools_def and build_rules are identical in Patches_for_EDK2 and Patches_for_UDK2018.

Edited June 9, 2018 by Zenith432

[Slice]

There is working combination

 

OldUDKPatches.zip

[Zenith432]

@Slice:

The Base.h you uploaded has some definitions  that are not in UDK2018.  They're from EDK2, but unless you used them yourself they should not break Clover build.

Spoiler

///
/// Tell the code optimizer that the function will return twice.
/// This prevents wrong optimizations which can cause bugs.
///
#ifndef RETURNS_TWICE
  #if defined (__GNUC__) || defined (__clang__)
    ///
    /// Tell the code optimizer that the function will return twice.
    /// This prevents wrong optimizations which can cause bugs.
    ///
    #define RETURNS_TWICE  __attribute__((returns_twice))
  #else
    ///
    /// Tell the code optimizer that the function will return twice.
    /// This prevents wrong optimizations which can cause bugs.
    ///
    #define RETURNS_TWICE
  #endif
#endif
==========

///
/// Minimum values for the signed UEFI Data Types
///
#define MIN_INT8   (((INT8)  -127) - 1)
#define MIN_INT16  (((INT16) -32767) - 1)
#define MIN_INT32  (((INT32) -2147483647) - 1)
#define MIN_INT64  (((INT64) -9223372036854775807LL) - 1)

==========
#elif defined(_M_ARM) || defined(_M_ARM64)
//
// MSFT ARM variable argument list support.
//

typedef char* VA_LIST;

#define VA_START(Marker, Parameter)     __va_start (&Marker, &Parameter, _INT_SIZE_OF (Parameter), __alignof(Parameter), &Parameter)
#define VA_ARG(Marker, TYPE)            (*(TYPE *) ((Marker += _INT_SIZE_OF (TYPE) + ((-(INTN)Marker) & (sizeof(TYPE) - 1))) - _INT_SIZE_OF (TYPE)))
#define VA_END(Marker)                  (Marker = (VA_LIST) 0)
#define VA_COPY(Dest, Start)            ((void)((Dest) = (Start)))

===========

 

 

The tools_def.txt you uploaded is badly outdated on many toolchains.  For XCODE8, it leaves out the -mno-red-zone.  This should not cause build error - but can cause runtime error.

 

So I don't know - what are the build errors?

[Slice]

I have to try again, it may be my mistakes like not clean build folder.

[Slice]

/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:1602:7: error: unknown type name 'IPv6_ADDRESS'
  OUT IPv6_ADDRESS       *Address,
      ^

/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:4906:1: error: unknown type name 'RETURNS_TWICE'
RETURNS_TWICE
^

/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:4908:1: error: 'ms_abi' only applies to function types; type here is 'int' [-Werror,-Wignored-attributes]
EFIAPI
^
<command line>:1:31: note: expanded from here
#define EFIAPI __attribute__((ms_abi))
                              ^
/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:4908:7: error: expected ';' after top level declarator
EFIAPI
      ^
      ;
7 errors generated.

Let continue.

I took Base.h from here

And tools_def.txt from Patched_to_UDK2018.

Deleted Build folder

./ebuild.sh -fr

I got these errors

 

XCODE5 has more mistakes about unknown types, but no message about ms_abi.

[Slice]

@Zenith432

I committed working combination fixed on red_zone. Check me, please, and correct if needed.

You also have to correct again GCC* toolsets, i don't use it and don't check.

I checked XCODE5 and XCODE8 on Xcode7.3.1. Looks good.

XCODE5: 778 208 bytes

XCODE8: 789 600 bytes

[Slice]

And next test Xcode9.4 under HighSierra

XCODE8: 777 440 bytes

[mhaeuser]

/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:1602:7: error: unknown type name 'IPv6_ADDRESS' OUT IPv6_ADDRESS       *Address,     ^/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:4906:1: error: unknown type name 'RETURNS_TWICE'RETURNS_TWICE^/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:4908:1: error: 'ms_abi' only applies to function types; type here is 'int' [-Werror,-Wignored-attributes]EFIAPI^ line>:1:31: note: expanded from here#define EFIAPI __attribute__((ms_abi))                             ^/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:4908:7: error: expected ';' after top level declaratorEFIAPI     ^     ;7 errors generated.

Let continue.
I took Base.h from here

And tools_def.txt from Patched_to_UDK2018.
Deleted Build folder
./ebuild.sh -fr
I got these errors
 
XCODE5 has more mistakes about unknown types, but no message about ms_abi.

Sorry if I overlooked something, but did you intend to show the UDK2008 (not 2018) branch?

[Zenith432]

11 hours ago, Slice said:

/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:1602:7: error: unknown type name 'IPv6_ADDRESS'
  OUT IPv6_ADDRESS       *Address,
      ^

/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:4906:1: error: unknown type name 'RETURNS_TWICE'
RETURNS_TWICE
^

/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:4908:1: error: 'ms_abi' only applies to function types; type here is 'int' [-Werror,-Wignored-attributes]
EFIAPI
^
<command line>:1:31: note: expanded from here
#define EFIAPI __attribute__((ms_abi))
                              ^
/Users/sergey/src/UDK2018/MdePkg/Include/Library/BaseLib.h:4908:7: error: expected ';' after top level declarator
EFIAPI
      ^
      ;
7 errors generated.

 

Your UDK2018 is contaminated with files from EDK2 that shouldn't be there.  These errors are from using MdePkg/Include/Library/BaseLib.h from EDK2 instead of the one from UDK2018.  This file is not in the patches.  It's not in UDK2018.  Yet it's in your UDK2018 tree.

You need to have modified files in UDK2018 only from the patches.  Nothing more.

There was nothing wrong with Patches_for_UDK2018 prior to r4524.  You were getting errors because of contaminated tree.

Now you committed older Base.h because it helps with your contaminated BaseLib.h.

Along with this you committed older tools_def.txt to help older Base.h.

The older tools_def.txt you committed doesn't have -mno-red-zone in RELEASE_XCODE8_X64_CC_FLAGS.  It's obvious in the diff it's deleted at the end of that line.

So now XCODE8 with UDK2018 patches generates binaries that can have stack corruption due to timer interrupts.

 

I don't know what you expect me to do about this...

Do you want me to fix the Base.h and tools_def.txt you committed in r4524 to make them work with your contaminated BaseLib.h?  (A file not in the patches).

Are you willing to clean your tree from unpatched files?  Because if you are, the proper solution is to revert r4524 completely.  It's wrong.

 

Do you want the patch that allows __builtin_ms_va_list to work with XCODE5 in Xcode 7.3.1 back?

 

[Slice]

OK, I will revert the whole tree and try again.

No, let Xcode7 works as XCODE8 and left XCODE5 for older versions.