Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

sys/new_delete: add malloc/free based new/delete implementation #17464

Merged
merged 2 commits into from
Jan 4, 2022

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Jan 3, 2022

Contribution description

This PR adds a module extracted from the Arduino core to provide C++ new and delete operators.

On some platforms libstdc++ is not used or not available, like on the AVR. Such platforms can use this module to implement the C++ new and delete operators using malloc and free respectively. However, to be thread-safe, a thread-safe implementation of malloc and free must be present.

The new_delete module is used by default by AVR if cpp module is used.

Testing procedure

Green CI. Module is compiled as part of C++ compilation of the arduino module for ATmega boards.

Issues/PRs references

Prerequesite for #17460 and #12518

@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: sys Area: System Platform: AVR Platform: This PR/issue effects AVR-based platforms labels Jan 3, 2022
@gschorcht gschorcht added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 3, 2022
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some style suggestions and a question, see inline

void * operator new(size_t size, void * ptr) noexcept {
(void)size;
return ptr;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite get this one. I cannot find an API doc, but the arguments do not seem to make much sense unless this expected to behave somewhat like realloc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://www.cplusplus.com/reference/new/operator%20new/

void* operator new (std::size_t size, void* ptr) noexcept;

is the placement function which simply returns ptr and doesn't allocate storage:

ptr 
       A pointer to an already-allocated memory block of the proper size.
       If called by a new-expression, the object is initialized (or constructed) at this location.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The counterpart is

void operator delete (void* ptr, void* voidptr2) noexcept;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found an good example how it could be used at cppreference.com

// within any block scope...
{
    alignas(T) unsigned char buf[sizeof(T)];
    // Statically allocate the storage with automatic storage duration
    // which is large enough for any object of type `T`.
    T* tptr = new(buf) T; // Construct a `T` object, placing it directly into your 
                          // pre-allocated storage at memory address `buf`.
    tptr->~T();           // You must **manually** call the object's destructor
                          // if its side effects is depended by the program.
}                         // Leaving this block scope automatically deallocates `buf`.

@gschorcht
Copy link
Contributor Author

@maribu I didn't change anything, it's just the original code. Do you think it is necessary to bring it into RIOT style? The static tests are happy with the code as it is.

@gschorcht
Copy link
Contributor Author

@maribu Fixed the style including indentation.

@gschorcht gschorcht force-pushed the sys/new_delete_module branch from d9af56c to 8bdb9fd Compare January 3, 2022 19:33
@gschorcht
Copy link
Contributor Author

May I squash?

@gschorcht
Copy link
Contributor Author

Compilation fails for AVR. It seems that avr-g++ (gcc 5.4.0) is not C++11 compatible. I can observe for ESP32.

@maribu
Copy link
Member

maribu commented Jan 3, 2022

Compilation fails for AVR. It seems that avr-g++ (gcc 5.4.0) is not C++11 compatible. I can observe for ESP32.

That old GCC does support C++11, if explicitly enabled. If I recall correctly it is just not the default back than (but it is for more recent toolchains).

We could fix that with just adding CXXFLAGS += -std=c++11 - which would be consistent with using C11.

Either way, please squash at will.

On some platforms `libstdc++` is not used or not available, like on the AVR. Such platforms can use this module to implement the C++ `new` and `delete` operators using `malloc` and `free` respectively. However, to be thread-safe, a thread-safe implementation of `malloc` and `free` must be present.
@gschorcht gschorcht force-pushed the sys/new_delete_module branch from 8fb7b7d to d778e77 Compare January 3, 2022 22:35
@gschorcht
Copy link
Contributor Author

gschorcht commented Jan 3, 2022

We could fix that with just adding CXXFLAGS += -std=c++11 - which would be consistent with using C11.

👍 Indeed, had to do the same for ESP's nvs flash modules.

Squashed.

@maribu maribu merged commit 30b3f9a into RIOT-OS:master Jan 4, 2022
@gschorcht gschorcht deleted the sys/new_delete_module branch January 5, 2022 05:18
@gschorcht
Copy link
Contributor Author

Thanks for reviewing and merging.

With -std=c++14 we will get a compilation problem because not all possible operators are defined 😟

sys/cpp_new_delete/new_delete.cpp:34:6: error: the program should also define 'void operator delete(void*, unsigned int)' [-Werror=sized-deallocation]

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: AVR Platform: This PR/issue effects AVR-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants