Skip to content

Commit

Permalink
new w/ OOM now aborts by defaults, or throw an exception (esp8266#7536)
Browse files Browse the repository at this point in the history
* replace `new` by `new (std::nothrow)`, remove `arduino_new`

* deleted:    arduino_new/arduino_new.ino

* fixes

* remove (std::nothrow) where nullptr case is not handled
remove legacy new management

* new w/ OOM raises an exception, shows the caller address for decoders

* overwrite weak `new (std::nothrow)` calls

* fix displaying caller address

* board generator: remove comments, remove now useless define

Co-authored-by: Earle F. Philhower, III <[email protected]>
Co-authored-by: Develo <[email protected]>
  • Loading branch information
devyte and earlephilhower authored Aug 31, 2020
2 parents be812d2 + ef92ab2 commit 5b767a3
Show file tree
Hide file tree
Showing 16 changed files with 134 additions and 423 deletions.
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;
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

0 comments on commit 5b767a3

Please sign in to comment.