Skip to content

Commit dbc5ecc

Browse files
committed
fix(FIR): size q15 state buffer for ARM_MATH_LOOPUNROLL, not just ARM_MATH_DSP
arm_fir_init_q15() only allocates the larger (numTaps+blockSize) state buffer when ARM_MATH_DSP is defined, otherwise it allocates the smaller (numTaps+blockSize-1). arm_fir_q15()'s non-MVE processing function has a separate ARM_MATH_LOOPUNROLL-gated code path (4-sample-at-a-time, using read_q15x2_ia()/__SMLALD()/__PKHBT(), which all have portable non-hardware-DSP fallback implementations and so do not themselves require ARM_MATH_DSP) that reads up to index numTaps+blockSize-1 from the state buffer - one element past what's allocated when ARM_MATH_LOOPUNROLL is defined without ARM_MATH_DSP, an out-of-bounds read. Verified with a standalone host reproduction of the exact read_q15x2_ia/__SMLALD/__PKHBT access pattern, placing the state buffer immediately before a PAGE_NOACCESS guard page: with the current (numTaps+blockSize-1)-sized buffer the reproduction crashes with an access violation; sized to (numTaps+blockSize) it completes cleanly. Fix: allocate the larger buffer whenever ARM_MATH_DSP *or* ARM_MATH_LOOPUNROLL is defined, matching what arm_fir_q15()'s loop-unrolled path actually needs regardless of whether hardware DSP instructions or their software fallback are in use. Fixes #305 Signed-off-by: 94xhn <87560781+94xhn@users.noreply.github.com>
1 parent b82f8ee commit dbc5ecc

1 file changed

Lines changed: 3 additions & 3 deletions

File tree

Source/FilteringFunctions/arm_fir_init_q15.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
{0.3, -0.3, 0, 0}.
7676
</pre>
7777
<code>pState</code> points to the array of state variables.
78-
<code>pState</code> is of length <code>numTaps+blockSize</code>, when ARM_MATH_DSP is defined. Otherwise, it is of length <code>numTaps+blockSize-1</code> where <code>blockSize</code> is the number of input samples processed by each call to <code>arm_fir_q15()</code>.
78+
<code>pState</code> is of length <code>numTaps+blockSize</code>, when ARM_MATH_DSP or ARM_MATH_LOOPUNROLL is defined (arm_fir_q15()'s loop-unrolled path reads up to <code>numTaps+blockSize-1</code>, requiring this larger size regardless of whether ARM_MATH_DSP itself is defined). Otherwise, it is of length <code>numTaps+blockSize-1</code> where <code>blockSize</code> is the number of input samples processed by each call to <code>arm_fir_q15()</code>.
7979
8080
@par Initialization of Helium version
8181
For Helium version the array of coefficients must be a multiple of 8 (8a) even if less
@@ -95,7 +95,7 @@ ARM_DSP_ATTRIBUTE arm_status arm_fir_init_q15(
9595
{
9696
arm_status status;
9797

98-
#if defined (ARM_MATH_DSP)
98+
#if defined (ARM_MATH_DSP) || defined (ARM_MATH_LOOPUNROLL)
9999

100100
/* The Number of filter coefficients in the filter must be even and at least 4 */
101101
if (numTaps & 0x1U)
@@ -139,7 +139,7 @@ ARM_DSP_ATTRIBUTE arm_status arm_fir_init_q15(
139139

140140
return (status);
141141

142-
#endif /* #if defined (ARM_MATH_DSP) */
142+
#endif /* #if defined (ARM_MATH_DSP) || defined (ARM_MATH_LOOPUNROLL) */
143143

144144
}
145145

0 commit comments

Comments
 (0)