Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

new w/ OOM now aborts by defaults, or throw an exception #7536

Merged
merged 11 commits into from
Aug 31, 2020
238 changes: 68 additions & 170 deletions boards.txt

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions cores/esp8266/Esp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -740,17 +740,17 @@ String EspClass::getSketchMD5()
}
uint32_t lengthLeft = getSketchSize();
const size_t bufSize = 512;
std::unique_ptr<uint8_t[]> buf(new uint8_t[bufSize]);
std::unique_ptr<uint8_t[]> buf(new (std::nothrow) uint8_t[bufSize]);
uint32_t offset = 0;
if(!buf.get()) {
return String();
return emptyString;
}
MD5Builder md5;
md5.begin();
while( lengthLeft > 0) {
size_t readBytes = (lengthLeft < bufSize) ? lengthLeft : bufSize;
if (!flashRead(offset, reinterpret_cast<uint32_t*>(buf.get()), (readBytes + 3) & ~3)) {
return String();
return emptyString;
}
md5.add(buf.get(), readBytes);
lengthLeft -= readBytes;
Expand Down
4 changes: 2 additions & 2 deletions cores/esp8266/Print.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ size_t Print::printf(const char *format, ...) {
size_t len = vsnprintf(temp, sizeof(temp), format, arg);
va_end(arg);
if (len > sizeof(temp) - 1) {
buffer = new char[len + 1];
buffer = new (std::nothrow) char[len + 1];
if (!buffer) {
return 0;
}
Expand All @@ -86,7 +86,7 @@ size_t Print::printf_P(PGM_P format, ...) {
size_t len = vsnprintf_P(temp, sizeof(temp), format, arg);
va_end(arg);
if (len > sizeof(temp) - 1) {
buffer = new char[len + 1];
buffer = new (std::nothrow) char[len + 1];
if (!buffer) {
return 0;
}
Expand Down
36 changes: 31 additions & 5 deletions cores/esp8266/abi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,33 @@ extern "C" void __cxa_pure_virtual(void) __attribute__ ((__noreturn__));
extern "C" void __cxa_deleted_virtual(void) __attribute__ ((__noreturn__));


#if !defined(__cpp_exceptions) && !defined(NEW_OOM_ABORT)
void *operator new(size_t size)
#if !defined(__cpp_exceptions)

// overwrite weak operators new/new[] definitions

void* operator new(size_t size)
{
void *ret = malloc(size);
if (0 != size && 0 == ret) {
umm_last_fail_alloc_addr = __builtin_return_address(0);
umm_last_fail_alloc_size = size;
__unhandled_exception(PSTR("OOM"));
}
return ret;
}

void* operator new[](size_t size)
{
void *ret = malloc(size);
if (0 != size && 0 == ret) {
umm_last_fail_alloc_addr = __builtin_return_address(0);
umm_last_fail_alloc_size = size;
__unhandled_exception(PSTR("OOM"));
}
return ret;
return ret;
}

void *operator new[](size_t size)
void* operator new (size_t size, const std::nothrow_t&)
{
void *ret = malloc(size);
if (0 != size && 0 == ret) {
Expand All @@ -52,7 +67,18 @@ void *operator new[](size_t size)
}
return ret;
}
#endif // arduino's std::new legacy

void* operator new[] (size_t size, const std::nothrow_t&)
{
void *ret = malloc(size);
if (0 != size && 0 == ret) {
umm_last_fail_alloc_addr = __builtin_return_address(0);
umm_last_fail_alloc_size = size;
}
return ret;
}

#endif // !defined(__cpp_exceptions)

void __cxa_pure_virtual(void)
{
Expand Down
3 changes: 2 additions & 1 deletion cores/esp8266/cbuf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/

#include <new> // std::nothrow
#include "cbuf.h"
#include "c_types.h"

Expand All @@ -43,7 +44,7 @@ size_t cbuf::resize(size_t newSize) {
return _size;
}

char *newbuf = new char[newSize];
char *newbuf = new (std::nothrow) char[newSize];
char *oldbuf = _buf;

if(!newbuf) {
Expand Down
29 changes: 0 additions & 29 deletions cores/esp8266/core_esp8266_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,35 +36,6 @@
#include <stddef.h> // size_t
#include <stdint.h>

#ifdef __cplusplus

namespace arduino
{
extern "C++"
template <typename T, typename ...TConstructorArgs>
T* new0 (size_t n, TConstructorArgs... TconstructorArgs)
{
// n==0: single allocation, otherwise it is an array
size_t offset = n? sizeof(size_t): 0;
size_t arraysize = n? n: 1;
T* ptr = (T*)malloc(offset + (arraysize * sizeof(T)));
if (ptr)
{
if (n)
*(size_t*)(ptr) = n;
for (size_t i = 0; i < arraysize; i++)
new (ptr + offset + i * sizeof(T)) T(TconstructorArgs...);
return ptr + offset;
}
return nullptr;
}
}

#define arduino_new(Type, ...) arduino::new0<Type>(0, ##__VA_ARGS__)
#define arduino_newarray(Type, n, ...) arduino::new0<Type>(n, ##__VA_ARGS__)

#endif // __cplusplus

#ifndef __STRINGIFY
#define __STRINGIFY(a) #a
#endif
Expand Down
6 changes: 6 additions & 0 deletions cores/esp8266/core_esp8266_postmortem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,12 @@ void __wrap_system_restart_local() {

cut_here();

if (s_unhandled_exception && umm_last_fail_alloc_addr) {
// now outside from the "cut-here" zone, print correctly the `new` caller address,
// idf-monitor.py will be able to decode this one and show exact location in sources
ets_printf_P(PSTR("\nlast failed alloc caller: 0x%08x\n"), (uint32_t)umm_last_fail_alloc_addr);
}

custom_crash_callback( &rst_info, sp_dump + offset, stack_end );

ets_delay_us(10000);
Expand Down
1 change: 1 addition & 0 deletions cores/esp8266/debug.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ void hexdump(const void *mem, uint32_t len, uint8_t cols);
extern "C" {
#endif

void __unhandled_exception(const char *str) __attribute__((noreturn));
void __panic_func(const char* file, int line, const char* func) __attribute__((noreturn));
#define panic() __panic_func(PSTR(__FILE__), __LINE__, __func__)

Expand Down
33 changes: 0 additions & 33 deletions doc/reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -323,36 +323,3 @@ C++
This assures correct behavior, including handling of all subobjects, which guarantees stability.

History: `#6269 <https://github.com/esp8266/Arduino/issues/6269>`__ `#6309 <https://github.com/esp8266/Arduino/pull/6309>`__ `#6312 <https://github.com/esp8266/Arduino/pull/6312>`__

- New optional allocator ``arduino_new``

A new optional global allocator is introduced with a different semantic:

- never throws exceptions on oom

- never calls constructors on oom

- returns nullptr on oom

It is similar to arduino ``new`` semantic without side effects
(except when parent constructors, or member constructors use ``new``).

Syntax is slightly different, the following shows the different usages:

.. code:: cpp

// with new:

SomeClass* sc = new SomeClass(arg1, arg2, ...);
delete sc;

SomeClass* scs = new SomeClass[42];
delete [] scs;

// with arduino_new:

SomeClass* sc = arduino_new(SomeClass, arg1, arg2, ...);
delete sc;

SomeClass* scs = arduino_newarray(SomeClass, 42);
delete [] scs;
3 changes: 1 addition & 2 deletions libraries/DNSServer/src/DNSServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,7 @@ void DNSServer::processNextRequest()
return;

std::unique_ptr<uint8_t[]> buffer(new (std::nothrow) uint8_t[currentPacketSize]);

if (buffer == NULL)
if (buffer == nullptr)
return;

_udp.read(buffer.get(), currentPacketSize);
Expand Down
2 changes: 1 addition & 1 deletion libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ bool HTTPClient::begin(String url, const uint8_t httpsFingerprint[20])
if (!beginInternal(url, "https")) {
return false;
}
_transportTraits = TransportTraitsPtr(new BearSSLTraits(httpsFingerprint));
_transportTraits = TransportTraitsPtr(new (std::nothrow) BearSSLTraits(httpsFingerprint));
if(!_transportTraits) {
DEBUG_HTTPCLIENT("[HTTP-Client][begin] could not create transport traits\n");
return false;
Expand Down
4 changes: 2 additions & 2 deletions libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,12 @@ bool ESP8266WebServerTemplate<ServerType>::authenticate(const char * username, c
authReq = authReq.substring(6);
authReq.trim();
char toencodeLen = strlen(username)+strlen(password)+1;
char *toencode = new char[toencodeLen + 1];
char *toencode = new (std::nothrow) char[toencodeLen + 1];
if(toencode == NULL){
authReq = "";
return false;
}
char *encoded = new char[base64_encode_expected_len(toencodeLen)+1];
char *encoded = new (std::nothrow) char[base64_encode_expected_len(toencodeLen)+1];
if(encoded == NULL){
authReq = "";
delete[] toencode;
Expand Down
10 changes: 7 additions & 3 deletions libraries/ESP8266WiFi/src/CertStoreBearSSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,13 @@ CertStore::CertInfo CertStore::_preprocessCert(uint32_t length, uint32_t offset,
memset(&ci, 0, sizeof(ci));

// Process it using SHA256, same as the hashed_dn
br_x509_decoder_context *ctx = new br_x509_decoder_context;
br_sha256_context *sha256 = new br_sha256_context;
br_x509_decoder_context *ctx = new (std::nothrow) br_x509_decoder_context;
br_sha256_context *sha256 = new (std::nothrow) br_sha256_context;
if (!ctx || !sha256) {
if (ctx)
delete ctx;
if (sha256)
delete sha256;
Copy link
Collaborator

Choose a reason for hiding this comment

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

is if (...) redundant here? afaik, delete will be no-op with nullptr

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't know which one is nullptr

Copy link
Collaborator

Choose a reason for hiding this comment

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

meaning, we can delete both. one nullptr, another created (or both nullptr)
i.e. https://godbolt.org/z/63PKa3

DEBUG_BSSL("CertStore::_preprocessCert: OOM\n");
return ci;
}
Expand Down Expand Up @@ -202,7 +206,7 @@ const br_x509_trust_anchor *CertStore::findHashedTA(void *ctx, void *hashed_dn,
return nullptr;
}
data.close();
cs->_x509 = new X509List(der, ci.length);
cs->_x509 = new (std::nothrow) X509List(der, ci.length);
free(der);
if (!cs->_x509) {
DEBUG_BSSL("CertStore::findHashedTA: OOM\n");
Expand Down
16 changes: 8 additions & 8 deletions libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ extern "C" {
// Set custom list of ciphers
bool WiFiClientSecure::setCiphers(const uint16_t *cipherAry, int cipherCount) {
_cipher_list = nullptr;
_cipher_list = std::shared_ptr<uint16_t>(new uint16_t[cipherCount], std::default_delete<uint16_t[]>());
_cipher_list = std::shared_ptr<uint16_t>(new (std::nothrow) uint16_t[cipherCount], std::default_delete<uint16_t[]>());
if (!_cipher_list.get()) {
DEBUG_BSSL("setCiphers: list empty\n");
return false;
Expand Down Expand Up @@ -1067,8 +1067,8 @@ bool WiFiClientSecure::_connectSSL(const char* hostName) {

_sc = std::make_shared<br_ssl_client_context>();
_eng = &_sc->eng; // Allocation/deallocation taken care of by the _sc shared_ptr
_iobuf_in = std::shared_ptr<unsigned char>(new unsigned char[_iobuf_in_size], std::default_delete<unsigned char[]>());
_iobuf_out = std::shared_ptr<unsigned char>(new unsigned char[_iobuf_out_size], std::default_delete<unsigned char[]>());
_iobuf_in = std::shared_ptr<unsigned char>(new (std::nothrow) unsigned char[_iobuf_in_size], std::default_delete<unsigned char[]>());
_iobuf_out = std::shared_ptr<unsigned char>(new (std::nothrow) unsigned char[_iobuf_out_size], std::default_delete<unsigned char[]>());

if (!_sc || !_iobuf_in || !_iobuf_out) {
_freeSSL(); // Frees _sc, _iobuf*
Expand Down Expand Up @@ -1183,8 +1183,8 @@ bool WiFiClientSecure::_connectSSLServerRSA(const X509List *chain,
_oom_err = false;
_sc_svr = std::make_shared<br_ssl_server_context>();
_eng = &_sc_svr->eng; // Allocation/deallocation taken care of by the _sc shared_ptr
_iobuf_in = std::shared_ptr<unsigned char>(new unsigned char[_iobuf_in_size], std::default_delete<unsigned char[]>());
_iobuf_out = std::shared_ptr<unsigned char>(new unsigned char[_iobuf_out_size], std::default_delete<unsigned char[]>());
_iobuf_in = std::shared_ptr<unsigned char>(new (std::nothrow) unsigned char[_iobuf_in_size], std::default_delete<unsigned char[]>());
_iobuf_out = std::shared_ptr<unsigned char>(new (std::nothrow) unsigned char[_iobuf_out_size], std::default_delete<unsigned char[]>());

if (!_sc_svr || !_iobuf_in || !_iobuf_out) {
_freeSSL();
Expand Down Expand Up @@ -1220,8 +1220,8 @@ bool WiFiClientSecure::_connectSSLServerEC(const X509List *chain,
_oom_err = false;
_sc_svr = std::make_shared<br_ssl_server_context>();
_eng = &_sc_svr->eng; // Allocation/deallocation taken care of by the _sc shared_ptr
_iobuf_in = std::shared_ptr<unsigned char>(new unsigned char[_iobuf_in_size], std::default_delete<unsigned char[]>());
_iobuf_out = std::shared_ptr<unsigned char>(new unsigned char[_iobuf_out_size], std::default_delete<unsigned char[]>());
_iobuf_in = std::shared_ptr<unsigned char>(new (std::nothrow) unsigned char[_iobuf_in_size], std::default_delete<unsigned char[]>());
_iobuf_out = std::shared_ptr<unsigned char>(new (std::nothrow) unsigned char[_iobuf_out_size], std::default_delete<unsigned char[]>());

if (!_sc_svr || !_iobuf_in || !_iobuf_out) {
_freeSSL();
Expand Down Expand Up @@ -1421,7 +1421,7 @@ bool WiFiClientSecure::probeMaxFragmentLength(IPAddress ip, uint16_t port, uint1
default: return false; // Invalid size
}
int ttlLen = sizeof(clientHelloHead_P) + (2 + sizeof(suites_P)) + (sizeof(clientHelloTail_P) + 1);
uint8_t *clientHello = new uint8_t[ttlLen];
uint8_t *clientHello = new (std::nothrow) uint8_t[ttlLen];
if (!clientHello) {
DEBUG_BSSL("probeMaxFragmentLength: OOM\n");
return false;
Expand Down
Loading