Disable C++ static destructors in ARMC6 compiler #11726

kjbracey · 2019-10-22T11:54:55Z

Description

We disable C++ static destructors as best as possible in the various toolchains, trying to eliminate unwanted destructor code. We don't want to waste RAM or ROM on the C++-standard-specified behaviour of running all statically-constructed objects' destructors in reverse order on exit; we're never going to perform an orderly exit.

Techniques used include:

SingletonPtr - makes an object be "lazily constructed" on first use, and also eliminates its destructor. Lazy construction adds ROM+RAM overhead.
__eabi_atexit is stubbed out, preventing RAM usage by run-time registration of static destructors.
GCC has exit wrapped to kill shutdown code
IAR has static destructors disabled in the compiler flags

Killing static destructors in the compiler is the optimum; if we only stub out __eabi_atexit, the compiler is still inserting calls to it, and referencing the destructors in those call.

Clang 8 added the compiler option -fno-c++-static-destructors (and the object attributes [[clang::no_destroy]] and [[clang::always_destroy]] for fine control).

We can hence enable that option in ARMC6 tool profiles, matching IAR.

This option appears to exist in ARM Compiler 6.11, but generates an apparently spurious linker error about EthernetInterface. It works in ARM Compiler 6.13, so this PR needs to wait until the compiler is updated.

Pull request type

[ ] Fix
[X] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@pan-, @bulislaw, @0xc0170

Static destruction code may still end up in a GCC image despite our efforts. Use of `SingletonPtr` will eliminate most, but references can still occur. For IAR and ARM toolchains they can be totally eliminated in the compiler (see ARMmbed#11726 for ARM). As the Clang attributes have been submitted for C++ standardisation (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1247r0.html), it seems likely that one day GCC will also support this, so it seems not worthwhile to create a `NotDestroyed<T>` template wrapper as a workaround, as I was originally intending. In the meantime, GCC is inserting calls to `__eabi_atexit`. That is stubbed out, so whatever it attempts to register will not be called. We can't stop it putting the calls in, but we can transparently slightly reduce code size by telling it to use `atexit` instead. It needs to pass fewer parameters for us to ignore, so the callsites become a little smaller.

40Grit · 2019-10-22T12:11:46Z

This is just for static destructors correct?

At some point it might be nice to have the peripheral API destructors safely disable and release resources associated with the peripherals.

kjbracey · 2019-10-22T12:29:24Z

This is just for static destructors correct?

Yes. We currently never actually call static destructors anyway, this just eliminates them earlier and harder. I should clarify that in #11727 too - that's actually "optimising GCC static non-destruction".

At some point it might be nice to have the peripheral API destructors safely disable and release resources associated with the peripherals.

Indeed, but that wouldn't happen if those were declared globally. There's no system shutdown.

You could make it happen dynamically by using new and delete, or we should be looking at C++17's std::variant, to achieve that without dynamic allocation:

// type-safe "union" object that can hold a monostate or a UARTSerial
// "monostate" is a dummy do-nothing object, and it's initialised holding that
std::variant<std::monostate, mbed::UARTSerial> serial_store;

int main() {
     // initialise serial (replacing the monostate)
     mbed::UARTSerial &serial = serial_store.emplace<mbed::UARTSerial>(GPS_RX, GPS_TX, 115200);
     // use serial
     run_gps(serial);
     // deinitialise serial (by replacing it with the monostate)
     serial_store.emplace<std::monostate>();
}

That concept then extends to multiple modes...

kjbracey · 2019-10-22T12:36:59Z

Obviously, more simply, this would work, but only for stuff small enough to fit on the stack, and assuming you're not doing something event driven.

Would make sense though if you had a reserved thread, so the storage would effectively be that thread's stack:

void gps_thread() {
     for (;;) {
        uint32_t = event_flags.wait_for_any(0x3);
        if (flag & 2) {
            break;
        }
        if (flag & 1) {
            // initialise serial (on the stack)
            UARTSerial serial(GPS_RX, GPS_TX, 115200);
            // use serial
            run_gps(serial);
            // deinitialise serial (by making it leave scope)
        }
    }
}

ciarmcom · 2019-10-22T13:00:26Z

@kjbracey-arm, thank you for your changes.
@bulislaw @0xc0170 @pan- @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

kjbracey · 2019-10-22T14:20:14Z

Saving for this in a cloud client example was

Total Static RAM memory (data + bss): 251394(+0) bytes
Total Flash memory (text + data): 404002(-882) bytes

So −0.2% on ROM.

40Grit · 2019-10-22T15:57:33Z

@kjbracey-arm
Ah! I woke up properly and realized this is for destructors of static objects, which I cannot think of a relevant use for in Mbed OS without stretching my imagination and understanding of C++.

I was thinking more from a standpoint of using the destructors as a standard way to clean up peripheral states and buffers when no longer using them.

The various considerations of entering sleep, resetting, low power, and very direct control of hardware resources whose memory cannot be "freed" lead me to the conclusion that it would probably better to manage peripheral control via dedicated member functions?

kjbracey · 2019-10-23T07:20:51Z

I was thinking more from a standpoint of using the destructors as a standard way to clean up peripheral states and buffers when no longer using them.

That should work. And can work, if you're in a position to destroy the object. I think std::variant as shown above is a powerful tool we could make use of. I can envisage a system with std::variant<std::monostate, mbed::ESP8266Wifi, mbed::BG96CellularModem> that does modal switches between the two network interfaces, sharing static memory.

it would probably better to manage peripheral control via dedicated member functions?

We're also adding suspend/resume calls to a number of objects/systems that can shut down hardware while retaining software state. That's a common use case.

mark-edgeworth

Looks ok to me

kjbracey · 2019-10-23T11:49:40Z

Just occurred to me that the option should really be under "cxx", not "common". Moved it - will slightly reduce log size to not pass it to C files.

pan-

Looks great. If I understand correctly a variable with the attribute [clang::always_destroy]] will continue to register the destructor in atexit ?

0xc0170 · 2019-10-23T12:54:54Z

CI started

kjbracey · 2019-10-23T12:55:38Z

If I understand correctly a variable with the attribute [clang::always_destroy]] will continue to register the destructor in atexit ?

Yes, it will continue to register it with __eabi_atexit, but that will still do nothing (apart from pull code into the image), because it's always been replaced with a dummy implementation.

kjbracey · 2019-10-23T12:59:42Z

CI started

Ah, as per the note at the bottom of the description, I think this will fail until the compiler is updated. I was getting link errors with 6.11. You'll probably need to stick a "needs: preceding PR" or something on. But let's check what happens.

mbed-ci · 2019-10-23T13:30:13Z

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM

kjbracey · 2019-10-30T09:49:27Z

I believe CI is now on ARM Compiler 6.13, so this should pass now.

kjbracey · 2020-02-17T10:15:09Z

Bunch of spurious failures?

0:56:10  ERROR: Issue with creating launcher for agent EC2 (ec2-eu-west-1) - ec2-linux-allinone-builds (i-057407ba2591e38b9). The agent has not been fully initialized yet

0xc0170 · 2020-02-27T12:09:28Z

CI restarted

0xc0170 · 2020-02-28T07:53:00Z

Exporters contain failures for ARMC6, related. I'll check what is the current version, although recently the exporters were disabled... so this might pass if restarted

We disable C++ static destructors as best as possible in the various toolchains, trying to eliminate unwanted destructor code. We don't want to waste RAM or ROM on the C++-standard-specified behaviour of running all statically-constructed objects' destructors in reverse order on exit; we're never going to perform an orderly exit. Techniques used include: * `SingletonPtr` - makes an object be "lazily constructed" on first use, and also eliminates its destructor. Lazy construction adds ROM+RAM overhead. * `__eabi_atexit` is stubbed out, preventing RAM usage by run-time registration of static destructors. * GCC has `exit` wrapped to kill shutdown code * IAR has static destructors disabled in the compiler flags Killing static destructors in the compiler is the optimum; if we only stub out `__eabi_atexit`, the compiler is still inserting calls to it, and referencing the destructors in those call. Clang 8 added the compiler option `-fno-c++-static-destructors` (and the object attributes `[[clang::no_destroy]]` and `[[clang::always_destroy]]` for fine control). We can hence enable that option in ARMC6 tool profiles, matching IAR. This option appears to exist in ARM Compiler 6.11, but generates an apparently spurious linker error about `EthernetInterface`. It works in ARM Compiler 6.13, so this PR needs to wait until the compiler is updated.

0xc0170 · 2020-03-24T15:32:14Z

I was able to rebase this, will restart tests now

Pull request has been modified.

0xc0170 · 2020-03-24T15:47:02Z

@mergify mergify bot dismissed stale reviews from mark-edgeworth, 0xc0170, and bulislaw 13 minutes ago

Just a rebase, all approved previosly

mergify · 2020-03-24T15:47:35Z

Sorry but I didn't understand the command.

Hey, I reacted but my real name is @Mergifyio

mark-edgeworth

Looks ok to me

mbed-ci · 2020-03-24T20:11:47Z

Test run: FAILED

Summary: 1 of 6 test jobs failed
Build number : 5
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2020-03-25T13:17:03Z

Test restarted

mergify · 2020-03-27T14:14:36Z

This PR does not contain release version label after merging.

kjbracey mentioned this pull request Oct 22, 2019

Slightly optimise GCC static destruction #11727

Closed

ciarmcom requested review from 0xc0170, bulislaw, pan- and a team October 22, 2019 13:00

ciarmcom added the needs: review label Oct 22, 2019

kjbracey force-pushed the armc6_no_static_destructors branch from 92886d7 to f3417f2 Compare October 23, 2019 07:14

kjbracey changed the title ~~Disable C++ static constructors in ARMC6~~ Disable C++ static destructors in ARMC6 compiler Oct 23, 2019

mark-edgeworth previously approved these changes Oct 23, 2019

View reviewed changes

0xc0170 previously approved these changes Oct 23, 2019

View reviewed changes

0xc0170 added the release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 label Oct 23, 2019

kjbracey force-pushed the armc6_no_static_destructors branch from f3417f2 to 2f1eedd Compare October 23, 2019 11:48

pan- approved these changes Oct 23, 2019

View reviewed changes

0xc0170 added the needs: preceding PR label Oct 23, 2019

kjbracey removed the needs: preceding PR label Oct 30, 2019

0xc0170 added the needs: CI label Feb 27, 2020

mergify bot added ready for merge and removed needs: CI labels Feb 27, 2020

0xc0170 removed the ready for merge label Feb 28, 2020

0xc0170 force-pushed the armc6_no_static_destructors branch from 2f1eedd to 1ecd960 Compare March 24, 2020 15:31

0xc0170 added needs: CI and removed needs: preceding PR labels Mar 24, 2020

mark-edgeworth approved these changes Mar 24, 2020

View reviewed changes

mergify bot added needs: work and removed needs: CI labels Mar 24, 2020

adbridge added ready for merge and removed needs: work labels Mar 27, 2020

adbridge merged commit 8392613 into ARMmbed:master Mar 27, 2020

adbridge added release-version: 6.0.0-beta-1 and removed ready for merge labels Mar 27, 2020

mergify bot added the release version missing When PR does not contain release version, bot should label it and we fix it afterwards label Mar 27, 2020

0xc0170 removed the release version missing When PR does not contain release version, bot should label it and we fix it afterwards label Jun 22, 2021

Disable C++ static destructors in ARMC6 compiler #11726

Disable C++ static destructors in ARMC6 compiler #11726

Uh oh!

Conversation

kjbracey commented Oct 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Reviewers

Uh oh!

40Grit commented Oct 22, 2019

Uh oh!

kjbracey commented Oct 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kjbracey commented Oct 22, 2019

Uh oh!

ciarmcom commented Oct 22, 2019

Uh oh!

kjbracey commented Oct 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

40Grit commented Oct 22, 2019

Uh oh!

kjbracey commented Oct 23, 2019

Uh oh!

mark-edgeworth left a comment

Choose a reason for hiding this comment

Uh oh!

kjbracey commented Oct 23, 2019

Uh oh!

pan- left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Oct 23, 2019

Uh oh!

kjbracey commented Oct 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kjbracey commented Oct 23, 2019

Uh oh!

mbed-ci commented Oct 23, 2019

Test run: FAILED

Uh oh!

kjbracey commented Oct 30, 2019

Uh oh!

kjbracey commented Feb 17, 2020

Uh oh!

0xc0170 commented Feb 27, 2020

Uh oh!

0xc0170 commented Feb 28, 2020

Uh oh!

0xc0170 commented Mar 24, 2020

Uh oh!

0xc0170 commented Mar 24, 2020

Uh oh!

mergify bot commented Mar 24, 2020

Uh oh!

mark-edgeworth left a comment

Choose a reason for hiding this comment

Uh oh!

mbed-ci commented Mar 24, 2020

Test run: FAILED

Uh oh!

0xc0170 commented Mar 25, 2020

Uh oh!

mergify bot commented Mar 27, 2020

Uh oh!

Uh oh!

kjbracey commented Oct 22, 2019 •

edited

Loading

kjbracey commented Oct 22, 2019 •

edited

Loading

kjbracey commented Oct 22, 2019 •

edited

Loading

kjbracey commented Oct 23, 2019 •

edited

Loading