Jief_Machak Posted April 24, 2020 Author Share Posted April 24, 2020 Nice summray of a windows install. I copied in a personnal readme file. Good to know it's compiling for you. Well, it's the linking that fails, so I can still compile to test VS compilation. I'll try a recompile from scratch later. Link to comment Share on other sites More sharing options...
Slice Posted April 24, 2020 Share Posted April 24, 2020 4 hours ago, Jief_Machak said: Nice summray of a windows install. I copied in a personnal readme file. Good to know it's compiling for you. Well, it's the linking that fails, so I can still compile to test VS compilation. I'll try a recompile from scratch later. I forgot to mention Python 3.8.2 to install for all users (run as admin). Link to comment Share on other sites More sharing options...
Slice Posted April 24, 2020 Share Posted April 24, 2020 clover\rEFIt_UEFI\Platform\Posix\abort.cpp(56) : error C2220: warning C4702: Link to comment Share on other sites More sharing options...
Slice Posted April 24, 2020 Share Posted April 24, 2020 Hmm if ( stop_at_panic ) { VA_LIST va; VA_START(va, format); panic_(format, va); VA_END(va); }else{ i_have_panicked = true; } So VS2017 see the line VA_END(va) is unreachable. Should it be DBG instead of panic() and panic at the end of the function? Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 24, 2020 Author Share Posted April 24, 2020 27 minutes ago, Slice said: Hmm if ( stop_at_panic ) { VA_LIST va; VA_START(va, format); panic_(format, va); VA_END(va); }else{ i_have_panicked = true; } So VS2017 see the line VA_END(va) is unreachable. Should it be DBG instead of panic() and panic at the end of the function? I've commented out VA_END and committed. Link to comment Share on other sites More sharing options...
Slice Posted April 24, 2020 Share Posted April 24, 2020 Next error clover\rEFIt_UEFI\cpp_unit_test\strcmp_test.cpp(104): warning C4477: "printf": ப ଠ "%lu" ॡ 㬥 ⨯ "unsigned long", ਠ⨢ 㬥 "1" ⨯ "size_t" Sorry for symbols. Looks like Microsoft can't control russian coding for VS2017. I can't decode this messages but I know warning 4477. it is using format %lu for variable of type size_t. Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 24, 2020 Author Share Posted April 24, 2020 I saw few minutes ago. Yes, it's supposed to be %zu for size_t. Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 24, 2020 Author Share Posted April 24, 2020 Just back home. Warning fixes committed. Link to comment Share on other sites More sharing options...
Slice Posted April 24, 2020 Share Posted April 24, 2020 Next test in VS2017. #if 1 //testing place { const CHAR16 *aaa = L"12345 "; const CHAR8 *bbb = "12345 "; DBG(" string %ls, size=%lld, len=%lld sizeof=%ld iStrLen=%lld\n", aaa, StrSize(aaa), StrLen(aaa), sizeof(aaa), iStrLen(bbb, 10)); const CHAR8* ссс = "Выход "; DBG(" string %ls, size=%lld, len=%lld sizeof=%ld iStrLen=%lld\n", ссс, AsciiStrSize(ссс), AsciiStrLen(ссс), sizeof(ссс), iStrLen(ссс, 10)); XString ddd = "Выход "_XS; // size_t sizex = ddd.allocatedSize(); DBG(" xstring %s, asize=%lld, sizeinbyte=%lld sizeof=%ld lastcharat=%lld\n", ddd.c_str(), ddd.allocatedSize(), ddd.sizeInBytes(), sizeof(ddd), ddd.indexOf(ddd.LastChar())); CHAR8 compatible[64]; UINT32 FakeLAN = 0x0030168c; UINT32 FakeID = FakeLAN >> 16; UINT32 FakeVendor = FakeLAN & 0xFFFF; snprintf(compatible, 64, "pci%x,%x", FakeVendor, FakeID); DBG(" FakeLAN = 0x%x\n", FakeLAN); DBG(" Compatible=%s strlen=%ld sizeof=%ld iStrLen=%lld\n", compatible, strlen(compatible), sizeof(compatible), iStrLen(compatible, 64)); // LowCase(compatible); // DBG(" Low Compatible=%s strlen=%ld sizeof=%ld iStrLen=%lld\n", compatible, // strlen(compatible), sizeof(compatible), iStrLen(compatible, 64)); DBG("void*=%ld int=%ld long=%ld longlong=%ld enum=%ld\n", sizeof(void*), sizeof(int), sizeof(long int), sizeof(long long), sizeof(EFI_ALLOCATE_TYPE)); } #endif The log is 0:168 0:000 string 12345 , size=16, len=7 sizeof=8 iStrLen=5 0:168 0:000 string ﯂⃤ , size=8, len=7 sizeof=8 iStrLen=5 0:168 0:000 xstring , asize=1, sizeinbyte=0 sizeof=16 lastcharat=0 0:168 0:000 FakeLAN = 0x30168c 0:169 0:000 Compatible=pci168c,30 strlen=10 sizeof=64 iStrLen=10 0:169 0:000 void*=8 int=4 long=4 longlong=8 enum=4 First of all I see DBG can't print russian letters while it could before these refactoring. ddd.allocatedSize() and sizeInBytes() are wrong. As well as lastchar. I will rebuild from clean folder. Link to comment Share on other sites More sharing options...
Slice Posted April 24, 2020 Share Posted April 24, 2020 Same result Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 24, 2020 Author Share Posted April 24, 2020 with this DBG(" xstring %s, asize=%zu, sizeinbyte=%zu sizeof=%zu lastcharat=%zu\n", ddd.c_str(), ddd.allocatedSize(), ddd.sizeInBytes(), sizeof(ddd), ddd.indexOf(ddd.LastChar())); I got xstring Выход , asize=13, sizeinbyte=12 sizeof=16 lastcharat=5 Don't jump to conclusion that DBG can't print russian letter. DBG can still send russian chars to the log. So I guess something is wrong when compiled by VS. I'll check the tomorrow. Link to comment Share on other sites More sharing options...
Slice Posted April 24, 2020 Share Posted April 24, 2020 55 minutes ago, Jief_Machak said: with this DBG(" xstring %s, asize=%zu, sizeinbyte=%zu sizeof=%zu lastcharat=%zu\n", ddd.c_str(), ddd.allocatedSize(), ddd.sizeInBytes(), sizeof(ddd), ddd.indexOf(ddd.LastChar())); I got xstring Выход , asize=13, sizeinbyte=12 sizeof=16 lastcharat=5 Don't jump to conclusion that DBG can't print russian letter. DBG can still send russian chars to the log. So I guess something is wrong when compiled by VS. I'll check the tomorrow. Yes, I just checked macOS compilation. Good here. 0:191 0:000 string 12345 , size=16, len=7 sizeof=8 iStrLen=5 0:191 0:000 string Выход , size=13, len=12 sizeof=8 iStrLen=10 0:191 0:000 xstring Выход , asize=12, sizeinbyte=11 sizeof=16 lastcharat=5 0:191 0:000 FakeLAN = 0x30168c 0:192 0:000 Compatible=pci168c,30 strlen=10 sizeof=64 iStrLen=10 0:192 0:000 void*=8 int=4 long=8 longlong=8 enum=4 So it is still the problem with VS encodings as long before. Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 24, 2020 Author Share Posted April 24, 2020 5 hours ago, Slice said: const CHAR8* ссс = "Выход "; DBG(" string %ls, size=%lld, len=%lld sizeof=%ld iStrLen=%lld\n", ссс, AsciiStrSize(ссс), AsciiStrLen(ссс), sizeof(ссс), iStrLen(ссс, 10)); Here, you are printing an utf8 string with %ls. Cannot work. Unfortunately, no printf warning on VS. When I try compilation with VS, I have no log. What are you cbuild parameter to build Clover with VS ? Link to comment Share on other sites More sharing options...
Pene Posted April 24, 2020 Share Posted April 24, 2020 (edited) Hi @Jief_Machak Well, so I installed today Ubuntu 20.04, and through to try to compile clover with Linux (it comes with GCC 9.3.0). Without changing anything, I realized it got to RunGenericMenu() @ rEFIT_UEFI/gui/REFIT_MENU_SCREEN.cpp and then halt on this line: ((*this).*(StyleFunc))(MENU_FUNCTION_INIT, NULL); Hmmm, so I thought to check if global vars are initialized. And, no - they weren't, because on Linux (at least with Ubuntu 20.04) it seems that .init_array is used instead of .ctors. So, I changed at .ctorss at BaseTools/Scripts/GccBase.lds to: .ctorss ALIGN(CONSTANT(COMMONPAGESIZE)) : { __beginning_of_section_ctors = .; /* create symbol for start of section */ KEEP (*(.ctors)) KEEP (*(.init_array)) __end_of_section_ctors = .; /* create symbol for end of section */ } It doesn't harm to keep both, as mac doesn't use .init_array. So let's look at "Build/Clover/RELEASE_GCC53/X64/rEFIt_UEFI/refit/DEBUG/CLOVERX64.dll" now: this looks good, it contains a function pointer, so let's see what's there: $ objdump -s -j .ctorss CLOVERX64-linux.dll CLOVERX64-Linux.dll: file format elf64-x86-64 Contents of section .ctorss: de480 b9020000 00000000 $ objdump -C -d CLOVERX64-linux.dll | grep -A 5 00000000000002b9 00000000000002b9 <_sub_I_65535_0.42785>: 2b9: 55 push %rbp 2ba: 48 8d 15 df a0 0d 00 lea 0xda0df(%rip),%rdx # da3a0 <NullXStrings> 2c1: 48 8d 05 00 ce 0b 00 lea 0xbce00(%rip),%rax # bd0c8 <vtable for XStringWArray+0x10> 2c8: 48 8d 0d dd 5c 07 00 lea 0x75cdd(%rip),%rcx # 75fac <XStringWArray::~XStringWArray() [clone .lto_priv.0]> 2cf: 48 89 e5 mov %rsp,%rbp Seems similar to what is in the binary produced by GCC on macOS - I will post this also as comparison: $ objdump -s -j .ctorss CLOVERX64-mac.dll CLOVERX64-mac.dll: file format elf64-x86-64 Contents of section .ctorss: de3c0 b9020000 00000000 $ objdump -C -d CLOVERX64-mac.dll | grep -A 5 00000000000002b9 00000000000002b9 <global constructors keyed to 65535_0_UefiApplicationEntryPoint.lib_0x108.42844>: 2b9: 55 push %rbp 2ba: 48 8d 15 1f a0 0d 00 lea 0xda01f(%rip),%rdx # da2e0 <NullXStrings> 2c1: 48 8d 05 40 cd 0b 00 lea 0xbcd40(%rip),%rax # bd008 <vtable for XStringWArray+0x10> 2c8: 48 8d 0d 01 5c 07 00 lea 0x75c01(%rip),%rcx # 75ed0 <XStringWArray::~XStringWArray() [clone .lto_priv.0]> 2cf: 48 89 e5 mov %rsp,%rbp OK, so I try this new binary produced on Linux, and it hangs when trying to initialize the global variables, at rEFIt_UEFI/cpp_util/global_ctor.cpp, when calling (*p)(); This was the debug output before the hang with the binary produced on linux: p: 7F142580, pend: 7F142588, size: 8 CTOR 7F142580 2132026752 CTOR 7F064000 2131116032 <halt> and, for comparison, by the binary produced on mac: p: 7F142380, pend: 7F142388, size: 8 CTOR 7F142380 2132026240 CTOR 7F0642B9 2131116729 <goes on> So it obviously hangs because is has a wrong address at *p - we saw in objdump that it is was the same "2b9" for both binaries - so it should be 7F0642B9, but it is 7F064000. Why? EDIT: To be convinced that this is the only problem, I changed: (*p)(); to (*p + 0x2b9)(); and indeed it produced a fully working binary. But I can't figure why it has a wrong address, when we saw at objdump that it is was right (2b9). Edited April 25, 2020 by Pene Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 25, 2020 Author Share Posted April 25, 2020 Something different with the piece of software that convert .dll to .efi, that would change that load address of the section ? Link to comment Share on other sites More sharing options...
Pene Posted April 25, 2020 Share Posted April 25, 2020 3 hours ago, Jief_Machak said: Something different with the piece of software that convert .dll to .efi, that would change that load address of the section ? Out of ideas really. Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 25, 2020 Author Share Posted April 25, 2020 14 hours ago, Slice said: So it is still the problem with VS encodings as long before. No, because UTF8 works in Visual Studio (whew). So this "XString ddd = "Выход "_XS;" works. With the new XString, you can even do "XString ddd = "Выход "_XSW;" I tried and I got : 2:286 0:000 string 12345 , size=16, len=7 sizeof=8 iStrLen=5 2:287 0:000 string Выход , size=13, len=12 sizeof=8 iStrLen=10 2:288 0:000 xstring Выход , asize=12, sizeinbyte=11 sizeof=16 lastcharat=5 Was there an encoding problem of the source file itself, so you got incorrect chars ? I guess that was the reason ? Link to comment Share on other sites More sharing options...
Slice Posted April 25, 2020 Share Posted April 25, 2020 29 minutes ago, Jief_Machak said: No, because UTF8 works in Visual Studio (whew). So this "XString ddd = "Выход "_XS;" works. With the new XString, you can even do "XString ddd = "Выход "_XSW;" I tried and I got : 2:286 0:000 string 12345 , size=16, len=7 sizeof=8 iStrLen=5 2:287 0:000 string Выход , size=13, len=12 sizeof=8 iStrLen=10 2:288 0:000 xstring Выход , asize=12, sizeinbyte=11 sizeof=16 lastcharat=5 Was there an encoding problem of the source file itself, so you got incorrect chars ? I guess that was the reason ? It was good in windows but converted by git I think. Else they will not be compiled. Link to comment Share on other sites More sharing options...
Pene Posted April 25, 2020 Share Posted April 25, 2020 (edited) On 4/25/2020 at 8:46 AM, Jief_Machak said: Something different with the piece of software that convert .dll to .efi, that would change that load address of the section ? Well, I figured that if I move the content of the .ctorss section into the .data section (with .init_array added to the KEEP, as previously mentioned), it gets the right address and produces working binaries (both on macOS and Ubuntu Linux 20.04). So, the changes I made to BaseTools/Scripts/GccBase.lds, are erasing the ".ctorss" section altogether and replacing the ".data" section with something like this: .data ALIGN(ALIGNOF(.text)) : ALIGN(CONSTANT(COMMONPAGESIZE)) { *(.data .data.* .gnu.linkonce.d.*) *(.bss .bss.*) __beginning_of_section_ctors = .; KEEP(*(.ctors .init_array)) __end_of_section_ctors = .; } I'm not sure why it works this way, whereas it got wrong data when it was in a separate .ctorss section, and I also don't know if it's right to do this. What do you think? And also an unrelated question: will the ctors/init_array always contain a single function pointer for us? As I saw, when debugging, that this is currently the case. Edited April 26, 2020 by Pene Link to comment Share on other sites More sharing options...
Slice Posted April 26, 2020 Share Posted April 26, 2020 What is the application of function .lastChar() if not an address of the last char? Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 26, 2020 Author Share Posted April 26, 2020 I've refactored boot options as a XStringArray, which allow to get rid of AddLoadOption and RemoveLoadOption If someone have the impression that something changed with the boot options, come to me 1 Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 26, 2020 Author Share Posted April 26, 2020 lastChar, just returns the last char. Definition is very simple and it's just a little convenience method. Useful to check if a path ends with a path separator or not. The idea with new string is to completely hide the encoding. The reason is because a char in an utf8 string can be 1,2,3 or 4 bytes. Therefore, if, for example, you want to replace that char by another, string[x] = new char doesn't work. So we have to not think anymore char or wchar as atomic unit containing a char. Getting the address of char seems not that useful. There is currently no need in Clover yet. That said, it could be useful to get the address, if for example, you want to skip the beginning of a string and give the address to a low level function. Probably more secure to use subString in that case. But if you want it, I can add a method like getAddressOfCharAt(pos)... Link to comment Share on other sites More sharing options...
Pene Posted April 26, 2020 Share Posted April 26, 2020 12 minutes ago, Jief_Machak said: I can't see any problem with that. And I think that it's common to create a section with one pointer for every static initialisation needed. Even Microsoft does it this way and god knows they like to be different. I surprise that the init section is called init_array on linux and .ctors on mac with the same compiler, though. Good job, thanks. I'll answer here, as it's more related to this thread. Well if you have nothing against it, I committed it. Now about .ctors vs .init_arary: it is kinda strange. I can say for sure that on all macOS only .ctors exsits, and that on Ubuntu 20.04 only .init_array exists. But it doesn't end here. According to what I read, .init_array is is the newer implementation, and it has one difference: .ctors function pointers should be initialized in backward order, and .init_array in forward order. But from what I saw during debugging, we actually have only one function pointer. That's why I asked you before if we will always have a single function pointer, because if yes, we probably should just call [0] and ignore the rest, for the case that cannot be completely excluded that there might be some other distributions that will have both .ctors and .init_array (it's only an assumption, as I only tested with Ubuntu 20.04). Link to comment Share on other sites More sharing options...
Jief_Machak Posted April 26, 2020 Author Share Posted April 26, 2020 In my memory, .ctor is not a single pointer. In globals_ctor, I iterate over the whole section to call all of them. Each one was a ctor of one and only one static object. I didn't ask myself if there was on order. So maybe we should improve globals_ctors. I don't think there is a pointer to call that will initialise everything. Link to comment Share on other sites More sharing options...
Pene Posted April 26, 2020 Share Posted April 26, 2020 (edited) 4 hours ago, Jief_Machak said: In my memory, .ctor is not a single pointer. In globals_ctor, I iterate over the whole section to call all of them. Each one was a ctor of one and only one static object. I didn't ask myself if there was on order. So maybe we should improve globals_ctors. I don't think there is a pointer to call that will initialise everything. You do iterate, but there is a single one. If you look at my sections above with DBG enabled, the first DBG is before the loop, and the second is the single function pointer produced from the loop (it's the DBG's you added there). It was the same on macOS and Linux. If you have time to spare, you can check for yourself and see that on macOS. For example: p: 7F142380, pend: 7F142388, size: 8 CTOR 7F142380 2132026240 CTOR 7F0642B9 2131116729 You can see also by the difference between pend and p that it's a single pointer. Edited April 26, 2020 by Pene Link to comment Share on other sites More sharing options...
Recommended Posts