C++ proposition

Jief_Machak · January 7, 2020

Hi,

I see you worked and gave jmacie this version to test : revision: 5102 (c++_globals, commit 81fdb66).

I don't think we should work in these branches. They are proof of concept. When we decide to go for C++ compiler, I'll do it in the master branch. Otherwise it may get out of control, don't you think ?

Jief_Machak · January 7, 2020

On 1/3/2020 at 6:36 PM, Download-Fritz said:

It's still not clear to me what the exact advantages are, or what problem you are trying to solve, I do not know C++ well enough to judge.

@Download-Fritz Hi,

for me object oriented language has the BIIIIIG advantage to be able to define type. The most common example is string. I'd like to have a string that I can manipulate like an int. I mean by that :

with an int, I can do

{
  int i = 2;
  i += 1;
  if ( i > 1 ) {
    return i; // the memory used by i is released, even though the execution doesn't continue
  }
} // here, the memory used by i is released

With a string object, like the one I propose (XStringW), I can do :

{
  XStringW s = L"my string"
  s += L"extended";            // of course, the meaning of operator + is different than for int !
  if ( s > L"my strong" ) {    // operator > different too
    return s; // destructor of s will be called, so no memory leaks
  }
} // destructor called.

The code is smaller, easier to read, no memory leaks possible.

You can also do more checks. For example, for a string-like object, you can define operator [] that breaks if the index is out of limits.

I also have a buffer object, an array object (vector-like) etc. (not included yet in my example). These are "fundation objects" or utility object.

Don't you think writing "if ( s1 == s2 )" is nicer than "if ( StrCmp(s1, s2) == 0 )" ?

Edited January 7, 2020 by Jief_Machak

mhaeuser · January 7, 2020

@Jief_Machak Thanks for your explaination, but well, I know how OOP works, the question is what the advantages are for edk2/Clover development - most of what you have mentioned you should keep to a minimum anyway.

Safe and easy string operations sound good in theory, but how often are those operations performed and how critical are they to keep to make up for the disadvantages? In practice, many UEFI firmwares are a bit borked and the last thing you want to do is stress the implementation. Constant allocations and deallocations could cause small memory leaks by the firmware itself, scatter memory around the address space (much contiguous space is essential for us), or even worse, trigger bad bugs that cause some sort of corruption. Some objects, like dynamic arrays or such, could have their use, but honestly I have not encountered such yet (at most a basic "guess size -> realloc on demand"), linked lists often make more sense considering they refer to individual objects (that may get added or removed on demand, like on a device connect).

Syntactic sugar - well meh, it's nice, but is it really worth the burden of introducing, validating and maintaining out-of-tree code?

I thought it maybe could make file parsing less terrible, but I'm probably not going to find out.

Jief_Machak · January 8, 2020

@Download-Fritz

Sure, the allocation deallocation mechanism should work well, for sure. But it seems that it's already done like this in Clover.

There few thing that can be done against constant allocation/deallocation :

1) not to use object and do everything manually. Like this, you control exactly what is done and when. It's a valid argument.

2) another could be to create few string objects with a fixed maximum length, for short life strings. Like string256 string1024. That way, when you declare a string, it goes on the stack instead of being dynamically allocated. And because it's an object, it can prevent memory overflow. Dynamically allocated string can still be used for long life string with unknown length at compile time.

3) Having our own implemented object allows to take memory from a private pool instead of AllocatePool. If there is Allocation/deallocation problem, it's easy to imaging that all the objects share a pool, declared globally, dynamically allocated but only once at start, and then each string object will get a piece of that, even being able to move not contiguous block to make room (in that case all chunks of allocated memory would have to be accessed through an indirection table). It probably recreating what AllocatePool does. But if it's not working well... why not recreating it better.

4) other ideas ?

Nice thing with object encapsulation, is that you can have different subsystem without changing the main code. Imagine we go for Allocated string and later realise it causes some problem, it's possible to write another way.

In every case, no out-of-tree code should maintained. I think we should switch to C++ compiler without introducing any objects. When we are sure the C++ compiler didn't introduce any bugs (or we had fixed them), we still have the freedom to decide if we introduce objects or not. If yes, which one and which kind. For example, a plist object.

Instead of having a huge huge toolbox, I think each object as a smaller API. If I want to know what can I do with an object, I just have tool in the class header file. For me it's better organised. Also a lot of syntactic sugar (ie operator definition). I have a sweet tooth.

But definitely not worth maintaining a parallel project , for sure. You're right.

mhaeuser · January 8, 2020

@Jief_Machak

1) No need for C++ then

2) That is a workaround for when assuming C++ is in place already

3) That sounds the best to me honestly, still assuming there are good reasons to even have a significant amount and frequency of allocations. An own allocator sounds especially useful for the case memory needs to be allocated at runtime, though especially that should be kept to an absolute minimum.

4) Well, my point is not to find workaround for issues C++ introduces, but that C++ introduces issues you need to find workaround for in the first place. Many things that make your life simpler, like automatic memory management, are things that have side effects you probably want to avoid.

By "out-of-tree" I meant edk2, not Clover.

My main point is, UEFI is not a nice userland to play around in. What it is meant to be is a feature-enhanced firmware, which by nature is security-critical and should be kept to a minimum to provide the features required, and what it is in reality is a borked piece of trash. You want to implement some sort of file system control, you discover the FAT32 driver is borked. You want to implement Internet Recovery, you discover the network stack is borked. You want to implement NVRAM logging, you discover NVRAM access is borked. A bunch of AptioFix is not supporting macOS, it's cleaning after AMI's {censored}ups. And in such an environment, you want to be nice and easy with calls, and conservative with resource consumption, especially when you deal with snowflakes like macOS that want a felt 4 GB (sarcasm obv) of contiguous memory in a limited range chosen at random.

Sherlocks · February 17, 2020

lastest commits, there is build fail

here is log

Spoiler

/Users/sherlocks/src/CloverBootloader/Library/OcGuardLib/Ubsan.c:1331:2: error: implicit declaration of function 'CopyMem' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

memcpy(rgNumber, &U128, sizeof(U128));

^

/Users/sherlocks/src/CloverBootloader/Library/OcGuardLib/Ubsan.h:200:37: note: expanded from macro 'memcpy'

#define memcpy(Dst, Src, Size) do { CopyMem(Dst, Src, Size); } while (0)

^

/Users/sherlocks/src/CloverBootloader/Library/OcGuardLib/Ubsan.c:1500:3: error: implicit declaration of function 'CopyMem' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

memcpy(&L, REINTERPRET_CAST(longest *, ulNumber), sizeof(longest));

^

/Users/sherlocks/src/CloverBootloader/Library/OcGuardLib/Ubsan.h:200:37: note: expanded from macro 'memcpy'

#define memcpy(Dst, Src, Size) do { CopyMem(Dst, Src, Size); } while (0)

^

/Users/sherlocks/src/CloverBootloader/Library/OcGuardLib/Ubsan.c:1544:3: error: implicit declaration of function 'CopyMem' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

memcpy(&UL, REINTERPRET_CAST(ulongest *, ulNumber), sizeof(ulongest));

^

/Users/sherlocks/src/CloverBootloader/Library/OcGuardLib/Ubsan.h:200:37: note: expanded from macro 'memcpy'

#define memcpy(Dst, Src, Size) do { CopyMem(Dst, Src, Size); } while (0)

^

3 errors generated.

make: *** [/Users/sherlocks/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/Library/OcGuardLib/OcGuardLib/OUTPUT/Ubsan.obj] Error 1

build.py...

: error 7000: Failed to execute command

make tbuild [/Users/sherlocks/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/Library/OcGuardLib/OcGuardLib]

build.py...

: error F002: Failed to build module

/Users/sherlocks/src/CloverBootloader/Library/OcGuardLib/OcGuardLib.inf [X64, XCODE8, RELEASE]

- Failed -

Build end time: 13:37:24, Feb.17 2020

Build total time: 00:00:10

sherlocks@SherloccBookPro ~ %

Slice · February 17, 2020

git pull

and make clean compilation.

Sherlocks · February 17, 2020

19 minutes ago, Slice said:

git pull

and make clean compilation.

i did clean all. and build again. strange. there is another issue.

Spoiler

[CC] usbfix

/Users/sherlocks/src/CloverBootloader/rEFIt_UEFI/cpp_util/XStringW.cpp:239:3: error: '__builtin_ms_va_start' used in System V ABI function

VA_START (va, format);

^

/Users/sherlocks/src/CloverBootloader/MdePkg/Include/Base.h:696:38: note: expanded from macro 'VA_START'

#define VA_START(Marker, Parameter) __builtin_ms_va_start (Marker, Parameter)

^

/Users/sherlocks/src/CloverBootloader/rEFIt_UEFI/cpp_util/XStringW.cpp:464:3: error: '__builtin_ms_va_start' used in System V ABI function

VA_START (va, format);

^

/Users/sherlocks/src/CloverBootloader/MdePkg/Include/Base.h:696:38: note: expanded from macro 'VA_START'

#define VA_START(Marker, Parameter) __builtin_ms_va_start (Marker, Parameter)

^

2 errors generated.

make: *** [/Users/sherlocks/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/rEFIt_UEFI/refit_cpp/OUTPUT/cpp_util/XStringW.obj] Error 1

build.py...

: error 7000: Failed to execute command

make tbuild [/Users/sherlocks/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/rEFIt_UEFI/refit_cpp]

build.py...

: error F002: Failed to build module

/Users/sherlocks/src/CloverBootloader/rEFIt_UEFI/refit_cpp.inf [X64, XCODE8, RELEASE]

- Failed -

Build end time: 14:27:14, Feb.17 2020

Build total time: 00:01:37

sherlocks@SherloccBookPro ~ %

Slice · February 17, 2020

I tested with Xcode11.3 toolset is XCODE8 and with gcc9.2 toolset GCC53. I see no issue.

Jief_Machak · February 17, 2020

You have to regenerate conf folder, I think.

Slice · February 17, 2020

12 minutes ago, Jief_Machak said:

You have to regenerate conf folder, I think.

Good idea. I forgot to commit changes to *.templates that is the reason

I committed the changes now.

Jief_Machak · February 17, 2020

I think we should commit deletion of the content of conf folder and also erase the content of Conf folder at each compilation. The re-generation is so quick, we can afford to do it each time.

Slice · February 17, 2020

6 minutes ago, Jief_Machak said:

I think we should commit deletion of the content of conf folder and also erase the content of Conf folder at each compilation. The re-generation is so quick, we can afford to do it each time.

It requires changes to compilation scripts.

I just never change it. But yes it will be good for beginners.

Jief_Machak · February 17, 2020

I'm suggesting that every one hold on few hours. I proposed to finish to switch to C++, do the automatic conf regeneration in the next few hours.

I will have to do few commits, and they may not compile straight away.

I'll post when it's back on track.

Is ok for every one ?

Jief_Machak · February 17, 2020

Ok. It compiling now on my High Sierra.

I'm using this to build : ./ebuild.sh --xcode5 --buildtarget=DEBUG --define=DEBUG_ON_SERIAL_PORT -fr -n 1

As a test, I've temporarily activated some console log. When clover starts, you should see what's one the picture. The most important part is that g_str = g_str:foobar. It must not say g_str = g_str:<NULL>. Could you all confirm this ? I'll then remove these log.

Is there anyone using --xcode8 to build ?

4 hours ago, Sherlocks said:

i did clean all. and build again. strange. there is another issue.

Hide contents

[CC] usbfix

/Users/sherlocks/src/CloverBootloader/rEFIt_UEFI/cpp_util/XStringW.cpp:239:3: error: '__builtin_ms_va_start' used in System V ABI function

VA_START (va, format);

^

/Users/sherlocks/src/CloverBootloader/MdePkg/Include/Base.h:696:38: note: expanded from macro 'VA_START'

#define VA_START(Marker, Parameter) __builtin_ms_va_start (Marker, Parameter)

^

/Users/sherlocks/src/CloverBootloader/rEFIt_UEFI/cpp_util/XStringW.cpp:464:3: error: '__builtin_ms_va_start' used in System V ABI function

VA_START (va, format);

^

/Users/sherlocks/src/CloverBootloader/MdePkg/Include/Base.h:696:38: note: expanded from macro 'VA_START'

#define VA_START(Marker, Parameter) __builtin_ms_va_start (Marker, Parameter)

^

2 errors generated.

make: *** [/Users/sherlocks/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/rEFIt_UEFI/refit_cpp/OUTPUT/cpp_util/XStringW.obj] Error 1

build.py...

: error 7000: Failed to execute command

make tbuild [/Users/sherlocks/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/rEFIt_UEFI/refit_cpp]

build.py...

: error F002: Failed to build module

/Users/sherlocks/src/CloverBootloader/rEFIt_UEFI/refit_cpp.inf [X64, XCODE8, RELEASE]

- Failed -

Build end time: 14:27:14, Feb.17 2020

Build total time: 00:01:37

sherlocks@SherloccBookPro ~ %

Got the same errors as you when using --xcode8. Could you try --xcode5, while I'm fixing --xcode8 ?

Jief_Machak · February 17, 2020

--xcode8 is fixed. Looks like it does the exact same compilation as --xcode5, except for 2 flags : -mno-red-zone and -mno-implicit-float is added when using --xcode8

Compilation with --gcc53 works too.

Matgen84 · February 17, 2020

23 minutes ago, Jief_Machak said:

--xcode8 is fixed. Looks like it does the exact same compilation as --xcode5, except for 2 flags : -mno-red-zone and -mno-implicit-float is added when using --xcode8

Compilation with --gcc53 works too.

I've some issue trying to compile with xcode8. How to solve it, please.

Spoiler

/bin/sh: /usr/local/bin/mtoc.NEW_jief: No such file or directory
make: *** [/Users/mathieu/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/Protocols/AppleKeyAggregator/AppleKeyAggregator/OUTPUT/AppleKeyAggregator.efi] Error 127

build.py...
: error 7000: Failed to execute command
make tbuild [/Users/mathieu/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/Protocols/AppleKeyAggregator/AppleKeyAggregator]

build.py...
: error 7000: Failed to execute command
make tbuild [/Users/mathieu/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/CloverEFI/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe]

build.py...
: error 7000: Failed to execute command
make tbuild [/Users/mathieu/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/CloverEFI/BiosKeyboard/KeyboardDxe]

build.py...
: error 7000: Failed to execute command
make tbuild [/Users/mathieu/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/LegacyBios/VideoDxe/VideoDxe]

build.py...
: error 7000: Failed to execute command
make tbuild [/Users/mathieu/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/Drivers/UsbKbDxe/UsbKbDxe]

build.py...
: error 7000: Failed to execute command
make tbuild [/Users/mathieu/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe]

build.py...
: error 7000: Failed to execute command
make tbuild [/Users/mathieu/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe]

build.py...
: error 7000: Failed to execute command
make tbuild [/Users/mathieu/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/FileSystems/GrubFS/src/HFSPLUS]

build.py...
: error 7000: Failed to execute command
make tbuild [/Users/mathieu/src/CloverBootloader/Build/Clover/RELEASE_XCODE8/X64/rEFIt_UEFI/refit]

build.py...
: error F002: Failed to build module
/Users/mathieu/src/CloverBootloader/Protocols/AppleKeyAggregator/AppleKeyAggregator.inf [X64, XCODE8, RELEASE]

- Failed -
Build end time: 12:42:31, Feb.17 2020
Build total time: 00:00:48

Jief_Machak · February 17, 2020

Yes, there is a new mtoc involved. But it should have automatically be compiled.

What is your build command ?

Matgen84 · February 17, 2020

10 minutes ago, Jief_Machak said:

Yes, there is a new mtoc involved. But it should have automatically be compiled.

What is your build command ?

The mtoc.New_jief won't be create if mtoc.NEW is present in src/opt/local/bin and /usr/local/bin/. So I rename mtoc.NEW, I can build your new mtoc and compile Clover with success.

My script use only: ./ebuild.sh -fr. All works fine before. What are the good arguments? Please.

Jief_Machak · February 17, 2020

That is not normal that mtoc.New_jief is not built if mtoc.NEW exists. I'll check.

I build with ./ebuild.sh --xcode5 --buildtarget=DEBUG --define=DEBUG_ON_SERIAL_PORT -fr -n $njobs $@

Matgen84 · February 17, 2020

2 minutes ago, Jief_Machak said:

That is not normal that mtoc.New_jief is not built if mtoc.NEW exists. I'll check.

I build with ./ebuild.sh --xcode5 --buildtarget=DEBUG --define=DEBUG_ON_SERIAL_PORT -fr -n $njobs $@

I take a look on my fork Dids's script.

Jief_Machak · February 17, 2020

Fix committed. Just forgot to rename in ebuild.sh.

Matgen84 · February 17, 2020

7 minutes ago, Jief_Machak said:

Fix committed. Just forgot to rename in ebuild.sh.

All works fine here with Xcode 8 and my fork script.

What is the right place of mtoc.New_Jief:

src/opt/local/bin ---> only
src/opt/local/bin + usr/local/bin

Because buildmtoc.sh don't create it in usr/local/bin (or it's the fault of my script)!

Jief_Machak · February 17, 2020

Just now, Matgen84 said:

All works fine here with Xcode 8 and my fork script.

What is the right place of mtoc.New_Jief:

src/opt/local/bin ---> only

src/opt/local/bin + usr/local/bin

Because buildmtoc.sh don't create it in usr/local/bin (or it's the fault of my script)!

I have to admit that I have a TOOLCHAIN var exported before calling ebuild, because I like all tools to stay inside the folder.

It doesn't need to be in 2 places.
If you remove all your mtoc, the next time you call ebuild it should be compiled and put in your TOOLCHAIN dir, which is the best place, I think.

Matgen84 · February 17, 2020

7 minutes ago, Jief_Machak said:

I have to admit that I have a TOOLCHAIN var exported before calling ebuild, because I like all tools to stay inside the folder.

It doesn't need to be in 2 places.
If you remove all your mtoc, the next time you call ebuild it should be compiled and put in your TOOLCHAIN dir, which is the best place, I think.

Thanks a lot

C++ proposition

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