Skip to content

Fix ABF NODE_MEMREQ instance header underallocation - #94

Closed
goyalpalak18 wants to merge 1 commit into
eembc:mainfrom
goyalpalak18:fix/abf-memreq-instance-size
Closed

Fix ABF NODE_MEMREQ instance header underallocation#94
goyalpalak18 wants to merge 1 commit into
eembc:mainfrom
goyalpalak18:fix/abf-memreq-instance-size

Conversation

@goyalpalak18

Copy link
Copy Markdown

Description

I tracked down a nasty bug causing HardFaults on our ARM and RISC-V targets. We had a memory calculation mismatch in ee_abf_f32.c that was silently eating our CMSIS-DSP safety padding. It was invisible on standard x86 builds, but broke on target hardware with strict memory protection (MPU/PMP) enabled.

Root Cause

In the NODE_MEMREQ calculation, we were only budgeting for two pointers. However, beamformer_f32_reset() actually uses all four pointers defined in the abf_f32_instance_t struct.

Because we shortchanged the allocation by 8 to 16 bytes, the instance header spilled over and consumed the 12-byte CMSIS-DSP safety padding at the end of the buffer. When vectorized operations (like Helium or NEON) did their normal read-past-end tail processing, they hit unmapped memory and triggered immediate HardFaults.

The Fix

I replaced the hardcoded pointer math with sizeof(abf_f32_instance_t). This correctly accounts for the entire struct, is self-documenting, and prevents the header from overwriting the safety padding.

Impact

  • Stability: Eliminates the HardFaults/crashes on our high-performance embedded targets.
  • Memory: Fully restores the required CMSIS-DSP safety padding (ensuring 16 bytes of padding on both 32-bit and 64-bit systems). Total heap allocation just goes up by a harmless 8-16 bytes.
  • Algorithm: Completely untouched. Data flow and output remain exactly the same.

Signed-off-by: goyalpalak18 <goyalpalak1806@gmail.com>
@goyalpalak18

Copy link
Copy Markdown
Author

@llefaucheur @joseph-yiu I put together a fix for the memory allocation issue in ee_abf_f32.c. Ready for review when you have a moment

@llefaucheur

Copy link
Copy Markdown
Contributor

Its good for me. I did not checked but I see no reason the Audiomark score to being changed with this patch.

@goyalpalak18

Copy link
Copy Markdown
Author

Thanks @llefaucheur . I can confirm the scores will stay exactly the same. Let me know if you need anything else before merging!

@joseph-yiu

Copy link
Copy Markdown
Contributor

Hi @goyalpalak18 , @llefaucheur ,
I am away right now (holiday) and can help reviewing and merging this into the dev branch when I am back next week.
regards,
Joseph

@joseph-yiu

Copy link
Copy Markdown
Contributor

Merged into dev branch (dev_2026q1). Thanks for the patch.
regards,
Joseph

@joseph-yiu joseph-yiu self-assigned this Feb 24, 2026
@joseph-yiu

Copy link
Copy Markdown
Contributor

Closing this pull request as the AudioMark v1.0.4 release is completed.

@joseph-yiu joseph-yiu closed this Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants