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

Convert operator new and delete to use CHIPMem.h #3260

Merged
merged 2 commits into from
Oct 16, 2020

Conversation

kpschoedel
Copy link
Contributor

Problem

Embedded device-side code should only use heap through CHIPMem.h.
In particular it should not use the C++ new and delete operators for
heap allocation without explicit placement using Platform::MemoryAlloc().

Summary of Changes

  • Added Platform::New<>() and Delete<>().
  • Convert uses of operator new and delete.

Fixes #3232 Remove explicit use of new from first-party embedded-device code

#### Problem

Embedded device-side code should only use heap through CHIPMem.h.
In particular it should not use the C++ new and delete operators for
heap allocation without explicit placement using Platform::MemoryAlloc().

#### Summary of Changes

- Added Platform::New<>() and Delete<>().
- Convert uses of operator new and delete.

Fixes project-chip#3232 Remove explicit use of `new` from first-party embedded-device code
@github-actions
Copy link

Size increase report for "esp32-example-build"

File Section File VM
chip-wifi-echo.elf .flash.text 64 64
chip-wifi-echo.elf .flash.rodata 8 8
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-wifi-echo.elf and ./pull_artifact/chip-wifi-echo.elf:

sections,vmsize,filesize
.debug_str,0,2922
.debug_line,0,1467
.debug_abbrev,0,913
.debug_loc,0,687
.debug_ranges,0,480
.strtab,0,77
.xt.prop._ZNSt8_Rb_treeIhSt4pairIKhN4chip18OptionalQRCodeInfoEESt10_Select1stIS4_ESt4lessIhESaIS4_EE14_M_create_nodeIJRKS4_EEEPSt13_Rb_tree_nodeIS4_EDpOT_$isra$255,0,76
.xt.prop._ZNSt8_Rb_treeIhSt4pairIKhN4chip27OptionalQRCodeInfoExtensionEESt10_Select1stIS4_ESt4lessIhESaIS4_EE13_M_clone_nodeINSA_11_Alloc_nodeEEEPSt13_Rb_tree_nodeIS4_EPKSE_RT_$isra$264,0,76
.flash.text,64,64
.symtab,0,32
.flash.rodata,8,8
[Unmapped],0,-11
.xt.prop._ZN4chip16ReferenceCountedINS_28SecurePairingSessionDelegateEE7ReleaseEv,0,-12
.xt.prop._ZN4chip16ReferenceCountedINS_9Transport4BaseEE7ReleaseEv,0,-12
.xt.prop._ZNSt8_Rb_treeIhSt4pairIKhN4chip18OptionalQRCodeInfoEESt10_Select1stIS4_ESt4lessIhESaIS4_EE14_M_create_nodeIJRKS4_EEEPSt13_Rb_tree_nodeIS4_EDpOT_$isra$244,0,-76
.xt.prop._ZNSt8_Rb_treeIhSt4pairIKhN4chip27OptionalQRCodeInfoExtensionEESt10_Select1stIS4_ESt4lessIhESaIS4_EE13_M_clone_nodeINSA_11_Alloc_nodeEEEPSt13_Rb_tree_nodeIS4_EPKSE_RT_$isra$253,0,-76
.debug_info,0,-307


@github-actions
Copy link

Size increase report for "nrfconnect-example-build"

File Section File VM
chip-lock.elf text 48 48
chip-lighting.elf text 48 48
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_info,0,7479
.debug_abbrev,0,1000
.debug_str,0,842
.debug_loc,0,472
.debug_line,0,413
.debug_ranges,0,160
text,48,48
[Unmapped],0,6
.debug_frame,0,4

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_info,0,4415
.debug_abbrev,0,950
.debug_line,0,199
[Unmapped],0,-4

Comparing ./master_artifact/chip-lighting.elf and ./pull_artifact/chip-lighting.elf:

sections,vmsize,filesize
.debug_info,0,7479
.debug_abbrev,0,1000
.debug_str,0,842
.debug_loc,0,472
.debug_line,0,413
.debug_ranges,0,160
text,48,48
.debug_frame,0,4
[Unmapped],0,-2


Comment on lines +156 to +157
* In a few cases it may be necessary to add explicit casts to arguments, notably
* when passing integer constants to smaller integer parameters.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Incidentally, the only reason this is not also true of things like std::make_unique is that gcc magically suppresses warnings from system headers — which also means that -Wconversion doesn't fire on code like this:

#include <memory>
struct Fewbits { Fewbits(short x) : m(x){}; short m; };
std::unique_ptr<Fewbits> f(long long manybits) { return std::make_unique<Fewbits>(manybits); }

@kpschoedel kpschoedel marked this pull request as draft October 15, 2020 20:43
@kpschoedel
Copy link
Contributor Author

With #3274 I can run locally and confirm the Cirque failure is genuine. Moving to draft state until fixed.

@kpschoedel kpschoedel marked this pull request as ready for review October 16, 2020 18:12
@rwalker-apple rwalker-apple merged commit f7c23fc into project-chip:master Oct 16, 2020
@kpschoedel kpschoedel deleted the x3232-mem-new branch October 23, 2020 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove explicit use of new from first-party embedded-device code
4 participants