Skip to content

Commit 4eef306

Browse files
authored
Cleanup pseudostack allocation paths (#72)
Consolidates the pseudostack machinery down to a single header and brings the custom `THREADSAFE` mode in line with upstream Opus. No functional change to decode/encode output. ## Consolidation - Deletes the standalone `patches/stack_alloc.h` and the `unused stack_alloc.patch`. `Silk/src` sources previously resolved this drifted copy via -I patches; they now use the single patched `celt/stack_alloc.h` like the celt sources already did. - Drops the `#else` catch-all in the mode selector for an explicit `#elif defined(NONTHREADSAFE_PSEUDOSTACK)` / `#elif defined(THREADSAFE_PSEUDOSTACK)` chain. ## Matching upstream in the `THREADSAFE` branch - Gates the `PUSH` overflow check behind `ENABLE_HARDENING`, the way upstream already gates `NONTHREADSAFE`. Off by default, so it drops a compare-and-branch from every allocation. - Aligns with `ALIGNOF(type)` instead of `sizeof(type)` and cast via `(type*)(void*)`, making `PUSH`/`ALIGN`/`ALIGNOF` byte-for-byte identical to upstream's `NONTHREADSAFE` branch. - Guards our fallback `celt_fatal` in `custom_support.h` behind `!ENABLE_ASSERTIONS && !ENABLE_HARDENING`, since `arch.h` defines it itself under those flags (avoids a redefinition collision under hardening and a missing symbol without it). ## Bug fixes - Aborts via `CELT_FATAL` when the per-thread pseudostack allocation returns `NULL`, instead of writing through a near-`NULL` pointer (the overflow guard degrades to a bare size check at a `NULL` base, and with the guard now off by default there is no check at all). - Routes `THREADSAFE` allocation through `opus_alloc_scratch()` in `custom_support.h` rather than a second `Kconfig` mapping. The old copy omitted `MALLOC_CAP_8BIT`, so the internal-RAM fallback could land in IRAM and fault on the first byte/halfword store into the pseudostack. - Size the `THREADSAFE` valgrind pseudostack to `GLOBAL_STACK_SIZE*2` to match its redzone-doubling `PUSH`; it was allocating 1x and overrunning past half usage. `_opus_alloc_and_register_pseudostack()` now takes the size from `ALLOC_STACK` so the buffer and the `PUSH` advance stay in step.
1 parent d9be825 commit 4eef306

7 files changed

Lines changed: 70 additions & 456 deletions

File tree

CLAUDE.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ patches/ # ESP32-specific patches applied at build time
1111
diffs/ # .patch files applied to staged opus copy
1212
celt/xtensa/ # Xtensa assembly optimizations
1313
silk/xtensa/ # SILK Xtensa optimizations
14-
stack_alloc.h # Thread-local pseudostack implementation
1514
custom_support.h # PSRAM-aware memory allocation
1615
cmake/ # Build system modules
1716
staging.cmake # Copies opus to build/opus-staged/ and applies patches

patches/custom_support.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#ifdef ESP_PLATFORM
3737
/* ESP-IDF build: Use PSRAM-aware allocation */
3838
#include "esp_heap_caps.h"
39+
#include "sdkconfig.h" /* CONFIG_OPUS_* macros used in the allocators below */
3940

4041
/* Override opus_alloc to use configurable memory allocation for Opus state/tables */
4142
#define OVERRIDE_OPUS_ALLOC
@@ -102,10 +103,16 @@ static inline void* opus_alloc_scratch(size_t size) {
102103
/* Host builds use the default Opus implementations (malloc/free) from os_support.h */
103104

104105
/* Function called on pseudostack overflow - required for pseudostack modes.
105-
* Called by the PUSH() macro when allocation exceeds GLOBAL_STACK_SIZE. */
106+
* Called by the PUSH() macro when allocation exceeds GLOBAL_STACK_SIZE.
107+
*
108+
* Only provide our own celt_fatal when upstream does not. When a consumer defines
109+
* ENABLE_ASSERTIONS or ENABLE_HARDENING, arch.h declares (and defines under CELT_C)
110+
* celt_fatal itself; defining our static inline here as well would collide. */
111+
#if !defined(ENABLE_ASSERTIONS) && !defined(ENABLE_HARDENING)
106112
static inline void celt_fatal(const char* str, const char* file, int line) {
107113
printf("FATAL ERROR: %s at %s:%d\n", str, file, line);
108114
abort();
109115
}
116+
#endif
110117

111118
#endif /* CUSTOM_SUPPORT_H */

patches/diffs/celt_stack_alloc.patch

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
#ifdef CELT_C
2424
char *scratch_ptr=0;
2525
char *global_stack=0;
26-
@@ -163,6 +165,74 @@
26+
@@ -163,6 +165,99 @@
2727
#define SAVE_STACK char *_saved_stack = global_stack;
2828
#define ALLOC_NONE 0
2929

@@ -61,7 +61,28 @@
6161
+extern _Thread_local char *global_stack;
6262
+
6363
+/* Allocate and register pseudostack - defined in thread_local_stack.c */
64-
+char* _opus_alloc_and_register_pseudostack(void);
64+
+char* _opus_alloc_and_register_pseudostack(size_t size);
65+
+
66+
+/* ALIGNOF mirrors the definition in the NONTHREADSAFE branch above; each
67+
+ * allocation-mode branch is self-contained, so it is repeated here. */
68+
+#if __STDC_VERSION__ >= 201112L
69+
+# include <stdalign.h>
70+
+# define ALIGNOF(T) alignof(T)
71+
+#elif defined(__GNUC__) || defined(__clang__)
72+
+# define ALIGNOF(T) __alignof__(T)
73+
+#else
74+
+# include <stddef.h>
75+
+# ifdef __cplusplus
76+
+template <typename T>
77+
+struct alignment_helper {
78+
+ char c;
79+
+ T member;
80+
+};
81+
+# define ALIGNOF(T) (offsetof(alignment_helper<T>, member))
82+
+# else
83+
+# define ALIGNOF(T) (offsetof(struct { char c; T member; }, member))
84+
+# endif
85+
+#endif
6586
+
6687
+#ifdef ENABLE_VALGRIND
6788
+
@@ -70,22 +91,26 @@
7091
+static _Thread_local char *global_stack_top = NULL;
7192
+
7293
+#define ALIGN(stack, size) ((stack) += ((size) - (long)(stack)) & ((size) - 1))
73-
+#define PUSH(stack, size, type) (VALGRIND_MAKE_MEM_NOACCESS(stack, global_stack_top-stack),ALIGN((stack),sizeof(type)/sizeof(char)),VALGRIND_MAKE_MEM_UNDEFINED(stack, ((size)*sizeof(type)/sizeof(char))),(stack)+=(2*(size)*sizeof(type)/sizeof(char)),(type*)((stack)-(2*(size)*sizeof(type)/sizeof(char))))
94+
+#define PUSH(stack, size, type) (VALGRIND_MAKE_MEM_NOACCESS(stack, global_stack_top-stack),ALIGN((stack),ALIGNOF(type)),VALGRIND_MAKE_MEM_UNDEFINED(stack, ((size)*sizeof(type)/sizeof(char))),(stack)+=(2*(size)*sizeof(type)/sizeof(char)),(type*)((stack)-(2*(size)*sizeof(type)/sizeof(char))))
7495
+#define RESTORE_STACK ((global_stack = _saved_stack),VALGRIND_MAKE_MEM_NOACCESS(global_stack, global_stack_top-global_stack))
75-
+#define ALLOC_STACK char *_saved_stack; ((global_stack = (global_stack==0) ? ((global_stack_top=_opus_alloc_and_register_pseudostack()+(GLOBAL_STACK_SIZE))-(GLOBAL_STACK_SIZE)) : global_stack),VALGRIND_MAKE_MEM_NOACCESS(global_stack, global_stack_top-global_stack)); _saved_stack = global_stack;
96+
+#define ALLOC_STACK char *_saved_stack; ((global_stack = (global_stack==0) ? ((global_stack_top=_opus_alloc_and_register_pseudostack(GLOBAL_STACK_SIZE*2)+(GLOBAL_STACK_SIZE*2))-(GLOBAL_STACK_SIZE*2)) : global_stack),VALGRIND_MAKE_MEM_NOACCESS(global_stack, global_stack_top-global_stack)); _saved_stack = global_stack;
7697
+
7798
+#else
7899
+
79-
+#include "arch.h"
80100
+#define ALIGN(stack, size) ((stack) += ((size) - (long)(stack)) & ((size) - 1))
81-
+#define PUSH(stack, size, type) (ALIGN((stack),sizeof(type)/(sizeof(char))),(void)(((int)((size)*(sizeof(type)/(sizeof(char)))) <= (scratch_ptr)+GLOBAL_STACK_SIZE-(stack))?0:CELT_FATAL("pseudostack overflow")),(stack)+=(size)*(sizeof(type)/(sizeof(char))),(type*)((stack)-(size)*(sizeof(type)/(sizeof(char)))))
101+
+#ifdef ENABLE_HARDENING
102+
+#include "arch.h"
103+
+#define PUSH(stack, size, type) (ALIGN((stack),ALIGNOF(type)),(void)(((int)((size)*(sizeof(type)/(sizeof(char)))) <= (scratch_ptr)+GLOBAL_STACK_SIZE-(stack))?0:CELT_FATAL("pseudostack overflow")),(stack)+=(size)*(sizeof(type)/(sizeof(char))),(type*)(void*)((stack)-(size)*(sizeof(type)/(sizeof(char)))))
104+
+#else
105+
+#define PUSH(stack, size, type) (ALIGN((stack),ALIGNOF(type)),(stack)+=(size)*(sizeof(type)/(sizeof(char))),(type*)(void*)((stack)-(size)*(sizeof(type)/(sizeof(char)))))
106+
+#endif
82107
+#if 0 /* Set this to 1 to instrument pseudostack usage */
83108
+#define RESTORE_STACK (printf("%ld %s:%d\n", global_stack-scratch_ptr, __FILE__, __LINE__),global_stack = _saved_stack)
84109
+#else
85110
+#define RESTORE_STACK (global_stack = _saved_stack)
86111
+#endif
87112
+/* Lazy allocation: check global_stack==0, allocate if needed, same pattern as non-threadsafe */
88-
+#define ALLOC_STACK char *_saved_stack; (global_stack = (global_stack==0) ? (_opus_alloc_and_register_pseudostack()) : global_stack); _saved_stack = global_stack;
113+
+#define ALLOC_STACK char *_saved_stack; (global_stack = (global_stack==0) ? (_opus_alloc_and_register_pseudostack(GLOBAL_STACK_SIZE)) : global_stack); _saved_stack = global_stack;
89114
+
90115
+#endif /* ENABLE_VALGRIND */
91116
+

patches/diffs/stack_alloc.patch

Lines changed: 0 additions & 107 deletions
This file was deleted.

0 commit comments

Comments
 (0)