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

Check for too-large events before we modify our buffer state. #25257

Merged
merged 1 commit into from
Feb 23, 2023

Conversation

bzbarsky-apple
Copy link
Contributor

We could end up in a situation where an INFO event that fit into the DEBUG buffer but did not fit into the INFO buffer would get written to the buffer, and possibly evict some other events, but then we would error out from LogEventPrivate, and leave ourselves in an inconsistent state.

The fix is to do the "this event fits in all the buffers that it might need to fit in" check before we start modifying any state.

We could end up in a situation where an INFO event that fit into the DEBUG
buffer but did not fit into the INFO buffer would get written to the buffer, and
possibly evict some other events, but then we would error out from
LogEventPrivate, and leave ourselves in an inconsistent state.

The fix is to do the "this event fits in all the buffers that it might need to
fit in" check before we start modifying any state.
@github-actions
Copy link

PR #25257: Size comparison from c6cadc8 to c06519d

Increases (8 builds for bl702, cc13x2_26x2, cc32xx)
platform target config section c6cadc8 c06519d change % change
bl702 lighting-app bl702 .debug_frame 492132 492140 8 0.0
.debug_ranges 371928 371952 24 0.0
.debug_str 3574444 3574478 34 0.0
.strtab 574062 574082 20 0.0
bl702+rpc .debug_frame 519896 519904 8 0.0
.debug_ranges 395704 395728 24 0.0
.debug_str 3977981 3978015 34 0.0
.strtab 635126 635146 20 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read/write) 170752 170952 200 0.1
lock-ftd LP_CC2652R7 (read/write) 171960 172144 184 0.1
lock-mtd LP_CC2652R7 (read/write) 180580 180772 192 0.1
pump-app LP_CC2652R7 (read/write) 159460 159612 152 0.1
.text 598128 598136 8 0.0
pump-controller-app LP_CC2652R7 (read/write) 174668 174836 168 0.1
cc32xx lock CC3235SF_LAUNCHXL .debug_frame 299752 299756 4 0.0
.debug_loc 2795342 2795399 57 0.0
.debug_ranges 281592 281616 24 0.0
.debug_str 3017877 3017911 34 0.0
.strtab 377630 377650 20 0.0
Decreases (14 builds for bl602, bl702, cc13x2_26x2, cc32xx, linux, qpg)
platform target config section c6cadc8 c06519d change % change
bl602 lighting-app bl602 (read/write) 1349554 1349366 -188 -0.0
.text 1025620 1025596 -24 -0.0
bl602+rpc (read/write) 1394994 1394798 -196 -0.0
.text 1056554 1056528 -26 -0.0
bl702 lighting-app bl702 (read/write) 1187907 1187715 -192 -0.0
.debug_info 40594484 40594457 -27 -0.0
.debug_line 5277256 5277176 -80 -0.0
.debug_loc 3415724 3415711 -13 -0.0
.rodata 107904 107744 -160 -0.1
.text 954966 954942 -24 -0.0
bl702+rpc (read/write) 1281039 1280847 -192 -0.0
.debug_info 45006770 45006742 -28 -0.0
.debug_line 5676190 5676110 -80 -0.0
.debug_loc 3612722 3612709 -13 -0.0
.rodata 122544 122384 -160 -0.1
.text 1032376 1032350 -26 -0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 680311 680111 -200 -0.0
.rodata 88487 88327 -160 -0.2
.text 591508 591468 -40 -0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 643511 643327 -184 -0.0
.rodata 78351 78199 -152 -0.2
.text 564840 564808 -32 -0.0
lock-ftd LP_CC2652R7 (read only) 676471 676287 -184 -0.0
.rodata 76679 76527 -152 -0.2
.text 599312 599280 -32 -0.0
lock-mtd LP_CC2652R7 (read only) 663107 662915 -192 -0.0
.rodata 103411 103251 -160 -0.2
.text 559216 559184 -32 -0.0
pump-app LP_CC2652R7 (read only) 689707 689555 -152 -0.0
.rodata 91099 90939 -160 -0.2
pump-controller-app LP_CC2652R7 (read only) 674643 674475 -168 -0.0
.rodata 86971 86811 -160 -0.2
.text 587192 587184 -8 -0.0
cc32xx lock CC3235SF_LAUNCHXL (read only) 642545 642361 -184 -0.0
.debug_info 20254045 20254017 -28 -0.0
.debug_line 2655138 2655094 -44 -0.0
.rodata 105817 105657 -160 -0.2
.text 534604 534580 -24 -0.0
linux thermostat-no-ble arm64 (read only) 2515692 2515388 -304 -0.0
.rodata 150728 150568 -160 -0.1
.text 2102896 2102752 -144 -0.0
qpg lighting-app qpg6105+debug (read/write) 1151372 1151180 -192 -0.0
.text 598468 598276 -192 -0.0
lock-app qpg6105+debug (read/write) 1118348 1118164 -184 -0.0
.text 565448 565264 -184 -0.0
Full report (16 builds for bl602, bl702, cc13x2_26x2, cc32xx, linux, qpg)
platform target config section c6cadc8 c06519d change % change
bl602 lighting-app bl602 (read/write) 1349554 1349366 -188 -0.0
.bss 94666 94666 0 0.0
.data 9752 9752 0 0.0
.text 1025620 1025596 -24 -0.0
bl602+rpc (read/write) 1394994 1394798 -196 -0.0
.bss 102714 102714 0 0.0
.data 10144 10144 0 0.0
.text 1056554 1056528 -26 -0.0
bl702 lighting-app bl702 0 0 0 0.0
(read only) 3358 3358 0 0.0
(read/write) 1187907 1187715 -192 -0.0
.bleromro 6342 6342 0 0.0
.bleromrw 124 124 0 0.0
.boot2 292 292 0 0.0
.bss 69777 69777 0 0.0
.bss_psram 30064 30064 0 0.0
.comment 48 48 0 0.0
.data 4080 4080 0 0.0
.debug_abbrev 1551744 1551744 0 0.0
.debug_aranges 134240 134240 0 0.0
.debug_frame 492132 492140 8 0.0
.debug_info 40594484 40594457 -27 -0.0
.debug_line 5277256 5277176 -80 -0.0
.debug_loc 3415724 3415711 -13 -0.0
.debug_ranges 371928 371952 24 0.0
.debug_str 3574444 3574478 34 0.0
.hbn 536 536 0 0.0
.hbn_noinit 260 260 0 0.0
.init 342 342 0 0.0
.init_array 144 144 0 0.0
.psram 0 0 0 0.0
.riscv.attributes 47 47 0 0.0
.rodata 107904 107744 -160 -0.1
.rsvd 2960 2960 0 0.0
.sha_ocram 72 72 0 0.0
.shstrtab 304 304 0 0.0
.stack 2048 2048 0 0.0
.strtab 574062 574082 20 0.0
.symtab 173600 173600 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3358 3358 0 0.0
.text 954966 954942 -24 -0.0
bl702+rpc 0 0 0 0.0
(read only) 3358 3358 0 0.0
(read/write) 1281039 1280847 -192 -0.0
.bleromro 6342 6342 0 0.0
.bleromrw 124 124 0 0.0
.boot2 292 292 0 0.0
.bss 77825 77825 0 0.0
.bss_psram 30320 30320 0 0.0
.comment 48 48 0 0.0
.data 4624 4624 0 0.0
.debug_abbrev 1699975 1699975 0 0.0
.debug_aranges 142480 142480 0 0.0
.debug_frame 519896 519904 8 0.0
.debug_info 45006770 45006742 -28 -0.0
.debug_line 5676190 5676110 -80 -0.0
.debug_loc 3612722 3612709 -13 -0.0
.debug_ranges 395704 395728 24 0.0
.debug_str 3977981 3978015 34 0.0
.hbn 536 536 0 0.0
.hbn_noinit 260 260 0 0.0
.init 342 342 0 0.0
.init_array 160 160 0 0.0
.psram 0 0 0 0.0
.riscv.attributes 47 47 0 0.0
.rodata 122544 122384 -160 -0.1
.rsvd 2960 2960 0 0.0
.sha_ocram 72 72 0 0.0
.shstrtab 304 304 0 0.0
.stack 2048 2048 0 0.0
.strtab 635126 635146 20 0.0
.symtab 192064 192064 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3358 3358 0 0.0
.text 1032376 1032350 -26 -0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 680311 680111 -200 -0.0
(read/write) 170752 170952 200 0.1
.bss 80756 80756 0 0.0
.data 3352 3352 0 0.0
.rodata 88487 88327 -160 -0.2
.text 591508 591468 -40 -0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 643511 643327 -184 -0.0
(read/write) 157416 157416 0 0.0
.bss 79948 79948 0 0.0
.data 3352 3352 0 0.0
.rodata 78351 78199 -152 -0.2
.text 564840 564808 -32 -0.0
lock-ftd LP_CC2652R7 (read only) 676471 676287 -184 -0.0
(read/write) 171960 172144 184 0.1
.bss 78212 78212 0 0.0
.data 3316 3316 0 0.0
.rodata 76679 76527 -152 -0.2
.text 599312 599280 -32 -0.0
lock-mtd LP_CC2652R7 (read only) 663107 662915 -192 -0.0
(read/write) 180580 180772 192 0.1
.bss 73468 73468 0 0.0
.data 3316 3316 0 0.0
.rodata 103411 103251 -160 -0.2
.text 559216 559184 -32 -0.0
pump-app LP_CC2652R7 (read only) 689707 689555 -152 -0.0
(read/write) 159460 159612 152 0.1
.bss 78180 78180 0 0.0
.data 3280 3280 0 0.0
.rodata 91099 90939 -160 -0.2
.text 598128 598136 8 0.0
pump-controller-app LP_CC2652R7 (read only) 674643 674475 -168 -0.0
(read/write) 174668 174836 168 0.1
.bss 78324 78324 0 0.0
.data 3304 3304 0 0.0
.rodata 86971 86811 -160 -0.2
.text 587192 587184 -8 -0.0
shell LP_CC2652R7 (read only) 671518 671518 0 0.0
(read/write) 181616 181616 0 0.0
.bss 82828 82828 0 0.0
.data 3348 3348 0 0.0
.rodata 85230 85230 0 0.0
.text 585976 585976 0 0.0
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 642545 642361 -184 -0.0
(read/write) 203664 203664 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197064 197064 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 930168 930168 0 0.0
.debug_aranges 87272 87272 0 0.0
.debug_frame 299752 299756 4 0.0
.debug_info 20254045 20254017 -28 -0.0
.debug_line 2655138 2655094 -44 -0.0
.debug_loc 2795342 2795399 57 0.0
.debug_ranges 281592 281616 24 0.0
.debug_str 3017877 3017911 34 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105817 105657 -160 -0.2
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 377630 377650 20 0.0
.symtab 256192 256192 0 0.0
.text 534604 534580 -24 -0.0
linux chip-tool-ipv6only arm64 (read only) 12061684 12061684 0 0.0
(read/write) 729224 729224 0 0.0
.bss 34136 34136 0 0.0
.data 3008 3008 0 0.0
.data.rel.ro 671552 671552 0 0.0
.dynamic 560 560 0 0.0
.got 15328 15328 0 0.0
.init 24 24 0 0.0
.init_array 208 208 0 0.0
.rodata 585972 585972 0 0.0
.text 9745156 9745156 0 0.0
thermostat-no-ble arm64 (read only) 2515692 2515388 -304 -0.0
(read/write) 145048 145048 0 0.0
.bss 56312 56312 0 0.0
.data 1784 1784 0 0.0
.data.rel.ro 77568 77568 0 0.0
.dynamic 560 560 0 0.0
.got 5336 5336 0 0.0
.init 24 24 0 0.0
.init_array 432 432 0 0.0
.rodata 150728 150568 -160 -0.1
.text 2102896 2102752 -144 -0.0
qpg lighting-app qpg6105+debug (read/write) 1151372 1151180 -192 -0.0
.bss 99796 99796 0 0.0
.data 852 852 0 0.0
.text 598468 598276 -192 -0.0
lock-app qpg6105+debug (read/write) 1118348 1118164 -184 -0.0
.bss 96284 96284 0 0.0
.data 864 864 0 0.0
.text 565448 565264 -184 -0.0

@bzbarsky-apple bzbarsky-apple merged commit b236bfb into project-chip:master Feb 23, 2023
@bzbarsky-apple bzbarsky-apple deleted the event-size-check branch February 23, 2023 16:51
lecndav pushed a commit to lecndav/connectedhomeip that referenced this pull request Mar 22, 2023
…t-chip#25257)

We could end up in a situation where an INFO event that fit into the DEBUG
buffer but did not fit into the INFO buffer would get written to the buffer, and
possibly evict some other events, but then we would error out from
LogEventPrivate, and leave ourselves in an inconsistent state.

The fix is to do the "this event fits in all the buffers that it might need to
fit in" check before we start modifying any state.
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.

3 participants