C++ proposition

[Jief_Machak]

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.

[Slice]

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).

[Slice]

clover\rEFIt_UEFI\Platform\Posix\abort.cpp(56) : error C2220:

 

warning C4702:

[Slice]

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?

[Jief_Machak]

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.

[Slice]

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.

[Jief_Machak]

I saw few minutes ago. Yes, it's supposed to be %zu for size_t.

[Jief_Machak]

Just back home.

Warning fixes committed.

[Slice]

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.

[Slice]

Same result

 

[Jief_Machak]

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.

[Slice]

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.

[Jief_Machak]

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 ?

[Pene]

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

[Jief_Machak]

Something different with the piece of software that convert .dll to .efi, that would change that load address of the section ?

[Pene]

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.

[Jief_Machak]

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 ?

  

 

 

[Slice]

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.

[Pene]

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

[Slice]

What is the application of function .lastChar() if not an address of the last char?

[Jief_Machak]

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 

[Jief_Machak]

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)...

[Pene]

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).

 

[Jief_Machak]

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.

[Pene]

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