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

Implement basic rand() #6998

Merged
merged 3 commits into from
May 21, 2021
Merged

Implement basic rand() #6998

merged 3 commits into from
May 21, 2021

Conversation

srickardti
Copy link
Contributor

@srickardti srickardti commented May 20, 2021

Disabled shadowing of the FreeRTOS config by the DMM header.
Implemented a basic PRNG for ctor startup.

The global initialization of the global message counter struct in
src/transport/MessageCounter.h results in a call to the C standard library
function rand() from a C++ ctor. This results in a call to pvPortMalloc,
causing a hard fault with the current buffer implementation.

Problem

rand() is called before main() so the calls to allocate a reent struct fail.

Summary of Changes

Added a basic non-reent PRNG for the TI platform.

Fixes #6997

Testing

Manually tested with an LP_CC2652R7. Execution was traced from the ResetISR to main with CCS and the device was left to free run. log output was viewed on the debug UART connection and the processor was halted in the power idle loop.

Disabled shadowing of the FreeRTOS config by the DMM header.
Implemented a basic PRNG for ctor startup.

The global initialization of the global message counter struct in
`src/transport/MessageCounter.h` results in a call to the C standard library
function `rand()` from a C++ ctor. This results in a call to pvPortMalloc,
causing a hard fault with the current buffer implementation.
@github-actions
Copy link

Size increase report for "esp32-example-build" from fb7cd0f

File Section File VM
chip-all-clusters-app.elf .flash.text 24 24
chip-all-clusters-app.elf .flash.rodata -4 -4
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.xt.lit._ZN12QRCodeScreenD5Ev,0,88
.xt.prop._ZN12QRCodeScreenD5Ev,0,52
.xt.prop._ZN4chip9Transport16AdminPairingInfo5ResetEv,0,40
.flash.text,24,24
[Unmapped],0,4
.flash.rodata,-4,-4
.xt.prop._ZN4chip8Platform3NewINS_9Transport16AdminPairingInfo24StorableAdminPairingInfoEJEEEPT_DpOT0_,0,-12
.xt.prop._ZNSt6vectorIhSaIhEE17_M_default_appendEj,0,-40
.xt.lit._ZN4chip8Platform3NewINS_9Transport16AdminPairingInfo24StorableAdminPairingInfoEJEEEPT_DpOT0_,0,-48
.xt.lit._ZNSt6vectorIhSaIhEE17_M_default_appendEj,0,-80

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize


Copy link
Contributor

@woody-apple woody-apple left a comment

Choose a reason for hiding this comment

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

Thanks for the change! We have updated our PR template, requesting changes to conform to the PR template:

#### Problem
What is being fixed?  Examples:
* Fix crash on startup
* Fixes #12345 Frobnozzle is leaky (exactly like that, so GitHub will auto-close the issue).

#### Change overview
What's in this PR

#### Testing
How was this tested? (at least one bullet point required)
* If unit tests were added, how do they cover this issue?
* If unit tests existed, how were they fixed/modified to prevent this in future?
* If new unit tests are not added, why not?
* If integration tests were added, how do they verify this change?
* If new integration tests are not added, why not?
* If manually tested, what platforms controller and device platforms were manually tested, and how?
* If no testing is required, why not?

@srickardti
Copy link
Contributor Author

Thanks for the change! We have updated our PR template, requesting changes to conform to the PR template:

Apologies, I must have gotten it in before the template was update. I've included the testing information now.

Copy link
Contributor

@woody-apple woody-apple left a comment

Choose a reason for hiding this comment

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

No apologies needed, the template is new :) Thanks!

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
@woody-apple woody-apple merged commit 82a8544 into project-chip:master May 21, 2021
@srickardti srickardti deleted the main branch June 4, 2021 01:53
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* Implement basic rand()

Disabled shadowing of the FreeRTOS config by the DMM header.
Implemented a basic PRNG for ctor startup.

The global initialization of the global message counter struct in
`src/transport/MessageCounter.h` results in a call to the C standard library
function `rand()` from a C++ ctor. This results in a call to pvPortMalloc,
causing a hard fault with the current buffer implementation.

* Restyled by whitespace

* Update src/platform/cc13x2_26x2/Random.c

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Justin Wood <woody@apple.com>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
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.

Stdlib rand() in global ctor causes hard fault
6 participants