SavageAUS Posted September 21, 2017 Share Posted September 21, 2017 Even with updated tools_def and UefiLib.h i still cant build with xcode 9 with updated script 4.5.4. Running from: macOS 10.13 Xcode 9.0 Build version 9A235 <-------------------------------------------------- ================================================================================ Compiler settings <-------------------------------------------------- Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/usr/include/c++/4.2.1 Apple LLVM version 9.0.0 (clang-900.0.37) Target: x86_64-apple-darwin17.0.0 Thread model: posix InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin <-------------------------------------------------- ================================================================================ BUILDTOOL is XCODE9 <-------------------------------------------------- ================================================================================ nasm check: <-------------------------------------------------- NASM version 2.13.01 compiled on May 1 2017 ================================================================================ gettext check: <-------------------------------------------------- found gettext at /Users/shane/src/opt/local/bin ================================================================================ boot6 <-------------------------------------------------- TOOLCHAIN_DIR: /Users/shane/src/opt/local NASM_PREFIX: /Users/shane/src/opt/local/bin/ NASM_VER: 2.13.01 Initializing workspace Loading previous configuration from /Users/shane/src/edk2/Conf/BuildEnv.sh WORKSPACE: /Users/shane/src/edk2 EDK_TOOLS_PATH: /Users/shane/src/edk2/BaseTools CONF_PATH: /Users/shane/src/edk2/Conf Running edk2 build for CloverX64 using the command: build -D NO_GRUB_DRIVERS_EMBEDDED -D CHECK_FLAGS -D USE_LOW_EBDA -p Clover/Clover.dsc -a X64 -b RELEASE -t XCODE9 -n 5 Build environment: Darwin-17.0.0-x86_64-i386-64bit Build start time: 10:34:46, Sep.21 2017 WORKSPACE = /Users/shane/src/edk2 ECP_SOURCE = /Users/shane/src/edk2/EdkCompatibilityPkg EDK_SOURCE = /Users/shane/src/edk2/EdkCompatibilityPkg EFI_SOURCE = /Users/shane/src/edk2/EdkCompatibilityPkg EDK_TOOLS_PATH = /Users/shane/src/edk2/BaseTools CONF_PATH = /Users/shane/src/edk2/Conf Architecture(s) = X64 Build target = RELEASE Toolchain = XCODE9 Active Platform = /Users/shane/src/edk2/Clover/Clover.dsc Flash Image Definition = /Users/shane/src/edk2/Clover/Clover.fdf Processing meta-data .... done! Building ... /Users/shane/src/edk2/MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf [X64] Building ... /Users/shane/src/edk2/MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf [X64] Building ... /Users/shane/src/edk2/MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf [X64] make: Nothing to be done for `tbuild'. make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/BaseLib/BaseLib.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/BasePrintLib/BasePrintLib.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf [X64] Building ... /Users/shane/src/edk2/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf [X64] make: Nothing to be done for `tbuild'. make: Nothing to be done for `tbuild'. make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/Clover/CloverEFI/OsxDxeIpl/DxeIpl.inf [X64] Building ... /Users/shane/src/edk2/MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf [X64] Building ... /Users/shane/src/edk2/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf [X64] make: Nothing to be done for `tbuild'. make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf [X64] Building ... /Users/shane/src/edk2/MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/BasePeCoffExtraActionLibNull/BasePeCoffExtraActionLibNull.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/UefiLib/UefiLib.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf [X64] Building ... /Users/shane/src/edk2/MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/Clover/Library/VBoxPeCoffLib/VBoxPeCoffLib.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf [X64] [CC] DxeInit make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/Clover/Library/DuetTimerLib/DuetTimerLib.inf [X64] make: Nothing to be done for `tbuild'. Building ... /Users/shane/src/edk2/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf [X64] make: Nothing to be done for `tbuild'. /Users/shane/src/edk2/MdePkg/Library/UefiLib/UefiLib.c:1515:1: error: conflicting types for 'GetBestLanguage' Building ... /Users/shane/src/edk2/MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLibNull.inf [X64] GetBestLanguage ( ^ /Users/shane/src/edk2/MdePkg/Include/Library/UefiLib.h:819:1: note: previous declaration is here GetBestLanguage ( ^ 1 error generated. make: *** [/Users/shane/src/edk2/Build/Clover/RELEASE_XCODE9/X64/MdePkg/Library/UefiLib/UefiLib/OUTPUT/UefiLib.obj] Error 1 build.py... : error 7000: Failed to execute command make tbuild [/Users/shane/src/edk2/Build/Clover/RELEASE_XCODE9/X64/MdePkg/Library/UefiLib/UefiLib] build.py... : error F002: Failed to build module /Users/shane/src/edk2/MdePkg/Library/UefiLib/UefiLib.inf [X64, XCODE9, RELEASE] - Failed - Build end time: 10:34:54, Sep.21 2017 Build total time: 00:00:08 o_Ops, ./ebuild.sh exited with error(s), aborting.. Am i doing something wrong? Missing something? Link to comment Share on other sites More sharing options...
telepati Posted September 21, 2017 Share Posted September 21, 2017 Guys, is there any detailed guide about Clover hotpatch? Link to comment Share on other sites More sharing options...
RehabMan Posted September 21, 2017 Share Posted September 21, 2017 Guys, is there any detailed guide about Clover hotpatch? google "rehabman hotpatch acpi" 3 Link to comment Share on other sites More sharing options...
Slice Posted September 21, 2017 Share Posted September 21, 2017 it seems the new Patches are missing a matching declaration GetBestLanguage in UefiLib.h so add line below fixes things. and of course then it can be built with Xcode9 without warnings. $ svn diff /Extra/Clover_Install/CloverGrowerPro/edk2/MdePkg/Include/Library/UefiLib.h Index: /Extra/Clover_Install/CloverGrowerPro/edk2/MdePkg/Include/Library/UefiLib.h =================================================================== --- /Extra/Clover_Install/CloverGrowerPro/edk2/MdePkg/Include/Library/UefiLib.h (revision 25373) +++ /Extra/Clover_Install/CloverGrowerPro/edk2/MdePkg/Include/Library/UefiLib.h (working copy) @@ -819,6 +819,7 @@ GetBestLanguage ( IN CONST CHAR8 *SupportedLanguages, IN BOOLEAN Iso639Language, + IN CONST CHAR8 *Lang, ... ); Thanks, I forgot to commit this file. Check rev 4214. EDK2 developers just don't care about clang code requirement. See stackoverflow So I need again correct them. 4 Link to comment Share on other sites More sharing options...
ccphuc2016 Posted September 21, 2017 Share Posted September 21, 2017 (edited) Haha, me as well. @Badruzeus r4211 is fine with XCode5, but for compatible, use old remap SMBIOS method now. syscl where new link download Clover 4211. Thank Edited September 21, 2017 by ccphuc2016 Link to comment Share on other sites More sharing options...
SavageAUS Posted September 21, 2017 Share Posted September 21, 2017 Clover 4214 builds fine with Xcode 9 and build_clover 4.5.4 + previous fixes. Great work guys. Have all previous fixes been pushed to source? tools_def and UefiLib.h Link to comment Share on other sites More sharing options...
HmO Posted September 21, 2017 Share Posted September 21, 2017 where new link download Clover 4211. Thank Clover 4218 1 Link to comment Share on other sites More sharing options...
Slice Posted September 21, 2017 Share Posted September 21, 2017 Clover 4214 builds fine with Xcode 9 and build_clover 4.5.4 + previous fixes. Great work guys. Have all previous fixes been pushed to source? tools_def and UefiLib.h UefiLib.h is in Clover sources now. tools_def I see no reason to change. 1 Link to comment Share on other sites More sharing options...
ccphuc2016 Posted September 21, 2017 Share Posted September 21, 2017 Clover 4214 THANK 1 Link to comment Share on other sites More sharing options...
mhaeuser Posted September 21, 2017 Share Posted September 21, 2017 EDK2 developers just don't care about clang code requirement. See stackoverflow So I need again correct them. Sorry, what did you fix? All I see is a new, apparently unused parameter. Link to comment Share on other sites More sharing options...
Slice Posted September 21, 2017 Share Posted September 21, 2017 Sorry, what did you fix? All I see is a new, apparently unused parameter. See stackoverflow explanations and tricks. Instead of (...) you must explicitly show first parameter (void* a, ...) to avoid underfined behavior. So VariableGetBestLanguage ( IN CONST CHAR8 *SupportedLanguages, IN BOOLEAN Iso639Language, + IN CONST CHAR8 *Lang, ... ) { which is first parameter in variable list, and - VA_START (Args, Iso639Language); + VA_START (Args, Lang); because Boolean is not a parameter from the list. The list must contains strings languages. Best wishes! 3 Clang is right. See [support.runtime]/3. – Kerrek SB Dec 3 '15 at 15:32 Link to comment Share on other sites More sharing options...
mhaeuser Posted September 21, 2017 Share Posted September 21, 2017 Instead of (...) you must explicitly show first parameter (void* a, ...) to avoid underfined behavior. Yes, you need at least one parameter, GetBestLanguage() even has two. because Boolean is not a parameter from the list. The list must contains strings languages. I do not see why that would matter, neither the stackoverflow page you linked, nor any site regaridng VA args I quickly read suggest it. Actually, you even broke the code because the second parameter of va_start() must be the argument preceeding the variable argument list. Doing it the way you do it now drops the first member of the list, which is now the unused parameter 'Lang'. Would you mind sharing the error/warning that made you change the code? EDIT: Talked to vit9696 and yes, your intention is right, the standard is not a piece of cake to read... anyway, if you want to make it work, change the loop to a do-while and use Lang as first member of the list. Link to comment Share on other sites More sharing options...
Slice Posted September 21, 2017 Share Posted September 21, 2017 You may not use my patches if you didn't understand them. the unused parameter 'Lang'. -> - VA_START (Args, Iso639Language); + VA_START (Args, Lang); Do you see? 1 Link to comment Share on other sites More sharing options...
mhaeuser Posted September 21, 2017 Share Posted September 21, 2017 *sigh* Thanks, I understand your "fix" well enough, I was just unsure about something from the specification, which was not related to your patch being wrong, but for the edk2 code to be wrong in the first place. You pass Param1, Param2, Param3, Param4, Param5. Which map to: SupportedLanguages, Iso639Language, Lang and the first two members of the VA list. A real life example would be: SupportedLanguages = "en-usde-de", Iso649Language = FALSE, Lang = "en-us", VA1 = "de-de", VA2 = "fr-fr" Now, what happens when running your patch? 1. "VA_START (Args, Lang)" opens the VA list and the beginning of the loop 2. "while ((Language = VA_ARG (Args, CHAR8 *)) != NULL)" - VA_ARG() returns VA1, i.e. "de-de" 3. On the next interation, the same function returns "fr-fr" Where did "en-us" go? Might it not be used, because you never use the value of Lang? RIP "en-us", you will never be forgotten... And yes, I will not use your patches, not because I don't understand them, but because I prefer working code. 1 Link to comment Share on other sites More sharing options...
Slice Posted September 22, 2017 Share Posted September 22, 2017 Agree, first argument will be lost #include <stdio.h> #include <stdlib.h> #include <stdarg.h> #define IN #define CONST const #define CHAR8 char #define BOOLEAN unsigned char #ifndef VA_START // Use the native arguments for tools. #define VA_START va_start #define VA_ARG va_arg #define VA_END va_end #define VA_LIST va_list #endif CHAR8 * VariableGetBestLanguage ( IN CONST CHAR8 *SupportedLanguages, IN BOOLEAN Iso639Language, IN CONST CHAR8 *Lang, ... ) { CHAR8 *Language; VA_LIST Args; VA_START (Args, Lang); while ((Language = VA_ARG (Args, CHAR8 *)) != NULL) { printf("%s ", Language); } VA_END (Args); return ("\nHello!\n"); } int main(int argc, char ** argv) { printf("%s\n", VariableGetBestLanguage("en-usde-de", 0, "", "en-us", "de-de", "fr-fr")); return 0; } -> iMac-2:TestC slice$ ./test de-de fr-fr fr-fr Hello! But why fr-fr is double? Revert my patch and use -Wno-varargs OK, it works iMac-2:TestC slice$ clang test.c -o test1 -Wno-varargs iMac-2:TestC slice$ ./test1 en-us de-de fr-fr fr-fr Hello! But the clang is right. This is wrong code style. I can do better iMac-2:TestC slice$ clang test.c -o test2 iMac-2:TestC slice$ ./test2 en-us de-de fr-fr Hello! Link to comment Share on other sites More sharing options...
RehabMan Posted September 22, 2017 Share Posted September 22, 2017 But why fr-fr is double? The answer to your question is in the documentation/source code comments. @param[in] ... A variable argument list that contains pointers toNull-terminated ASCII strings that contain one or more language codes in the format specified by Iso639Language. ... The variable argument list is terminated by a NULL. In other words, your test case is bugged. You wrote: VariableGetBestLanguage("en-usde-de", 0, "", "en-us", "de-de", "fr-fr") Should be: VariableGetBestLanguage("en-usde-de", 0, "", "en-us", "de-de", "fr-fr", NULL) Note: My fix was to change "IN BOOLEAN Iso639Language," to "IN UINTN Iso639Language,". Based on my understanding of the compiler's complaint, it should be ok. Link to comment Share on other sites More sharing options...
RehabMan Posted September 22, 2017 Share Posted September 22, 2017 yes. I agree your code method is better - no warnings or overrides! with a slight code mod this example proves it without a doubt - Iso639Language value is passed properly .... { CHAR8 *Language; VA_LIST Args; // VA_START (Args, Iso639Language); VA_START (Args, Lang); while ((Language = VA_ARG (Args, CHAR8 *)) != NULL) { printf("%s ", Language); if (Iso639Language) printf("T "); else printf("F "); } VA_END (Args); return ("\nHello!\n"); } int main(int argc, char ** argv) { printf("%s\n", VariableGetBestLanguage("en-usde-de", 1, "", "en-us", "de-de", "fr-fr",NULL)); return 0; } $ a.out en-us T de-de T fr-fr T Hello! 2cents more... this way seems to me a bit more straight forward of a fix. You are still not understanding the intent of the original edk2 code. The edk2 code is correct. The compiler is complaining about a minor standards compliancy issue that does not affect the platform we are coding to. But, as I mentioned, it is easy to avoid the warning by using a type for the vararg anchor that is not subject to promotion. Link to comment Share on other sites More sharing options...
tluck Posted September 22, 2017 Share Posted September 22, 2017 You are still not understanding the intent of the original edk2 code. The edk2 code is correct. The compiler is complaining about a minor standards compliancy issue that does not affect the platform we are coding to. But, as I mentioned, it is easy to avoid the warning by using a type for the vararg anchor that is not subject to promotion. I do understand. i reverse my course - i am in agreement with you. using unsigned long (UINTN in edk2) keeps original code intent and avoids compiler nuisance. Link to comment Share on other sites More sharing options...
Slice Posted September 22, 2017 Share Posted September 22, 2017 The compiler do not warning variable size difference? BOOLEAN UINTN The edk2 code is correct. For gcc (linux) or VS (Windows). But they are not care about clang. We may just wait while someone send a message to edk2 list with the claim and someone resolves it. Link to comment Share on other sites More sharing options...
mhaeuser Posted September 22, 2017 Share Posted September 22, 2017 Note: My fix was to change "IN BOOLEAN Iso639Language," to "IN UINTN Iso639Language,". Based on my understanding of the compiler's complaint, it should be ok. Imo Slice's attempt is the better one because all types stay the same then, he would just need to turn thewhile-into a do-while-loop and use Lang on the first iteration. Clang thinks passing an argument that is default-argument-promoted to va_start is UB.. I read the spec and honestly, idk, when you're not used to all tge expressions, it's a pain to read. Basically depends on whether char is "compatible with" int, if I found the part Clang refers to. In regards to C11, it seems like the arg preceeding the VA needs to be of a compatible type with the one of the following var as well. I'm on phone, so can't check right now, but may you pass nothing to a VA list? If so, keeping "Lang" around would enforce at least passing one arg, which is good. In regards to C11 (or maybe even C99, the spec is a pain), literally defining the first member of the VA might be beneficial, idk. EDIT: char is not compatible with int and that is definitely an(the?) issue, but *MIGHT* not be the only one. It definitely isn't when C11 compliancy is a concern... thx vit EDIT2: Comments on C11 were false. 1 Link to comment Share on other sites More sharing options...
RehabMan Posted September 22, 2017 Share Posted September 22, 2017 he would just need to turn thewhile-into a do-while-loop and use Lang on the first iteration. I thought of that as well, but wanted to avoid changing the core logic so much. Link to comment Share on other sites More sharing options...
Gogeta5026 Posted September 22, 2017 Share Posted September 22, 2017 It took a 2-3 days but I figured out that...In CloverIf I have CsmVideoDxe-64.efi in my drivers64UEFI folder, I can use Clover to:1) Boot into Windows and Linux without any problems 2) I CAN NOT boot into High Sierra in any way, my screen is corrupt High Sierra with CsmVideoDxe-64.efi in my drivers64UEFI folder If I DONT't have CsmVideoDxe-64.efi in my drivers64UEFI folder, I can use Clover to:1) Boot into High Sierra without any problems 2) I CAN NOT boot into Windows and Linux Linux and Windows without CsmVideoDxe-64.efi in my drivers64UEFI folder (just a cursor)MY QUESTION IS...Is there a solution to this problem? Can I keep CsmVideoDxe-64.efi in my folder so, Windows and Linux will boot and disable it from High Sierra? Should I just take CsmVideoDxe-64.efi out but... How do I get into Windows and Linux through Clover then? Is there another way to fix this problem??? I have: Intel Core i7 7700K 4.2 Ghz Gigabyte GA-Z270MX/Gaming 5 Samsung CF791 Series 34-Inch Curved Widescreen Monitor (C34F791) 3440x1440 4K Samsung U28E590D 28-Inch UHD LED-Lit Monitor with Freesync support 3840x2160 4K AMD Radeon Pro Vega Air-Cooled Frontier Edition 16 GB ANY HELP WOULD BE GREAT! I attached my config.plist if that helps in any way for somebody. config.plist.zip Link to comment Share on other sites More sharing options...
mhaeuser Posted September 22, 2017 Share Posted September 22, 2017 I thought of that as well, but wanted to avoid changing the core logic so much. This should be submited to edk2-devel anyway, maintaing a modded "fork" is more work than getting the changes upstream once. Link to comment Share on other sites More sharing options...
MICKHAEL Posted September 22, 2017 Share Posted September 22, 2017 It took a 2-3 days but I figured out that... In Clover If I have CsmVideoDxe-64.efi in my drivers64UEFI folder, I can use Clover to: 1) Boot into Windows and Linux without any problems 2) I CAN NOT boot into High Sierra in any way, my screen is corrupt High Sierra with CsmVideoDxe-64.efi in my drivers64UEFI folder If I DONT't have CsmVideoDxe-64.efi in my drivers64UEFI folder, I can use Clover to:1) Boot into High Sierra without any problems 2) I CAN NOT boot into Windows and Linux Linux and Windows without CsmVideoDxe-64.efi in my drivers64UEFI folder (just a cursor) MY QUESTION IS... Is there a solution to this problem? Can I keep CsmVideoDxe-64.efi in my folder so, Windows and Linux will boot and disable it from High Sierra? Should I just take CsmVideoDxe-64.efi out but... How do I get into Windows and Linux through Clover then? Is there another way to fix this problem??? I have: Intel Core i7 7700K 4.2 Ghz Gigabyte GA-Z270MX/Gaming 5 Samsung CF791 Series 34-Inch Curved Widescreen Monitor (C34F791) 3440x1440 4K Samsung U28E590D 28-Inch UHD LED-Lit Monitor with Freesync support 3840x2160 4K AMD Radeon Pro Vega Air-Cooled Frontier Edition 16 GB ANY HELP WOULD BE GREAT! I attached my config.plist if that helps in any way for somebody. I think you enabled CSM and installed Windows/Linux in MBR ... search how to convert GPT, then disable CSM 1 Link to comment Share on other sites More sharing options...
nms Posted September 22, 2017 Share Posted September 22, 2017 The compiler do not warning variable size difference? BOOLEAN <-> UINTN For gcc (linux) or VS (Windows). But they are not care about clang. We may just wait while someone send a message to edk2 list with the claim and someone resolves it. Was already filed as bug #410. The solution -- hide bugs under rag )-; 3 Link to comment Share on other sites More sharing options...
Recommended Posts