Fix operator new

[kibergus]

Implementation of operator new in arduino contradicts C++ standard.
If malloc returned NULL it MUST throw exception or cal std::terminate in case if exceptions are disabled.
If you want operator new to return NULL just use non throw version of it which is not provided by arduino. Beside that, there should be operator new[] and placement variants of the operators.

There is correct realization in uclibc++ and I suggest using it instead of current implementation. To achieve it gnu ABI specific functions must be moved to separate header (see #107). After that new.h and new.cpp must be deleted and uclibc++ should be used instead.

If you have strong objections against uclibc++ but are still concerned about quality of the library, I can separate implementation of operator new from uclibc++ and provide patches for integrating it to arduino. But this is not a right way because it would tear standard library implementation to peaces.

[matthijskooijman]

Hmm. I agree that implementing things by the spec makes sense, but currently Arduino always compiles with -fno-exceptions, which means we cannot implement a throwing-new function.

Because of the extra complexity for users and the extra runtime overhead, I think that enabling exceptions within the Arduino environment is not going to happen. Also, it does not seem reasonable for all user code to use the nothrow syntax new (nothrow) T when they don't care about exceptions at all.

Of course, when someone is compiling the Arduino core without -fno-exceptions, it would be wrong to define a new operator that does not throw. However, since the Arduino core itself doesn't really know about exceptions and in particular does not define the std::bad_alloc exception, it seems reasonable that if exceptions are enabled, then the Arduino core should simply not define any new or delete operators at all. Users that still need them should make sure they include proper versions that throw, for example by linking against uclibc++ like you did.

Now, you suggest that with -fno-exceptions, the only option for the new operator is to throw an exception, or call std::terminate. However, I'm not entirely positive that that is what the standard says.

Looking at the C++ 2003 spec (ISO/IEC 14882:2003)

Section 3.7.3.1 describes what "allocation functions" (e.g., operator new functions, either in global scope or as static members of classes) look like and how they should behave. Paragraph 3 is about allocation failures:

An allocation function that fails to allocate storage can invoke the
currently installed new_handler (18.4.2.2), if any. [Note: A
program-supplied allocation function can obtain the address of the
currently installed new_handler using the set_new_handler function
(18.4.2.3). ] If an allocation function declared with an empty
exception-specification (15.4), throw(), fails to allocate storage, it
shall return a null pointer. Any other allocation function that fails
to allocate storage shall only indicate failure by throw- ing an
exception of class std::bad_alloc (18.4.2.1) or a class derived from
std::bad_alloc.

This shows that an allocation function that is not declared to throw an exception is allowed to return NULL.

A bit further, section 5.3.4 describes how the "new expression" should work. In particular, paragraph 13 describes what happens when the allocation function returns NULL:

[Note: unless an allocation function is declared with an empty
exception-specification (15.4), throw(), it indicates failure to
allocate storage by throwing a bad_alloc exception (clause 15,
18.4.2.1); it returns a non-null pointer otherwise. If the allocation
function is declared with an empty exception-specification, throw(),
it returns null to indicate failure to allocate storage and a non-null
pointer otherwise. ] If the allocation function returns null,
initialization shall not be done, the deallocation function shall not
be called, and the value of the new-expression shall be null.

This means that the code generated for a "new expression" is required to handle the "return NULL" case when the operator new that is used is declared to not throw an exception. In particular, this prevents problems with the constructor being called with a NULL this pointer.

It occurs to me that this support is mostly intended for when a class supplies a static operator new member, so that it can just return NULL instead, but the way it is stated in the specification means it should also apply to the global operator new function.

Furthermore section 18.4.1.1 specifies the operator new that the compiler or standard library must supply. Paragraph 3 specifies its behaviour:

Required behavior: Return a non-null pointer to suitably aligned
storage (3.7.3), or else throw a bad_alloc exception. This requirement
is binding on a replacement version of this function.

Note that calling abort() or std::terminate() is not an option here (though arguably if you terminate the program, you're sortof not violating this requirement). I was planning to argue that this behaviour is required only of the library implmentation, not of a user-supplied implementation (and as far as the C++ spec is concerned, the Arduino core is user code). However, it turns out that the user-supplied replacement function is actually required to implement the same behaviour, as stated above.

Now, with all the above in mind, let's get back to -fno-exceptions. AFAIU, the C++ specification does not say anything about such an option - for things to be valid C++, you need to support exceptions. I think this also means that if we do disable exceptions, we can't strictly follow the C++ specification anymore. This make sense, since the C++ spec says "Succeed or throw an exception", but if we can't throw an exception we can't magically expect allocation to succeed.

Given all of the above, I'd suggest the following:

• Only define new and delete functions if -fno-exceptions is specified (i.e., if __EXCEPTIONS is not defined).
• Copy the implementation from uclibc++ (since that also handles calling operator new with a zero size, for example, and has a clear LGPL license).
• Modify the uclibc++ implementation to declare it does not throw any exceptions (throw()) and make it return NULL on failure.

How does this sound? IIUC, this would solve your specific problem, since you can then enable exceptions and link against uclibc++ without problems, right?

[kibergus]

I absolutely agree that exceptions should not be enabled by default in arduino IDE. And I'm not sure that it is possible to enable them: I don't remember well, but there were missing parts which would not allow to build program with exceptions for AVR. So my suggestion was for the case when -fno-exceptions has been specified.
Now, if -fno-exceptions is specified, you can't compile progtram with try-catch section. So we can be sure, that if operator new some how throws std::bad_alloc, than nobody would catch it and std::terminate would be called. And it is even allowed by standard not to unwind stack if nobody catches the exception. That's why I suggest to call std::terminate (or abort()) in throwing version of operator new in case if exceptions are disabled and malloc has returned 0.
This is just my interpretation of -fno-exceptions for operator new: imagine that exception has been thrown and then do everything what is required in this case.

P.S. My intention was not to enable exceptions. I just don't know how to handle std::bad_alloc in most cases. And many programmers who I know have the same oppinion. std::bad_alloc means that my program can't work at all and should be terminated. So I wanted this termination to be automatical. Make it crash in safe and predictable place, rather when zero pointer is accessed. So idea in explicetely calling nothrow version of new is to additionally remind a programmer to check returned value.

P.P.S. I've used dinamic memory allocation only on initialization stage. So termination of embedded program if it can't be started correctly was exactly what I needed.

[matthijskooijman]

Ah, if you look at it like that, it actually seems you can still conform
to the spec, even with -fno-exceptions and calling abort or terminate
is indeed the right thing to do.

Also, you make a good point that terminating the program is better than
continuing with an unexpected NULL pointer somewhere. I previously said
that we can't expect users to use the new (nothrow) syntax because
typical Arduino users don't know what that would mean, but I now also
realize that in practice a typical Arduino user won't actually check the
return value of a new call either (hence, better to terminate instead of
returning NULL).

Looking at your pullrequest, you suggest to remove new and delete and
include uclibc++. I don't suppose this is likely to happen, unless it
can happen without much overhead.

So, if we want to provide a new implementation, but not include full
uclibc++, what would we need to do?

Perhaps something like this:

• Copy the new and delete implementations from uclibc++
• Change it to call abort() instead of calling std::__throw_bad_alloc()

I guess it still make sense to define new and delete only when
__EXCEPTIONS is not defined, just in case compiling with exceptions
ever becomes possible (which would make this implementation incorrect).

Would this allow you to use uclibc++ in the way you intend to?

[matthijskooijman]

One more thing: I guess it would make sense to also copy the nothrow versions from uclibc++, to allow people to use nothrow when they know what they're doing and want to check the new return value.

[kibergus]

Looking at your pullrequest, you suggest to remove new and delete and
include uclibc++. I don't suppose this is likely to happen, unless it
can happen without much overhead.

I agree with you. Intergating uclibc++ and maintaining it would require too big efforts.

Perhaps something like this:
Copy the new and delete implementations from uclibc++
Change it to call abort() instead of calling std::__throw_bad_alloc()

This logic is already implemented in uclibc++. std::__throw_bad_alloc is defined here:
https://github.com/kibergus/StandardCplusplus/blob/master/func_exception.cpp
And it is a tool to call std::terminate in case of disabled exceptions and throwing exceptions if they are enabled.

You can replace std::terminate() with call to abort(), but I like having std::terminate more, because user can set his own terminate handler, which would do something more appropriate, than spinning in endless loop. Possible variants:

• Spin in a loop and blink SOS with a onboard led, indicating, that board didn't just hang, but specific error occured.
• Switch off all outputs as soon as error occured to transfer electronics controlled by the board to a safe state.
• Indicate, that there is a problem, wait a bit and reboot the board.
  This costs one global function pointer in RAM and default std::terminate implementation, which contains single call call to abort() in ROM.

One more thing: I guess it would make sense to also copy the nothrow versions from uclibc++, to allow people to use nothrow when they know what they're doing and want to check the new return value.

Yes, this would be good.

Would this allow you to use uclibc++ in the way you intend to?

I'm fine even with slightly hacked arduino IDE. Or I can remove new and delete implementation from arduino port of uclibc++.

[matthijskooijman]

I saw the std::__throw_bad_alloc already. However, we can't use that as-is in uclibc, since we don't define the bad_alloc class. Hower, we can of course define std::__throw_bad_alloc anyway but make it a weak definition so it can be overridden (e.g., when you link against uclibc++ with exceptions enabled, it should automatically use the exception-throwing version).

Additionally, we could define a weak version of std::terminate that just calls abort(), so users can override it with their own version at compile time (instead of at runtime with std::set_terminate). If you link agains uclibc++, then you'll get the uclibc++ std:terminate, which does support runtime override.

I also just noticed that uclibc++'s operator new has a throws(std::bad_alloc) clause, which we of course can't have in the Arduino implementation (for lack of a std::bad_alloc class). I think we can just remove the throws declaration, meaning the operator new can possibly throw any exception, right?

So:

• Copy the new and delete implementations from uclibc++ (including array, placement and nothrow versions).
• Change it to remove throw(std::bad_alloc).
• Define a weak std::__throw_bad_alloc that calls std::terminate()
• Define a weak std::terminate that calls abort()

Would that work?

[matthijskooijman]

@kibergus, any comments on my latest proposal? :-)

[kibergus]

Sorry for slow reaction: I couldn't answer your letter immediately and then forgot about it. I think, tha your suggestions are very good. Thank you.

[matthijskooijman]

Ok, now only to implement it :-)

If you want to have a crack at a pullrequest for this (against the ide-1.5.x branch, I think would be best), feel free. If not, I'll probably do it once I find some time (not for a few weeks at least, though).

[matthijskooijman]

@matthijskooijman

[facchinm]

If still interesting, I ported the PR to https://github.com/arduino/ArduinoCore-avr . If someone wants to adopt it, please open a PR using this link https://github.com/arduino/ArduinoCore-avr/compare/pr_108?expand=1 , thanks.

[matthijskooijman]

@facchinm, I think the changes proposed in comments are still interesting, not so much the PR itself. I've opened an issue in the AVR repo about this to keep track.