Clover Problems and Solutions

mhaeuser · April 7, 2016

3 patches sent, 3 patches merged. I admit they were rather small, but such changes are not big either...

TheRacerMaster · April 9, 2016

Strangely enough I seem to have more issues building Clover with GCC 5.3 than I do with Xcode. While it does build, it hangs at Testing hardware when booting. Last lines from the log (over serial debug from a VM) are:

3167640576:3814967  538982198:64013152105016  ScanSPD() start
3221067504:3157931056  4116325476609306931:3977862860598817073  ScanSPD() end
3167591681:3208261944  9042607841292598:3762818168875267120  Get Acpi Tables List from RSDT:

Anyone else having this issue?

zxv · April 9, 2016

Strangely enough I seem to have more issues building Clover with GCC 5.3 than I do with Xcode. While it does build, it hangs at Testing hardware when booting. Last lines from the log (over serial debug from a VM) are:
3167640576:3814967  538982198:64013152105016  ScanSPD() start
3221067504:3157931056  4116325476609306931:3977862860598817073  ScanSPD() end
3167591681:3208261944  9042607841292598:3762818168875267120  Get Acpi Tables List from RSDT:
Anyone else having this issue?

Only time I ran into issues w/ gcc53 builds was when I forgot to add the patches. Otherwise it's been working fine w/o apparent issue for the last week or so.

TheRacerMaster · April 9, 2016

Only time I ran into issues w/ gcc53 builds was when I forgot to add the patches. Otherwise it's been working fine w/o apparent issue for the last week or so.

Strange... I'm already using the Patches_for_EDK2.

Zenith432 · April 10, 2016

So, about the code bloat - I found that linking without the ld script gcc4.9-ld-script solves it. The code for GCC is compiled with "-ffunction-sections -fdata-sections" which makes the compiler put each function and data piece in a different section. The ld script has some alignment commands that end up inserting quite a bit of padding.

So in #3427, I removed the previous "handling" of using -Os optimization globally, and instead discontinued the use of ld script for GCC 5.3. In addition, I synced the cc and ld flags with BaseTools/Conf from edk2. Now gcc 5.3 builds boot6, boot7 and also -ia32 without overflow.

TheRacerMaster: do you have latest patches from #3428? Also, make sure to delete edk2/Build between switching -gcc53 and -xcode5 so you don't get object files from one toolchain built into the other.

tluck · April 10, 2016

was able to build 3428 with gcc5.3. i use CloverGrowerPro scripts and had to increase ramdisk to 1024000. but final results seem to be reasonable sizes etc.

Zenith432 · April 11, 2016

I found one problem with -xcode5 - is that varargs were being handled by the wrong section in patched Base.h - and this was causing mayhem. Fixed in #3429. Now the result for -xcode5 build is that CloverGUI works when run with boot6/7 made by gcc, but boot6/7 made by xcode5 crash with a CPU exception of "undefined opcode". I don't know how to locate the place in the code where it crashes from the register dump. I looked at some of the map files in Build, but couldn't figure it out.

Same result.

Slice · April 11, 2016

I think we should compare binaries produced by gcc and by clang, those files that was written in ASM instead of C:

edk2/MdePkg/Library/BaseLib/X64/Thunk16.asm/.S/.nasm

Previously I was fighting with it and saw gcc is good, clang is not.

Other asm?

Zenith432 · April 11, 2016

Here's how to build Clover on Fedora 23.

dnf install the following packages
- gcc-c++
- git and git-svn (or svn)
- nasm
- binutils-x86_64-linux.gnu-2.26
create an entry that allows dlopen() to find liblto_plugin.so

sudo echo '/usr/libexec/gcc/x86_64-redhat-linux/5.3.1' > /etc/ld.so.conf.d/gcc.conf
sudo ldconfig

patch edk2/Clover/Patches_for_EDK2/Conf/tools_def.txt as follows (and then copy to edk2/Conf)

diff --git a/Patches_for_EDK2/Conf/tools_def.txt b/Patches_for_EDK2/Conf/tools_def.txt
--- a/Patches_for_EDK2/Conf/tools_def.txt
+++ b/Patches_for_EDK2/Conf/tools_def.txt
@@ -190,8 +190,8 @@ DEFINE GCC48_X64_PREFIX        = ENV(TOOLCHAIN_DIR)/cross/bin/x86_64-clover-linu
 DEFINE GCC49_IA32_PREFIX       =  ENV(TOOLCHAIN_DIR)/cross/bin/x86_64-clover-linux-gnu-
 DEFINE GCC49_X64_PREFIX        =  ENV(TOOLCHAIN_DIR)/cross/bin/x86_64-clover-linux-gnu-
 
-DEFINE GCC53_IA32_PREFIX       = ENV(TOOLCHAIN_DIR)/cross/bin/x86_64-clover-linux-gnu-
-DEFINE GCC53_X64_PREFIX        = ENV(TOOLCHAIN_DIR)/cross/bin/x86_64-clover-linux-gnu-
+DEFINE GCC53_IA32_PREFIX       = /usr/bin/
+DEFINE GCC53_X64_PREFIX        = /usr/bin/
 
 DEFINE UNIX_IASL_BIN           = /usr/local/bin/iasl
 #DEFINE UNIX_IASL_BIN           = $(HOME)/programs/iasl
@@ -4720,7 +4720,8 @@ RELEASE_GCC48_AARCH64_CC_FLAGS   = DEF(GCC48_AARCH64_CC_FLAGS) -Wno-unused-but-s
 ##################
 *_GCC53_IA32_OBJCOPY_PATH         = DEF(GCC53_IA32_PREFIX)objcopy
 *_GCC53_IA32_CC_PATH              = DEF(GCC53_IA32_PREFIX)gcc
-*_GCC53_IA32_SLINK_PATH           = DEF(GCC53_IA32_PREFIX)gcc-ar
+#*_GCC53_IA32_SLINK_PATH           = DEF(GCC53_IA32_PREFIX)gcc-ar
+*_GCC53_IA32_SLINK_PATH           = /usr/x86_64-linux-gnu/bin/ar
 *_GCC53_IA32_DLINK_PATH           = DEF(GCC53_IA32_PREFIX)gcc
 *_GCC53_IA32_ASLDLINK_PATH        = DEF(GCC53_IA32_PREFIX)gcc
 *_GCC53_IA32_ASM_PATH             = DEF(GCC53_IA32_PREFIX)gcc
@@ -4734,6 +4735,7 @@ RELEASE_GCC48_AARCH64_CC_FLAGS   = DEF(GCC48_AARCH64_CC_FLAGS) -Wno-unused-but-s
 *_GCC53_IA32_ASLDLINK_FLAGS       = DEF(GCC53_IA32_X64_ASLDLINK_FLAGS),-m,elf_i386
 *_GCC53_IA32_ASM_FLAGS            = DEF(GCC53_ASM_FLAGS) -m32 -march=i386
 *_GCC53_IA32_CC_FLAGS             = DEF(GCC53_IA32_CC_FLAGS) ENV(GCC53_IA32_EXTRA_CC_FLAGS) -c
+*_GCC53_IA32_SLINK_FLAGS          = --plugin liblto_plugin.so
 *_GCC53_IA32_DLINK_FLAGS          = DEF(GCC53_IA32_X64_DLINK_FLAGS),-m,elf_i386,--oformat=elf32-i386 DEF(GCC53_IA32_CC_FLAGS)
 *_GCC53_IA32_RC_FLAGS             = DEF(GCC_IA32_RC_FLAGS)
 *_GCC53_IA32_OBJCOPY_FLAGS        =
@@ -4744,7 +4746,8 @@ RELEASE_GCC48_AARCH64_CC_FLAGS   = DEF(GCC48_AARCH64_CC_FLAGS) -Wno-unused-but-s
 ##################
 *_GCC53_X64_OBJCOPY_PATH         = DEF(GCC53_X64_PREFIX)objcopy
 *_GCC53_X64_CC_PATH              = DEF(GCC53_X64_PREFIX)gcc
-*_GCC53_X64_SLINK_PATH           = DEF(GCC53_X64_PREFIX)gcc-ar
+#*_GCC53_X64_SLINK_PATH           = DEF(GCC53_X64_PREFIX)gcc-ar
+*_GCC53_X64_SLINK_PATH           = /usr/x86_64-linux-gnu/bin/ar
 *_GCC53_X64_DLINK_PATH           = DEF(GCC53_X64_PREFIX)gcc
 *_GCC53_X64_ASLDLINK_PATH        = DEF(GCC53_X64_PREFIX)gcc
 *_GCC53_X64_ASM_PATH             = DEF(GCC53_X64_PREFIX)gcc
@@ -4758,6 +4761,7 @@ RELEASE_GCC48_AARCH64_CC_FLAGS   = DEF(GCC48_AARCH64_CC_FLAGS) -Wno-unused-but-s
 *_GCC53_X64_ASLDLINK_FLAGS       = DEF(GCC53_IA32_X64_ASLDLINK_FLAGS),-m,elf_x86_64
 *_GCC53_X64_ASM_FLAGS            = DEF(GCC53_ASM_FLAGS) -m64
 *_GCC53_X64_CC_FLAGS             = DEF(GCC53_X64_CC_FLAGS) ENV(GCC53_X64_EXTRA_CC_FLAGS) -c
+*_GCC53_X64_SLINK_FLAGS          = --plugin liblto_plugin.so
 *_GCC53_X64_DLINK_FLAGS          = DEF(GCC53_X64_DLINK_FLAGS) DEF(GCC53_X64_CC_FLAGS) ENV(GCC53_X64_EXTRA_CC_FLAGS)
 *_GCC53_X64_RC_FLAGS             = DEF(GCC_X64_RC_FLAGS)
 *_GCC53_X64_OBJCOPY_FLAGS        =

finally, './ebuild.sh -gcc53', or './ebuild.sh -gcc53 -mc -DDISABLE_USB_SUPPORT' or './ebuild.sh -gcc53 -ia32'.

I couldn't get ar from binutils 2.25 to handle object files generated by gcc 5.3.1 with link-time-optimization. That's why the extra steps of installing ar from binutils 2.26 and directing to it.

mhaeuser · April 11, 2016

Why are the _PREFIX-variables forced anyway? They were solely intended for Windows and cross-compiler usage by TianoCore. When they care not set, they are treated as blank, so ENV(WHATEVER_PREFIX)gcc will resolve to just gcc - and as gcc's parent folder should be in PATH, it will work OOTB (/usr/bin/ should be in PATH too, so your new _PREFIX_variables are kinda obsolete). I would set them in ebuild rather than force the patch in the tools_def.

Zenith432 · April 11, 2016

Ok, here are the results for specific file you mentioned Thunk16. I'm not sure which variant was assembled, but each build (gcc53, xcode5) only assembled one variant. The gcc53 Thunk16.obj is an ELF file. The xcode5 Thunk16.obj is a mach-o file. To compare, I extracted the .text sections with dd. They're both 0x204 bytes in length and exactly identical.

Attached are their binary dumps and disassemblies (objdump for ELF file, otool or mach-o file).

There are 26 obj files created from assembly in the build. I'd really rather not go through this manually for each one.

I think we should compare binaries produced by gcc and by clang, those files that was written in ASM instead of C:

edk2/MdePkg/Library/BaseLib/X64/Thunk16.asm/.S/.nasm

Previously I was fighting with it and saw gcc is good, clang is not.

Other asm?

PS Fritz, don't you have commit access to the repository? How about organising this stuff?

PPS I looked over the map files in the Build again to see if there's any hope of tracing an instruction-pointer address printed by the exception handler to the code. Probably not without adding additional debug prints. It does some funny business with the EFI binaries using specialized EDK2 tools while packing them into the firmware volume.

Oh, and BTW, I realized that the uncompressed FV is not really 2.6MB. It's closer to about 880KB of actual code+data, and the rest is reserve space

Thunk16.tar.gz

mhaeuser · April 11, 2016

Back when I was doing some NVRAM refactoring (not setting stuff that is OS X's business), all I got was some sarcastic comments, so I will definitely do nothing not discussed before.

Also, I do not use Clover anymore and was only trying to give some input to good practise (not using = harder to validate changes), because I think that EDK2 should not be altered and changes should be done to Clover-internal forks, if necessary, and environment variables rather in the build script than in tools_def.

I occasionally built modded Clovers for a few people to test something, but I stopped for two reasons. 1) It no longer builds on Windows (I think GrubFS was the reason) and 2) I need to mod EDK2... though I am building more than only Clover and hence want a clean tree without messing around.

Zenith432 · April 11, 2016

The GrubFS drivers are not really necessary. I commented out their build in the .dsc/.fdf, and instead added entries for inserting a prebuilt binary. I take the binaries from efifs.

             T   D
             Y C I
             P F A
DRV VERSION  E G G #D  #C  DRIVER NAME                         IMAGE PATH
=== ======== = = = === === =================================== ==========
 3B 0000000A D N N   1   0 Platform Console Management Driver  MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(51CCF399-4FDF-4E55-A45B-E123F84D456A)
 3C 0000000A D N N   1   0 Platform Console Management Driver  
 3D 0000000A B N N   1   1 Console Splitter Driver             MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(408EDCEC-CF6D-477C-A5A8-B4844E3DE281)
 3E 0000000A B N N   1   1 Console Splitter Driver             
 3F 0000000A ? N N   0   0 Console Splitter Driver             
 43 0000000A D N N   1   0 Graphics Console Driver             MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(CCCB0C28-4B24-11D5-9A5A-0090273FC14D)
 46 00000010 D N N   1   0 efifs 0.8.0 exfat driver (GRUB 2.02 MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(6AE8348F-6DB5-4D7E-B8C2-378A1F111F68)
 48 00000000 D N N   2   0 Clover hfs File System Driver       MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(6E506CC2-65E0-4814-994A-08ECDA046987)
 49 0000000A B N N   1  16 PCI Bus Driver                      MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(35C0C168-2607-4E51-BB53-448E3ED1A87F)
 4A 00000010 B N N   1   1 BIOS[INT10] Video Driver            MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(29CF55F8-B675-4F5D-8F2F-B87A3ECFD063)
 4B 00000003 D N N   1   0 BIOS[INT16] Keyboard Driver         MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(5479662B-6AE4-49E8-A6BD-6DE4B625811F)
 4C 00000003 B N N   1   3 BIOS[INT13] Block Io Driver         MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(4495E47E-42A9-4007-8C17-B6664F909D04)
 4D 0000000A D N N   1   0 PC-AT ISA Device Enumeration Driver MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(38A0EC22-FBE7-4911-8BC1-176E0D6C1DBD)
 4E 0000000A B N N   1  18 ISA Bus Driver                      MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(240612B5-A063-11D4-9A3A-0090273FC14D)
 4F 0000000A D N N  14   0 Generic Disk I/O Driver             MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(6B38F7B4-AD98-40E9-9093-ACA2B5A253C4)
 52 0000000B B N N   3  11 Partition Driver(MBR/GPT/Apple)     MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(1FA1F39E-FEFF-4AAE-BD7B-38A070A3B609)
 53 0000000A D N N   2   0 FAT File System Driver              MemoryMapped(0xB,0xE6DBC000,0xE705BFFF)/FvFile(961578FE-B6B7-44C3-AF35-6BC705CD2B1F)
 84 00000010 D N N   5   0 efifs 0.8.0 ntfs driver (GRUB 2.02~ FS1:\EFI\CLOVER\drivers64\ntfs_x64.efi

dgsga · April 11, 2016

I found one problem with -xcode5 - is that varargs were being handled by the wrong section in patched Base.h - and this was causing mayhem. Fixed in #3429. Now the result for -xcode5 build is that CloverGUI works when run with boot6/7 made by gcc, but boot6/7 made by xcode5 crash with a CPU exception of "undefined opcode". I don't know how to locate the place in the code where it crashes from the register dump. I looked at some of the map files in Build, but couldn't figure it out.

I can confirm that build 3430 compiles a fully working bootx64.efi binary using -clang build flag. This is a first for me so great work Zenith432! It weighs in at only 647KB as opposed to 668KB for a binary compiled with GCC 5.3. Thanks to all working hard to improve this great bootloader

Slice · April 11, 2016

I can confirm that build 3430 compiles a fully working bootx64.efi binary using -clang build flag. This is a first for me so great work Zenith432! It weighs in at only 647KB as opposed to 668KB for a binary compiled with GCC 5.3. Thanks to all working hard to improve this great bootloader

It was working before. The problem always was with boot files only.

As well working Shell and OVMF compiled by clang.

Zenith432 · April 11, 2016

It was working before for people that don't use patched MdePkg/Include/Base.h from Patches_for_EDK2. The relevant part is

-#elif defined(__GNUC__) && !defined(NO_BUILTIN_VA_FUNCS)
+//#elif defined(__GNUC__) && !defined(NO_BUILTIN_VA_FUNCS)
+#elif defined(__GNUC__) && !defined(__x86_64__)

For clang builds, the symbol NO_BUILTIN_VA_FUNCS is not defined in tools_def.txt, so clang would use the builtin varargs implementation (with ms abi) - and it works. However, due to patch, the builtin varargs for x64 were only used with GCC 4.8 or above (not clang which reports 4.2.1). This would cause clang to use the varargs implementation defined after the #else clause. The fallback implementation after the #else is incorrect for clang in ms-abi mode, and messes up the stack. I tried CloverGUI built with xcode together with boot7 built with gcc53 before rev #3429 - and it hung.

Slice · April 11, 2016

It worked before 3319

--- a/Patches_for_EDK2/MdePkg/Include/Base.h
+++ b/Patches_for_EDK2/MdePkg/Include/Base.h
@@ -479,7 +479,7 @@
 #define VA_COPY(Dest, Start)          __va_copy (Dest, Start)
 
 //#elif defined(__GNUC__) && !defined(NO_BUILTIN_VA_FUNCS)
-#elif defined(__GNUC__) && !defined(__x86_64__) || __APPLE__
+#elif defined(__GNUC__) && !defined(__x86_64__)
 //
 // Use GCC built-in macros for variable argument lists.
 //

Take into account that clang before 3.7 was not correctly supported ms_va_args.

Only with ElCapitan we got good clang.

zxv · April 12, 2016

Just compiled Clover r3430 under Clang/XCode 7.3. It works.

Funny. Whereas CLOVERX64.efi became smaller (664K → 647K), the drivers became much larger—e.g. OsxAptioFix2Drv.efi went 18K → 24K.

Zenith432 · April 12, 2016

Funny. Whereas CLOVERX64.efi became smaller (664K → 647K), the drivers became much larger—e.g. OsxAptioFix2Drv.efi went 18K → 24K.

clang is set up in Clover.dsc to use -Os on all modules. GCC uses -O3 on a couple, and -O0 on others. However, GCC is set up to use lto (link-time-optimization). clang isn't. clang has -flto as well, but it needs modifying the command given to ld in order to set up. I haven't tried that because the firmware crashes with an exception anyway.

About the exception - there's a utility in BaseTools VolInfo for listing the contents of a firmware volume (FV). It's like a tar archive, and there is no way to find the load address for an .efi binary from it. The binary gets copied into a memory address from the memory allocator. Binaries are not page-aligned inside the FV, and are not run "in place" from the FV.

So what I'm thinking of is to add code to the exception handler to search down from the exception address for a COFF header, to display the position of the COFF header, and some values from it that are enough to uniquely identify the .efi binary containing the exception point.

Zenith432 · April 12, 2016

I found the exception. It's in BiosVideo

BiosVideo/LegacyBiosThunk.c

BOOLEAN
EFIAPI
LegacyBiosInt86 (
  IN  BIOS_VIDEO_DEV                 *BiosDev,
  IN  UINT8                           BiosInt,
  IN  IA32_REGISTER_SET              *Regs
  )
{
....

  Stack16 = (UINT16 *)((UINT8 *) BiosDev->ThunkContext->RealModeBuffer + BiosDev->ThunkContext->RealModeBufferSize - sizeof (UINT16));
Code generated by clang:
0000000000005f38	488b4620        	movq	0x20(%rsi), %rax
0000000000005f3c	8b4808          	movl	0x8(%rax), %ecx
0000000000005f3f	8b4010          	movl	0x10(%rax), %eax
0000000000005f42	8d4401fe        	leal	-0x2(%rcx,%rax), %eax

  ThunkRegSet.E.SS   = (UINT16) (((UINTN) Stack16 >> 16) << 12);
  ThunkRegSet.E.ESP  = (UINT16) (UINTN) Stack16;
Code generated by clang:
0000000000005f46	0fb7c8          	movzwl	%ax, %ecx
0000000000005f49	c1e804          	shrl	$0x4, %eax
0000000000005f4c	2500f00000      	andl	$0xf000, %eax           ## imm = 0xF000
0000000000005f51	6689442456      	movw	%ax, 0x56(%rsp)
0000000000005f56	894c242c        	movl	%ecx, 0x2c(%rsp)

  ThunkRegSet.E.Eip  = (UINT16)((UINT32 *)NULL)[BiosInt];
  ThunkRegSet.E.CS   = (UINT16)(((UINT32 *)NULL)[BiosInt] >> 16);
Code generated by clang:
0000000000005f5a	0f0b            	ud2
0000000000005f5c	0f0b            	ud2

mhaeuser · April 12, 2016

if (BiosInt != 0) {
  ThunkRegSet.E.Eip = (UINT16)*(UINT32 *)(UINTN)(BiosInt * sizeof (UINT32));
}

Maybe like that?

Slice · April 12, 2016

I found the exception. It's in BiosVideo

BiosVideo/LegacyBiosThunk.c

BOOLEAN
EFIAPI
LegacyBiosInt86 (
  IN  BIOS_VIDEO_DEV                 *BiosDev,
  IN  UINT8                           BiosInt,
  IN  IA32_REGISTER_SET              *Regs
  )
{
....

  Stack16 = (UINT16 *)((UINT8 *) BiosDev->ThunkContext->RealModeBuffer + BiosDev->ThunkContext->RealModeBufferSize - sizeof (UINT16));
Code generated by clang:
0000000000005f38	488b4620        	movq	0x20(%rsi), %rax
0000000000005f3c	8b4808          	movl	0x8(%rax), %ecx
0000000000005f3f	8b4010          	movl	0x10(%rax), %eax
0000000000005f42	8d4401fe        	leal	-0x2(%rcx,%rax), %eax

  ThunkRegSet.E.SS   = (UINT16) (((UINTN) Stack16 >> 16) << 12);
  ThunkRegSet.E.ESP  = (UINT16) (UINTN) Stack16;
Code generated by clang:
0000000000005f46	0fb7c8          	movzwl	%ax, %ecx
0000000000005f49	c1e804          	shrl	$0x4, %eax
0000000000005f4c	2500f00000      	andl	$0xf000, %eax           ## imm = 0xF000
0000000000005f51	6689442456      	movw	%ax, 0x56(%rsp)
0000000000005f56	894c242c        	movl	%ecx, 0x2c(%rsp)

  ThunkRegSet.E.Eip  = (UINT16)((UINT32 *)NULL)[BiosInt];
  ThunkRegSet.E.CS   = (UINT16)(((UINT32 *)NULL)[BiosInt] >> 16);
Code generated by clang:
0000000000005f5a	0f0b            	ud2
0000000000005f5c	0f0b            	ud2

Update: I changed the code to something else that compiles right. Now it's stuck again on a new problem with a blank screen.

Good!

I found 6 such functions in the project, we should find a working one.

Zenith432 · April 12, 2016

You're right, that's why it was still hanging. I found the same code in LegacyBiosThunk following places
DuetPkg/BiosVideoThunkDxe/LegacyBiosThunk.c
BiosKeyboard/LegacyBiosThunk.c
BiosVideo/LegacyBiosThunk.c
LegacyBios/BlockIoDxe/LegacyBiosThunk.c
rEFIt_UEFI/Platform/LegacyBiosThunk.c

I changed them all to working code, and now boot7 built with xcode works

Update: boot6 works too.

Good!
I found 6 such functions in the project, we should find a working one.

mhaeuser · April 12, 2016

I changed them all to working code, and now boot7 built with xcode works

What did you change, if I may ask?

Zenith432 · April 12, 2016

diff --git a/BiosKeyboard/LegacyBiosThunk.c b/BiosKeyboard/LegacyBiosThunk.c
--- a/BiosKeyboard/LegacyBiosThunk.c
+++ b/BiosKeyboard/LegacyBiosThunk.c
@@ -196,8 +196,13 @@ LegacyBiosInt86 (
   ThunkRegSet.E.SS   = (UINT16) (((UINTN) Stack16 >> 16) << 12);
   ThunkRegSet.E.ESP  = (UINT16) (UINTN) Stack16;
 
+#ifdef __clang__
+  ThunkRegSet.E.Eip  = *(UINT16*)(((UINTN)BiosInt) << 2);
+  ThunkRegSet.E.CS   = *(UINT16*)((((UINTN)BiosInt) << 2) + 2U);
+#else
   ThunkRegSet.E.Eip  = (UINT16)((UINT32 *)NULL)[BiosInt];
   ThunkRegSet.E.CS   = (UINT16)(((UINT32 *)NULL)[BiosInt] >> 16);
+#endif
   BiosDev->ThunkContext->RealModeState = &ThunkRegSet;
   AsmThunk16 (BiosDev->ThunkContext);
   
diff --git a/BiosVideo/LegacyBiosThunk.c b/BiosVideo/LegacyBiosThunk.c
--- a/BiosVideo/LegacyBiosThunk.c
+++ b/BiosVideo/LegacyBiosThunk.c
@@ -196,8 +196,13 @@ LegacyBiosInt86 (
   ThunkRegSet.E.SS   = (UINT16) (((UINTN) Stack16 >> 16) << 12);
   ThunkRegSet.E.ESP  = (UINT16) (UINTN) Stack16;
 
+#ifdef __clang__
+  ThunkRegSet.E.Eip  = *(UINT16*)(((UINTN)BiosInt) << 2);
+  ThunkRegSet.E.CS   = *(UINT16*)((((UINTN)BiosInt) << 2) + 2U);
+#else
   ThunkRegSet.E.Eip  = (UINT16)((UINT32 *)NULL)[BiosInt];
   ThunkRegSet.E.CS   = (UINT16)(((UINT32 *)NULL)[BiosInt] >> 16);
+#endif
   BiosDev->ThunkContext->RealModeState = &ThunkRegSet;
   AsmThunk16 (BiosDev->ThunkContext);
   
diff --git a/LegacyBios/BlockIoDxe/LegacyBiosThunk.c b/LegacyBios/BlockIoDxe/LegacyBiosThunk.c
--- a/LegacyBios/BlockIoDxe/LegacyBiosThunk.c
+++ b/LegacyBios/BlockIoDxe/LegacyBiosThunk.c
@@ -196,8 +196,13 @@ LegacyBiosInt86 (
   ThunkRegSet.E.SS   = (UINT16) (((UINTN) Stack16 >> 16) << 12);
   ThunkRegSet.E.ESP  = (UINT16) (UINTN) Stack16;
 
+#ifdef __clang__
+  ThunkRegSet.E.Eip  = *(UINT16*)(((UINTN)BiosInt) << 2);
+  ThunkRegSet.E.CS   = *(UINT16*)((((UINTN)BiosInt) << 2) + 2U);
+#else
   ThunkRegSet.E.Eip  = (UINT16)((UINT32 *)NULL)[BiosInt];
   ThunkRegSet.E.CS   = (UINT16)(((UINT32 *)NULL)[BiosInt] >> 16);
+#endif
   BiosDev->ThunkContext->RealModeState = &ThunkRegSet;
   AsmThunk16 (BiosDev->ThunkContext);
   
diff --git a/rEFIt_UEFI/Platform/LegacyBiosThunk.c b/rEFIt_UEFI/Platform/LegacyBiosThunk.c
--- a/rEFIt_UEFI/Platform/LegacyBiosThunk.c
+++ b/rEFIt_UEFI/Platform/LegacyBiosThunk.c
@@ -256,8 +256,13 @@ LegacyBiosInt86 (
   ThunkRegSet.E.SS   = (UINT16) (((UINTN) Stack16 >> 16) << 12);
   ThunkRegSet.E.ESP  = (UINT16) (UINTN) Stack16;
 
+#ifdef __clang__
+  ThunkRegSet.E.Eip  = *(UINT16*)(((UINTN)BiosInt) << 2);
+  ThunkRegSet.E.CS   = *(UINT16*)((((UINTN)BiosInt) << 2) + 2U);
+#else
   ThunkRegSet.E.Eip  = (UINT16)((UINT32 *)NULL)[BiosInt];
   ThunkRegSet.E.CS   = (UINT16)(((UINT32 *)NULL)[BiosInt] >> 16);
+#endif
   mThunkContext->RealModeState = &ThunkRegSet;
   AsmThunk16 (mThunkContext);

What did you change, if I may ask?

The instance in DuetPkg/BiosVideoThunkDxe turned out not to need patching because it's not used by Clover.

Update: I've committed rev 3431 with code that works for both gcc and clang, and also uses sizeof() instead of shifting.

Clover Problems and Solutions

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites