Skip to content

Revert "libs/libc/semaphore: Fix DEBUGASSERTS" #16216

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

Conversation

tmedicci
Copy link
Contributor

Summary

  • Revert "libs/libc/semaphore: Fix DEBUGASSERTS"

This reverts commit 3009922 to fix a problem with esp32-devkitc:blewifi, which fails to boot up if CONFIG_DEBUG_ASSERTIONS=y.

Introduced by #16176 with the following description:

The DEBUGASSERTS in nxsem_wait and nxsem_trywait are non-functional, they don't check anything. These were broken in previous commits.

The above statements are not valid. Originally, there was no problem calling nxsem_trywait from the interrupt and the DEBUGASSERT simply checked a corner case: if ran from the interrupt context, the current (interrupted) task may be the idle task. This case is allowed only if called from an interrupt and that's what the original commit checks with:

  DEBUGASSERT(!OSINIT_IDLELOOP() || !sched_idletask() ||
              up_interrupt_context());

Impact

Impact on user: YES, it fixes the esp32-devkitc:blewifi defconfig, which can now be used as before.

Impact on build: NO.

Impact on hardware: YES. Restore the old behavior for esp32-devkitc:blewifi defconfig.

Impact on documentation: NO.

Impact on security: NO.

Impact on compatibility: NO.

Testing

Build the esp32-devkitc:blewifi and compare the results before and after applying this patch. Before applying it, the defconfig fails to boot.

Building

make -j distclean && ./tools/configure.sh esp32-devkitc:blewifi && kconfig-tweak -e DEBUG_FEATURES && kconfig-tweak -e DEBUG_ASSERTIONS && make olddefconfig && make flash ESPTOOL_PORT=/dev/ttyUSB1 ESPTOOL_BINDIR=./ -s -j$(nproc) 

Running

picocom -b 115200 /dev/ttyUSB1

Results

Before

Before applying this patch, the device fails to boot:

*** Booting NuttX ***
I (95) boot: chip revision: v3.0
I (95) boot.esp32: SPI Speed      : 40MHz
I (95) boot.esp32: SPI Mode       : DIO
I (96) boot.esp32: SPI Flash Size : 4MB
I (100) boot: Enabling RNG early entropy source...
dram: lma 0x00001020 vma 0x3ffd5b80 len 0x5150   (20816)
iram: lma 0x00006178 vma 0x40080000 len 0x26fe4  (159716)
padd: lma 0x0002d168 vma 0x00000000 len 0x2e90   (11920)
imap: lma 0x00030000 vma 0x400f0000 len 0x8b828  (571432)
padd: lma 0x000bb830 vma 0x00000000 len 0x47c8   (18376)
dmap: lma 0x000c0000 vma 0x3f410000 len 0x16d4c  (93516)
total segments stored 6
A__esp32_start: ESP32 chip revision is v3.0
BI (68) phy_init: phy_version 4830,54550f7,Jun 20 2024,14:22:08
dump_assert_info: Current Version: NuttX  10.4.0 fd12f6d1ed Apr 14 2025 14:00:36 xtensa
dump_assert_info: Assertion failed : at file: semaphore/sem_trywait.c:113 task: nsh_main process: nsh_main 0x400f98e0
up_dump_register:    PC: 4009f3e7    PS: 00060022
up_dump_register:    A0: 8009f6e1    A1: 3ffd6840    A2: 00000001    A3: 3ffc4c80
up_dump_register:    A4: 0000186a    A5: 00000000    A6: 00000000    A7: 00000001
up_dump_register:    A8: 3ffc3a38    A9: 3ffd6a10   A10: 00000011   A11: 00000000
up_dump_register:   A12: 00000001   A13: 3ffddfb0   A14: 00000000   A15: 00000001
up_dump_register:   SAR: 0000001b CAUSE: 00000000 VADDR: 00000000
up_dump_register:  LBEG: 4000c2e0  LEND: 4000c2f6  LCNT: 00000000
dump_stackinfo: IRQ Stack:
dump_stackinfo:   base: 0x3ffd5bb0
dump_stackinfo:   size: 00004096
dump_stackinfo:     sp: 0x3ffd6840
stack_dump: 0x3ffd6820: 3f414ded 3ffd6840 4000c2f6 00000000 8009f449 3ffd6860 3ffd6840 00000000
stack_dump: 0x3ffd6840: 3ffd5bb0 00001000 3ffd6820 3ffd6bb0 800f6153 3ffd6950 3f4109af 00000071
stack_dump: 0x3ffd6860: 3ffdd2d4 3ffdd2d4 400f98e0 00000000 7474754e 00000058 00000000 00000000
stack_dump: 0x3ffd6880: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
stack_dump: 0x3ffd68a0: 00000000 2e303100 00302e34 00000000 00000000 00000000 64660000 36663231
stack_dump: 0x3ffd68c0: 64653164 72704120 20343120 35323032 3a343120 333a3030 00000036 00000000
stack_dump: 0x3ffd68e0: 00000000 00000000 00000000 65747800 0061736e 00000000 00000000 00000000
stack_dump: 0x3ffd6900: 00000000 3ffdd230 3ffc4c80 00000000 3f4109af 00000071 00000000 00000000
stack_dump: 0x3ffd6920: 00000006 00060e23 00000071 00000000 00000bc8 3ffde290 3ffdd6c8 3ffd5bb0
stack_dump: 0x3ffd6940: 8008a04c 3ffd6970 3ffdfa28 00000001 00000000 00000001 3ffd51f4 00000050
stack_dump: 0x3ffd6960: 8008ac12 3ffd6990 3ffdfa28 3ffd69b0 00000001 00000008 40084498 3ffde050
stack_dump: 0x3ffd6980: 8008ae62 3ffd69b0 4008a03c 3ffd93a8 00000000 00000200 3ffc5644 00000001
stack_dump: 0x3ffd69a0: 8008aba9 3ffd69e0 00000000 00000011 00000000 00000000 00000000 00000000
stack_dump: 0x3ffd69c0: 3ffd9490 00000003 00000000 00000000 8008a250 3ffd6a10 00000011 00000000
stack_dump: 0x3ffd69e0: 00000000 00000000 00000000 00000000 00000000 0000186a 00000000 0000008e
stack_dump: 0x3ffd6a00: 80083341 3ffd6a30 00000011 00000000 0000186a 3ffddfb0 00000000 00000001
stack_dump: 0x3ffd6a20: 800848ba 3ffd6a50 4008a244 00000011 0000186a 00000000 00000000 00000001
stack_dump: 0x3ffd6a40: 80086c4d 3ffd6a90 3ffe3320 3ffd5b94 00000000 00000000 00000000 00000000
stack_dump: 0x3ffd6a60: 00000000 00000000 00000000 00000000 00000000 0000186a 00000000 00000000
stack_dump: 0x3ffd6a80: 800552d0 3ffd6ad0 3ffb80ec 3ffe3320 00000000 00000000 00000000 00000000
stack_dump: 0x3ffd6aa0: 0000000a 3ffd9290 00000000 00000000 0000186a 00000001 3ffd51f4 00000050
stack_dump: 0x3ffd6ac0: 80086d2a 3ffd6af0 3ffb8d40 00000200 00000001 00000008 40084498 3ffde050
stack_dump: 0x3ffd6ae0: 8008a140 3ffd6b10 40055248 00000000 40086b5c 00000200 3ffc5644 00000001
stack_dump: 0x3ffd6b00: 8009f909 3ffd6b30 00000000 3ffddf34 3ffdd230 00000003 00000000 00000000
stack_dump: 0x3ffd6b20: 8009f36a 3ffd6b50 0000000a 3ffddf34 3ffdfe70 00000000 00000000 0000008e
stack_dump: 0x3ffd6b40: 8009e964 3ffd6b70 3ffdd230 3ffddf34 800f5e39 3ffddfb0 00000000 00000001
stack_dump: 0x3ffd6b60: 40080cf8 3ffd6b90 00000020 0000001f 3ffc0d48 3ffc3a38 00000000 00000000
stack_dump: 0x3ffd6b80: 00040023 3ffd6bb0 ffffffef 00018020 00000005 00000020 3ffd73e0 3ffdfbc0
stack_dump: 0x3ffd6ba0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
dump_stackinfo: User Stack:
dump_stackinfo:   base: 0x3ffdd6c8
dump_stackinfo:   size: 00003016
dump_stackinfo:     sp: 0x3ffde010
stack_dump: 0x3ffddff0: 00000001 3f413045 c0200000 00000000 80124d97 3ffde030 00000000 3ffde050
stack_dump: 0x3ffde010: 00000000 00000000 00000000 00000000 80126f22 3ffde050 3ffc5644 00000001
stack_dump: 0x3ffde030: 00000074 3ffde0c8 40138c00 4013c188 8014bbd6 3ffde150 00000000 00000000
stack_dump: 0x3ffde050: 3ffd73e0 00000074 00000001 40138c48 40138c34 4013c1c4 40138ea8 4013c214
stack_dump: 0x3ffde070: 4013c1fc 4013c24c 4013c234 40138c6c 4013c19c 4013c400 401789cc 4013c1b0
stack_dump: 0x3ffde090: 40138c24 4013c26c 4013c2b4 40138c14 4013c2c8 4013c310 4013c324 4013c370
stack_dump: 0x3ffde0b0: 40138b28 4013f3e8 4013f46c 401789c4 4013c188 40138c00 0000000a 00000020
stack_dump: 0x3ffde0d0: 00000001 00000000 00000020 00000000 00000005 00000000 00000000 00000001
stack_dump: 0x3ffde0f0: 00000001 00000000 00000000 00000000 00000006 00000000 000002f0 00000020
stack_dump: 0x3ffde110: 00000021 00000000 00000001 00000007 1f2f3f4f 3f41303f 00000004 00000000
stack_dump: 0x3ffde130: 00000020 3ffc5398 0000000a 00000006 8014bb3c 3ffde180 00000000 3ffd6e48
stack_dump: 0x3ffde150: 8014bb2b 3ffde180 00000000 3ffd6e48 00000001 00000000 00000000 ff000000
stack_dump: 0x3ffde170: 8014baba 3ffde1a0 3ffd6ea8 3f4114d4 3f413324 00000000 00000003 00000000
stack_dump: 0x3ffde190: 8011daef 3ffde1c0 00000000 3f4114e0 3ffdd1b0 3ffdd190 3ffd6c20 00000000
stack_dump: 0x3ffde1b0: 800f9952 3ffde1e0 0000ff01 00000000 000000ff 0000ff00 00ff0000 ff000000
stack_dump: 0x3ffde1d0: 800f9900 3ffde200 00000000 3ffde220 3ffdacd0 00001b70 00000000 00060020
stack_dump: 0x3ffde1f0: 800f6048 3ffde220 00000001 3ffdd6a8 00000001 00000001 00060a20 3ffc1868
stack_dump: 0x3ffde210: 800f3403 3ffde250 400f98e0 00000001 00000064 3ffde270 00000000 3ffdd230
stack_dump: 0x3ffde230: 3f426d2c 3ffde460 3ffdd2f8 00000000 00000000 3ffde270 00000000 400f98e0
stack_dump: 0x3ffde250: 3ffdd6a8 3ffdd130 00000001 3ffdd2f8 00000000 3ffde290 00000000 00000000
stack_dump: 0x3ffde270: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
stack_dump: 0x3ffde290: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
dump_tasks:    PID GROUP PRI POLICY   TYPE    NPX STATE   EVENT      SIGMASK          STACKBASE  STACKSIZE   COMMAND
dump_tasks:   ----   --- --- -------- ------- --- ------- ---------- ---------------- 0x3ffd5bb0      4096   irq
dump_task:       0     0   0 FIFO     Kthread -   Ready              0000000000000000 0x3ffc0010      3056   Idle_Task
dump_task:       1     0 224 RR       Kthread -   Waiting Semaphore  0000000000000000 0x3ffdb190      4032   hpwork 0x3ffd6c64 0x3ffd6c94
dump_task:       2     0 100 RR       Kthread -   Waiting Semaphore  0000000000000000 0x3ffdc268      4024   lpwork 0x3ffd6c18 0x3ffd6c48
dump_task:       3     3 100 RR       Task    -   Running            0000000000000000 0x3ffdd6c8      3016   nsh_main
dump_task:       4     0 223 RR       Kthread -   Waiting Semaphore  0000000000000000 0x3ffdea50      4048   rt_timer
dump_task:       5     0 253 RR       Kthread -   Waiting MQ empty   0000000000000000 0x3ffe4f80      4048   btController
dump_task:       6     0 100 RR       Kthread -   Waiting MQ empty   0000000000000000 0x3ffe6098      4040   BT_HCI_Tx

After

By reverting the commit, the device boots successfully, restoring the previous behavior:

rst:0x1 (POWERON_RESET),boot:0x12 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:2
load:0x3ffd5b80,len:20816
load:0x40080000,len:159716
1150 mmu set 00010000, pos 00010000
1150 mmu set 00020000, pos 00020000
entry 0x4008c154
*** Booting NuttX ***
I (95) boot: chip revision: v3.0
I (95) boot.esp32: SPI Speed      : 40MHz
I (95) boot.esp32: SPI Mode       : DIO
I (96) boot.esp32: SPI Flash Size : 4MB
I (100) boot: Enabling RNG early entropy source...
dram: lma 0x00001020 vma 0x3ffd5b80 len 0x5150   (20816)
iram: lma 0x00006178 vma 0x40080000 len 0x26fe4  (159716)
padd: lma 0x0002d168 vma 0x00000000 len 0x2e90   (11920)
imap: lma 0x00030000 vma 0x400f0000 len 0x8b820  (571424)
padd: lma 0x000bb828 vma 0x00000000 len 0x47d0   (18384)
dmap: lma 0x000c0000 vma 0x3f410000 len 0x16d4c  (93516)
total segments stored 6
A__esp32_start: ESP32 chip revision is v3.0
BI (68) phy_init: phy_version 4830,54550f7,Jun 20 2024,14:22:08
telnetd [8:100]

NuttShell (NSH) NuttX-10.4.0
nsh> bt bnep0 scan start
nsh> 

@github-actions github-actions bot added Area: OS Components OS Components issues Size: S The size of the change in this PR is small labels Apr 15, 2025
@tmedicci
Copy link
Contributor Author

@jlaitine , can you take a look please?

@jlaitine
Copy link
Contributor

jlaitine commented Apr 15, 2025

I kind of understand your explanation; but why on earth someone tries to take a semaphore in interrupt?

Is the situation such that you know that the semaphore can't block at that point in the irq, and you want to cause some task to block on it later?

But reverting that is fine by me !

@tmedicci
Copy link
Contributor Author

I kind of understand your explanation; but why on earth someone tries to take a semaphore in interrupt?

This comes from the bt adapter, which is inspired on ESP-IDF's. Take a look at semphr_take_from_isr_wrapper. Such callback calls FreeRTOS's interface that is safe to be called from interrupt.

Is the situation such that you know that the semaphore can't block at that point in the irq, and you want to cause some task to block on it later?

Yes, exactly. It's used internally as signalling only (that's why we use the non-blocking version).

But reverting that is fine by me !

Thanks!

@acassis
Copy link
Contributor

acassis commented Apr 15, 2025

@jlaitine please review (GH doesn't allow me add you as review)

Copy link
Contributor

@acassis acassis left a comment

Choose a reason for hiding this comment

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

@tmedicci please include the Summary info to the git commit log

This reverts commit 3009922 to
fix a problem with `esp32-devkitc:blewifi`, which fails to boot up
if `CONFIG_DEBUG_ASSERTIONS=y`.

Introduced by apache#16176 with the
following description:

> The DEBUGASSERTS in nxsem_wait and nxsem_trywait are
non-functional, they don't check anything. These were broken in
previous commits.

The above statements are not valid. Originally, there was no
problem calling `nxsem_trywait` from the interrupt and the
`DEBUGASSERT` simply checked a corner case: if ran from the
interrupt context, the current (interrupted) task may be the idle
task. This case is allowed only if called from an interrupt and
that's what the original commit checks with:

```
  DEBUGASSERT(!OSINIT_IDLELOOP() || !sched_idletask() ||
              up_interrupt_context());
```
@tmedicci tmedicci force-pushed the bugfix/esp32-devkitc_blewifi_debugassert branch from a1696a6 to 48c2167 Compare April 15, 2025 19:41
@tmedicci
Copy link
Contributor Author

@tmedicci please include the Summary info to the git commit log

Done.

@tmedicci tmedicci requested a review from acassis April 15, 2025 20:18
@xiaoxiang781216 xiaoxiang781216 merged commit 40c6af6 into apache:master Apr 16, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: OS Components OS Components issues Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants