From 97a6a187c38325741373ce8685ed54ba1b00e5c8 Mon Sep 17 00:00:00 2001 From: vlazzarini Date: Wed, 14 Oct 2026 10:54:28 +0100 Subject: [PATCH 1/4] Address review findings in Faust opcodes for Csound 7 - faustgen: terminate control label strings with NUL instead of newline - faustcompile: pass the LLVM target as a real string argument, with overloads for the 4- and 5-argument forms; guard factory creation and the shared factory/DSP registries with a mutex - faustdsp: free the controls object on deletion - faustplay: null-check the DSP registry, validate channel counts before using the engine, do not re-initialise or delete the borrowed engine, and clear all outputs on the early-exit path - faustaudio/faustgen: validate channel counts before registering the DSP instance, and fix the instance counter increment - faustcompile: fix a crash at teardown caused by the reset callback and the asynchronous compilation thread referring to per-note opcode memory; keep their state in a persistent heap object and snapshot the code, args and target before spawning the thread --- 7.x/src/faustcsound/faustgen.cpp | 317 +++++++++++++++++++------------ 1 file changed, 192 insertions(+), 125 deletions(-) diff --git a/7.x/src/faustcsound/faustgen.cpp b/7.x/src/faustcsound/faustgen.cpp index 7d85066..041e16d 100644 --- a/7.x/src/faustcsound/faustgen.cpp +++ b/7.x/src/faustcsound/faustgen.cpp @@ -40,6 +40,7 @@ #include "faust/gui/UI.h" #include #include +#include #ifdef _WIN32 #include #else @@ -49,6 +50,12 @@ #define MAXARG 40 +/* Faust factory creation and the shared factory/DSP registries are not + thread-safe, so all factories are created and all registry lists are + updated while holding this lock. It is shared by every opcode instance, + including the asynchronous faustcompile threads. */ +static std::mutex faust_registry_mutex; + static int dir_exists(const char *p) { #ifdef _WIN32 DWORD a = GetFileAttributesA(p); @@ -138,7 +145,7 @@ class controls : public UI { pctl->nxt = new ctl; pctl = pctl->nxt; strncpy(pctl->label, label, 63); - pctl->label[63] = '\n'; pctl->label[64] = '\0'; + pctl->label[63] = '\0'; pctl->label[64] = '\0'; pctl->zone = zone; pctl->min = min; pctl->max = max; @@ -249,15 +256,16 @@ struct faustobj { * Faust compile opcode usage: - ihandle faustcompile Scode, Sargs[,iasync, istacksize] + ihandle faustcompile Scode, Sargs[,iasync, istacksize[,Starget]] ihandle - handle to compiled code Scode - Faust program Sargs - Faust compiler args istacksize - compiler stack size in megabytes (default 1MB). NOTE: this - argument is accepted for backwards compatibility but is currently ignored, - as the Csound 7 threading API no longer supports setting the stack size. + argument is accepted for backwards compatibility but is currently ignored, + as the Csound 7 threading API no longer supports setting the stack size. iasync - async operation (1 = on, 0 = off) default to 1 + Starget - optional LLVM target string (empty selects the native target) **/ struct faustcompile { OPDS h; @@ -266,10 +274,17 @@ struct faustcompile { STRINGDAT *args; cs_float *async; cs_float *stacksize; - cs_float *extra; + STRINGDAT *target; /* optional LLVM target string */ +}; + +/* Persistent state for a compiled factory. The asynchronous compilation + thread and the reset callback must not refer to the per-note faustcompile + opcode, whose memory is released as soon as the instrument that runs + faustcompile ends. */ +struct faustcompile_state { + CSOUND *csound; llvm_dsp_factory *factory; void *thread; - void *lock; }; char **parse_cmd(CSOUND *csound, char *str, int32_t *argc) { @@ -308,105 +323,118 @@ char **parse_cmd(CSOUND *csound, char *str, int32_t *argc) { int32_t delete_faustcompile(CSOUND *csound, void *p) { - faustcompile *pp = ((faustcompile *)p); + faustcompile_state *st = ((faustcompile_state *)p); faustobj *fobj, *prv, **pfobj; - if (pp->thread) - csound->JoinThread(pp->thread); - pfobj = (faustobj **)csound->QueryGlobalVariable(csound, "::factory"); - if(pfobj != NULL) { - fobj = *pfobj; - prv = fobj; - while (fobj != NULL) { - if (fobj->obj == (void *)pp->factory) { - prv->nxt = fobj->nxt; - break; - } + if (st->thread) + csound->JoinThread(st->thread); + { + std::lock_guard guard(faust_registry_mutex); + pfobj = (faustobj **)csound->QueryGlobalVariable(csound, "::factory"); + if(pfobj != NULL) { + fobj = *pfobj; prv = fobj; - fobj = fobj->nxt; - } - if (fobj != NULL) { - if (*pfobj == fobj) - *pfobj = fobj->nxt; - deleteDSPFactory(pp->factory); - csound->Free(csound, fobj); + while (fobj != NULL) { + if (fobj->obj == (void *)st->factory) { + prv->nxt = fobj->nxt; + break; + } + prv = fobj; + fobj = fobj->nxt; + } + if (fobj != NULL) { + if (*pfobj == fobj) + *pfobj = fobj->nxt; + deleteDSPFactory(st->factory); + csound->Free(csound, fobj); + } } } - csound->DestroyMutex(pp->lock); + csound->Free(csound, st); return OK; } struct hdata { - faustcompile *p; CSOUND *csound; + char *code; + char *args; + char *target; + cs_float *hptr; + faustcompile_state *state; }; void *init_faustcompile_thread(void *pp) { - faustcompile *p = ((hdata *)pp)->p; + hdata *hd = (hdata *)pp; faustobj **pffactory, *ffactory; - CSOUND *csound = ((hdata *)pp)->csound; + CSOUND *csound = hd->csound; llvm_dsp_factory *factory; int32_t argc = 0; std::string err_msg; - char *cmd = (char *) csound->Calloc(csound, p->args->size + 9); - char *ccode = csound->Strdup(csound, p->code->data); - char *extra; - cs_float test = *p->extra; - strcpy(cmd, p->args->data); + size_t alen = hd->args ? strlen(hd->args) : 0; + char *cmd = (char *) csound->Calloc(csound, alen + 9); + const char *ccode = hd->code; + const char *target = hd->target ? hd->target : ""; + if (alen) + strcpy(cmd, hd->args); #ifdef USE_DOUBLE strcat(cmd, " -double"); #endif const char **argv = (const char **) parse_cmd(csound, cmd, &argc); const char *varname = "::factory"; - if(test) - extra = ((STRINGDAT *) p->extra)->data; - else - extra = (char *) ""; - - // Need to protect this - csound->LockMutex((void *) p->lock); - factory = createDSPFactoryFromString("faustop", (const char *) ccode, - argc, argv, extra, err_msg, 3); - csound->UnlockMutex((void *) p->lock); + { + std::lock_guard guard(faust_registry_mutex); + factory = createDSPFactoryFromString("faustop", ccode, + argc, argv, target, err_msg, 3); + hd->state->factory = factory; + } if (factory == NULL) { csound->Message(csound, Str("\nFaust compilation problem:\nline %s\n"), err_msg.c_str()); - *(p->hptr) = FL(-2.0); // error code. + if (hd->hptr) + *(hd->hptr) = FL(-2.0); // error code. csound->Free(csound, argv); csound->Free(csound, cmd); - csound->Free(csound, ccode); - csound->Free(csound, pp); + csound->Free(csound, hd->code); + csound->Free(csound, hd->args); + if (hd->target) + csound->Free(csound, hd->target); + csound->Free(csound, hd); return NULL; } - pffactory = (faustobj **)csound->QueryGlobalVariable(csound, varname); - if (pffactory == NULL) { - csound->CreateGlobalVariable(csound, varname, sizeof(faustobj *)); + { + std::lock_guard guard(faust_registry_mutex); pffactory = (faustobj **)csound->QueryGlobalVariable(csound, varname); - ffactory = (faustobj *)csound->Calloc(csound, sizeof(faustobj)); - ffactory->obj = factory; - ffactory->nxt = NULL; - ffactory->cnt = 0ul; - *pffactory = ffactory; - } else { - ffactory = *pffactory; - while (ffactory->nxt) { + if (pffactory == NULL) { + csound->CreateGlobalVariable(csound, varname, sizeof(faustobj *)); + pffactory = (faustobj **)csound->QueryGlobalVariable(csound, varname); + ffactory = (faustobj *)csound->Calloc(csound, sizeof(faustobj)); + ffactory->obj = factory; + ffactory->nxt = NULL; + ffactory->cnt = 0ul; + *pffactory = ffactory; + } else { + ffactory = *pffactory; + while (ffactory->nxt) { + ffactory = ffactory->nxt; + } + ffactory->nxt = (faustobj *) csound->Calloc(csound, sizeof(faustobj)); + ffactory->nxt->cnt = ffactory->cnt+1ul; ffactory = ffactory->nxt; + ffactory->obj = factory; } - ffactory->nxt = (faustobj *) csound->Calloc(csound, sizeof(faustobj)); - ffactory->nxt->cnt = ffactory->cnt+1ul; - ffactory = ffactory->nxt; - ffactory->obj = factory; } - p->factory = factory; - if(p->hptr) - *p->hptr = FL(ffactory->cnt); + if(hd->hptr) + *hd->hptr = FL(ffactory->cnt); csound->Free(csound, argv); csound->Free(csound, cmd); - csound->Free(csound, ccode); - csound->Free(csound, pp); + csound->Free(csound, hd->code); + csound->Free(csound, hd->args); + if (hd->target) + csound->Free(csound, hd->target); + csound->Free(csound, hd); return NULL; } @@ -417,22 +445,37 @@ static uintptr_t init_faustcompile_thread_runner(void *pp) { } int32_t init_faustcompile(CSOUND *csound, faustcompile *p) { + faustcompile_state *st = + (faustcompile_state *)csound->Malloc(csound, sizeof(faustcompile_state)); + st->csound = csound; + st->factory = NULL; + st->thread = NULL; + + /* Snapshot everything the asynchronous thread needs, so that it never + touches the per-note opcode memory (which is freed when this instrument + ends). */ hdata *data = (hdata *)csound->Malloc(csound, sizeof(hdata)); data->csound = csound; - data->p = p; + data->code = csound->Strdup(csound, p->code->data); + data->args = csound->Strdup(csound, p->args ? p->args->data : ""); + data->target = NULL; + if (p->INOCOUNT > 4 && p->target != NULL && p->target->data != NULL) + data->target = csound->Strdup(csound, p->target->data); + data->hptr = p->hptr; + data->state = st; + *p->hptr = -1.0; - p->lock = csound->Create_Mutex(0); - p->thread = NULL; if(!(int)*p->async) { /* synchronous compilation, run on the calling thread (large stack) */ init_faustcompile_thread(data); } else { - p->thread = csound->CreateThread(init_faustcompile_thread_runner, data); + st->thread = csound->CreateThread(init_faustcompile_thread_runner, data); } /* Factory cleanup is deferred to reset, so that instruments using the - compiled factory are not affected when the compiling instrument ends. */ - csound->RegisterResetCallback(csound, p, delete_faustcompile); + compiled factory are not affected when the compiling instrument ends. + The callback owns the persistent state, not the opcode. */ + csound->RegisterResetCallback(csound, st, delete_faustcompile); return OK; } @@ -465,6 +508,7 @@ int32_t delete_faustdsp(CSOUND *csound, void *p) { if (fobj != NULL) { if (*pfobj == fobj) *pfobj = fobj->nxt; + delete fobj->ctls; csound->Free(csound, fobj); delete pp->engine; } else @@ -504,13 +548,16 @@ int32_t init_faustdsp(CSOUND *csound, faustdsp *p) { fobjp = (faustobj **)csound->QueryGlobalVariable(csound, "::factory"); if (fobjp == NULL) return csound->InitError(csound, "%s", Str("no factory available\n")); - fobj = *fobjp; - while ((int32_t)fobj->cnt != factory) { - fobj = fobj->nxt; - if (fobj == NULL) - return csound->InitError(csound, Str("factory not found %d\n"), - (int32_t)factory); + fobj = NULL; + { + std::lock_guard guard(faust_registry_mutex); + fobj = *fobjp; + while (fobj != NULL && (int32_t)fobj->cnt != factory) + fobj = fobj->nxt; } + if (fobj == NULL) + return csound->InitError(csound, Str("factory not found %d\n"), + (int32_t)factory); dsp = ((llvm_dsp_factory *)fobj->obj)->createDSPInstance(); if (dsp == NULL) @@ -577,6 +624,8 @@ int32_t init_faustplay(CSOUND *csound, faustplay *p) { if (fobjp == NULL) return csound->InitError(csound, "%s", Str("no dsp instances available\n")); fobj = *fobjp; + if (fobj == NULL) + return csound->InitError(csound, "%s", Str("no dsp instances available\n")); while ((int32_t)fobj->cnt != instance) { fobj = fobj->nxt; @@ -586,14 +635,17 @@ int32_t init_faustplay(CSOUND *csound, faustplay *p) { } p->engine = (llvm_dsp *)fobj->obj; - p->engine->init((int)csound->GetEngineSr(csound)); + /* The engine is owned and initialised by the opcode that created it + (faustdsp, faustaudio or faustgen), so it is neither re-initialised nor + deleted here; doing so would reset its state and invalidate the + registry. */ if (p->engine->getNumInputs() != p->INOCOUNT - 1) { - delete p->engine; + p->engine = NULL; return csound->InitError(csound, "%s", Str("wrong number of input args\n")); } if (p->engine->getNumOutputs() != p->OUTOCOUNT) { - delete p->engine; + p->engine = NULL; return csound->InitError(csound, "%s", Str("wrong number of output args\n")); } @@ -619,7 +671,7 @@ int32_t perf_faustplay(CSOUND *csound, faustplay *p) { cs_float **out_tmp = (cs_float **)p->memout.auxp; if (UNLIKELY(early)) { - for (i = 0; i < p->OUTOCOUNT - 1; i++) + for (i = 0; i < p->OUTOCOUNT; i++) memset(p->outs[i], '\0', nsmps * sizeof(cs_float)); nsmps -= early; } @@ -746,18 +798,34 @@ int32_t init_faustaudio(CSOUND *csound, faustgen *p) { fobjp = (faustobj **)csound->QueryGlobalVariable(csound, "::factory"); if (fobjp == NULL) return csound->InitError(csound, "%s", Str("no factory available\n")); - fobj = *fobjp; - while ((int32_t)fobj->cnt != factory) { - fobj = fobj->nxt; - if (fobj == NULL) - return csound->InitError(csound, Str("factory not found %d\n"), - (int32_t)factory); + fobj = NULL; + { + std::lock_guard guard(faust_registry_mutex); + fobj = *fobjp; + while (fobj != NULL && (int32_t)fobj->cnt != factory) + fobj = fobj->nxt; } + if (fobj == NULL) + return csound->InitError(csound, Str("factory not found %d\n"), + (int32_t)factory); dsp = ((llvm_dsp_factory *)fobj->obj)->createDSPInstance(); if (dsp == NULL) return csound->InitError(csound, "%s", Str("Faust instantiation problem\n")); + /* Validate the channel counts before publishing the instance, so that an + invalid DSP is never left reachable through the ::dsp registry. */ + if (dsp->getNumInputs() != p->INOCOUNT - 1) { + delete ctls; + delete dsp; + return csound->InitError(csound, "%s", Str("wrong number of input args\n")); + } + if (dsp->getNumOutputs() != p->OUTOCOUNT - 1) { + delete ctls; + delete dsp; + return csound->InitError(csound, "%s", Str("wrong number of output args\n")); + } + dsp->buildUserInterface(ctls); pfdsp = (faustobj **)csound->QueryGlobalVariable(csound, varname); if (pfdsp == NULL) { @@ -794,17 +862,6 @@ int32_t init_faustaudio(CSOUND *csound, faustgen *p) { p->engine = (llvm_dsp *)fdsp->obj; p->engine->init((int)csound->GetEngineSr(csound)); - if (p->engine->getNumInputs() != p->INOCOUNT - 1) { - delete p->engine; - p->engine = NULL; - return csound->InitError(csound, "%s", Str("wrong number of input args\n")); - } - if (p->engine->getNumOutputs() != p->OUTOCOUNT - 1) { - delete p->engine; - p->engine = NULL; - return csound->InitError(csound, "%s", Str("wrong number of output args\n")); - } - /* memory for sampAccurate offsets */ if (parms->sampleAccurate) { size_t size; @@ -838,8 +895,11 @@ void *init_faustgen_thread(void *pp) { controls *ctls = new controls(); const char *varname = "::dsp"; p->engine = NULL; - p->factory = createDSPFactoryFromString( - "faustop", (const char *)p->code->data, argc, argv, "", err_msg, 3); + { + std::lock_guard guard(faust_registry_mutex); + p->factory = createDSPFactoryFromString( + "faustop", (const char *)p->code->data, argc, argv, "", err_msg, 3); + } if (p->factory == NULL) { csound->InitError(csound, Str("Faust compilation problem: %s\n"), err_msg.c_str()); @@ -857,6 +917,30 @@ void *init_faustgen_thread(void *pp) { dsp->buildUserInterface(ctls); + /* Validate the channel counts before publishing the instance, so that an + invalid DSP is never left reachable through the ::dsp registry. */ + if (dsp->getNumInputs() != p->INOCOUNT - 1) { + csound->InitError(csound, "%s", Str("wrong number of input args\n")); + delete ctls; + delete dsp; + deleteDSPFactory(p->factory); + p->factory = NULL; + csound->Free(csound, pp); + return NULL; + } + if (dsp->getNumOutputs() != p->OUTOCOUNT - 1) { + csound->InitError(csound, + Str("wrong number of output args: need %d had %d\n"), + dsp->getNumOutputs(), + p->OUTOCOUNT - 1); + delete ctls; + delete dsp; + deleteDSPFactory(p->factory); + p->factory = NULL; + csound->Free(csound, pp); + return NULL; + } + pfdsp = (faustobj **)csound->QueryGlobalVariable(csound, varname); if (pfdsp == NULL || *pfdsp == NULL) { csound->CreateGlobalVariable(csound, varname, sizeof(faustobj *)); @@ -873,7 +957,7 @@ void *init_faustgen_thread(void *pp) { fdsp = fdsp->nxt; } fdsp->nxt = (faustobj *)csound->Calloc(csound, sizeof(faustobj)); - fdsp->nxt->cnt = fdsp->cnt++; + fdsp->nxt->cnt = fdsp->cnt + 1; fdsp = fdsp->nxt; fdsp->obj = dsp; fdsp->ctls = ctls; @@ -881,27 +965,6 @@ void *init_faustgen_thread(void *pp) { p->engine = dsp; dsp->init((int)csound->GetEngineSr(csound)); - if (p->engine->getNumInputs() != p->INOCOUNT - 1) { - csound->InitError(csound, "%s", Str("wrong number of input args\n")); - delete p->engine; - deleteDSPFactory(p->factory); - p->factory = NULL; - p->engine = NULL; - csound->Free(csound, pp); - return NULL; - } - if (p->engine->getNumOutputs() != p->OUTOCOUNT - 1) { - csound->InitError(csound, - Str("wrong number of output args: need %d had %d\n"), - p->engine->getNumOutputs(), - p->OUTOCOUNT - 1); - delete p->engine; - deleteDSPFactory(p->factory); - csound->Free(csound, pp); - p->engine = NULL; - p->factory = NULL; - return NULL; - } /* memory for sampAccurate offsets */ if (parms->sampleAccurate) { @@ -1005,6 +1068,8 @@ int32_t init_faustctl(CSOUND *csound, faustctl *p) { if (fobjp == NULL) return csound->InitError(csound, "%s", Str("no dsp instances available\n")); fobj = *fobjp; + if (fobj == NULL) + return csound->InitError(csound, "%s", Str("no dsp instances available\n")); while ((int32_t)fobj->cnt != instance) { fobj = fobj->nxt; @@ -1073,7 +1138,9 @@ static OENTRY localops[] = { {(char *)"faustgen", S(faustgen), 0, (char *)"immmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm", (char *)"SM", (SUBR)init_faustgen, (SUBR)perf_faust, (SUBR)delete_faustgen, NULL, 0}, - {(char *)"faustcompile", S(faustcompile), 0, (char *)"i", (char *)"SSppo", + {(char *)"faustcompile", S(faustcompile), 0, (char *)"i", (char *)"SSpp", + (SUBR)init_faustcompile, NULL, NULL, NULL, 0}, + {(char *)"faustcompile", S(faustcompile), 0, (char *)"i", (char *)"SSppS", (SUBR)init_faustcompile, NULL, NULL, NULL, 0}, {(char *)"faustaudio", S(faustgen), 0, (char *)"immmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm", (char *)"iM", From 4e20eedb7340e312c68ca96bc8f2d449b0b2f21e Mon Sep 17 00:00:00 2001 From: vlazzarini Date: Wed, 14 Oct 2026 11:01:31 +0100 Subject: [PATCH 2/4] Fix standard header include order in faustgen.cpp Including after csdl.h pulled in after Csound's interlocks.h had defined macros such as _CR and TR, which clash with identifiers used by in libstdc++ and the MSVC STL. Include the C++ standard headers before csdl.h. --- 7.x/src/faustcsound/faustgen.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/7.x/src/faustcsound/faustgen.cpp b/7.x/src/faustcsound/faustgen.cpp index 041e16d..623f9e5 100644 --- a/7.x/src/faustcsound/faustgen.cpp +++ b/7.x/src/faustcsound/faustgen.cpp @@ -32,15 +32,19 @@ (convenient for one-off Faust programs) */ +/* The C++ standard headers must be included before "csdl.h": the latter pulls + in Csound's "interlocks.h", which defines short generic macros such as + _CR and TR that clash with identifiers used inside (reached via + ) in both libstdc++ and the MSVC STL. */ +#include +#include +#include #include "csdl.h" #ifndef FAUSTFLOAT #define FAUSTFLOAT cs_float #endif #include "faust/dsp/llvm-dsp.h" #include "faust/gui/UI.h" -#include -#include -#include #ifdef _WIN32 #include #else From efbeb90324ef34e88319b8f379e8fd13b9976f4a Mon Sep 17 00:00:00 2001 From: vlazzarini Date: Wed, 14 Oct 2026 11:13:50 +0100 Subject: [PATCH 3/4] Use a separate mutex for Faust factory compilation Serialising createDSPFactoryFromString with the registry mutex allowed a long async LLVM compile to block factory lookups on the performance thread in faustdsp/faustaudio, defeating the purpose of asynchronous compilation. Add faust_compile_mutex for factory creation and keep faust_registry_mutex for the short ::factory list operations. --- 7.x/src/faustcsound/faustgen.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/7.x/src/faustcsound/faustgen.cpp b/7.x/src/faustcsound/faustgen.cpp index 623f9e5..a3d5530 100644 --- a/7.x/src/faustcsound/faustgen.cpp +++ b/7.x/src/faustcsound/faustgen.cpp @@ -54,10 +54,15 @@ #define MAXARG 40 -/* Faust factory creation and the shared factory/DSP registries are not - thread-safe, so all factories are created and all registry lists are - updated while holding this lock. It is shared by every opcode instance, - including the asynchronous faustcompile threads. */ +/* Faust/LLVM factory creation is not thread-safe, so all calls to + createDSPFactoryFromString are serialised by this lock. It is deliberately + separate from the registry lock: a compilation can take a long time, and + holding the registry lock for its duration would stall factory lookups + (and thus the performance thread) in faustdsp/faustaudio. */ +static std::mutex faust_compile_mutex; + +/* Protects the shared ::factory and ::dsp registry lists. Held only for the + short operations that read or modify those lists. */ static std::mutex faust_registry_mutex; static int dir_exists(const char *p) { @@ -387,11 +392,11 @@ void *init_faustcompile_thread(void *pp) { const char *varname = "::factory"; { - std::lock_guard guard(faust_registry_mutex); + std::lock_guard guard(faust_compile_mutex); factory = createDSPFactoryFromString("faustop", ccode, argc, argv, target, err_msg, 3); - hd->state->factory = factory; } + hd->state->factory = factory; if (factory == NULL) { csound->Message(csound, Str("\nFaust compilation problem:\nline %s\n"), @@ -900,7 +905,7 @@ void *init_faustgen_thread(void *pp) { const char *varname = "::dsp"; p->engine = NULL; { - std::lock_guard guard(faust_registry_mutex); + std::lock_guard guard(faust_compile_mutex); p->factory = createDSPFactoryFromString( "faustop", (const char *)p->code->data, argc, argv, "", err_msg, 3); } From 4a5353b89449de1a934455ffaafcc42cf34f9ef5 Mon Sep 17 00:00:00 2001 From: vlazzarini Date: Wed, 14 Oct 2026 11:19:55 +0100 Subject: [PATCH 4/4] Resolve faustcompile handles without touching per-note memory The asynchronous compilation worker wrote the resulting handle (or error code) through p->hptr, which init_faustcompile had stored in persistent state. For a note-local destination that memory is reclaimed when the instrument ends, so a compile that outlived the note wrote to freed memory. Reserve the ::factory registry slot during init and publish the handle synchronously, while the opcode is still alive. The worker now only fills in the reserved slot (NULL = pending, (void*)-1 = failure, otherwise the factory); faustdsp/faustaudio resolve the slot and wait for it through a new resolve_factory() helper. This removes the use-after-free for local and global handles alike, and hdata no longer carries p->hptr. --- 7.x/src/faustcsound/faustgen.cpp | 219 ++++++++++++++++--------------- 1 file changed, 115 insertions(+), 104 deletions(-) diff --git a/7.x/src/faustcsound/faustgen.cpp b/7.x/src/faustcsound/faustgen.cpp index a3d5530..108951f 100644 --- a/7.x/src/faustcsound/faustgen.cpp +++ b/7.x/src/faustcsound/faustgen.cpp @@ -294,6 +294,7 @@ struct faustcompile_state { CSOUND *csound; llvm_dsp_factory *factory; void *thread; + faustobj *node; /* reserved ::factory slot, published by the worker */ }; char **parse_cmd(CSOUND *csound, char *str, int32_t *argc) { @@ -341,19 +342,21 @@ int32_t delete_faustcompile(CSOUND *csound, void *p) { pfobj = (faustobj **)csound->QueryGlobalVariable(csound, "::factory"); if(pfobj != NULL) { fobj = *pfobj; - prv = fobj; + prv = NULL; while (fobj != NULL) { - if (fobj->obj == (void *)st->factory) { - prv->nxt = fobj->nxt; + if (fobj == st->node) { + if (prv == NULL) + *pfobj = fobj->nxt; + else + prv->nxt = fobj->nxt; break; } prv = fobj; fobj = fobj->nxt; } if (fobj != NULL) { - if (*pfobj == fobj) - *pfobj = fobj->nxt; - deleteDSPFactory(st->factory); + if (st->factory != NULL) + deleteDSPFactory(st->factory); csound->Free(csound, fobj); } } @@ -367,14 +370,18 @@ struct hdata { char *code; char *args; char *target; - cs_float *hptr; faustcompile_state *state; }; +/* A factory slot is published in three states: obj == NULL while the + asynchronous compilation is still running, obj == (void *)-1 if it + failed, and otherwise obj is the llvm_dsp_factory. */ +#define FAUST_SLOT_PENDING ((void *)NULL) +#define FAUST_SLOT_FAILED ((void *)-1L) + void *init_faustcompile_thread(void *pp) { hdata *hd = (hdata *)pp; - faustobj **pffactory, *ffactory; CSOUND *csound = hd->csound; llvm_dsp_factory *factory; int32_t argc = 0; @@ -389,7 +396,6 @@ void *init_faustcompile_thread(void *pp) { strcat(cmd, " -double"); #endif const char **argv = (const char **) parse_cmd(csound, cmd, &argc); - const char *varname = "::factory"; { std::lock_guard guard(faust_compile_mutex); @@ -398,45 +404,18 @@ void *init_faustcompile_thread(void *pp) { } hd->state->factory = factory; - if (factory == NULL) { + if (factory == NULL) csound->Message(csound, Str("\nFaust compilation problem:\nline %s\n"), err_msg.c_str()); - if (hd->hptr) - *(hd->hptr) = FL(-2.0); // error code. - csound->Free(csound, argv); - csound->Free(csound, cmd); - csound->Free(csound, hd->code); - csound->Free(csound, hd->args); - if (hd->target) - csound->Free(csound, hd->target); - csound->Free(csound, hd); - return NULL; - } + /* Publish the outcome into the slot reserved during init; never touch the + per-note output pointer, which may already have been reclaimed. */ { std::lock_guard guard(faust_registry_mutex); - pffactory = (faustobj **)csound->QueryGlobalVariable(csound, varname); - if (pffactory == NULL) { - csound->CreateGlobalVariable(csound, varname, sizeof(faustobj *)); - pffactory = (faustobj **)csound->QueryGlobalVariable(csound, varname); - ffactory = (faustobj *)csound->Calloc(csound, sizeof(faustobj)); - ffactory->obj = factory; - ffactory->nxt = NULL; - ffactory->cnt = 0ul; - *pffactory = ffactory; - } else { - ffactory = *pffactory; - while (ffactory->nxt) { - ffactory = ffactory->nxt; - } - ffactory->nxt = (faustobj *) csound->Calloc(csound, sizeof(faustobj)); - ffactory->nxt->cnt = ffactory->cnt+1ul; - ffactory = ffactory->nxt; - ffactory->obj = factory; - } + if (hd->state->node != NULL) + hd->state->node->obj = factory ? (void *)factory : FAUST_SLOT_FAILED; } - if(hd->hptr) - *hd->hptr = FL(ffactory->cnt); + csound->Free(csound, argv); csound->Free(csound, cmd); csound->Free(csound, hd->code); @@ -453,16 +432,92 @@ static uintptr_t init_faustcompile_thread_runner(void *pp) { return 0; } +/* Append a new (pending) entry to the ::factory registry and return it. Must + be called with faust_registry_mutex held. */ +static faustobj *reserve_factory_slot(CSOUND *csound) { + faustobj **pffactory, *ffactory, *fnew; + const char *varname = "::factory"; + + pffactory = (faustobj **)csound->QueryGlobalVariable(csound, varname); + if (pffactory == NULL) { + csound->CreateGlobalVariable(csound, varname, sizeof(faustobj *)); + pffactory = (faustobj **)csound->QueryGlobalVariable(csound, varname); + } + ffactory = *pffactory; + if (ffactory == NULL) { + fnew = (faustobj *)csound->Calloc(csound, sizeof(faustobj)); + fnew->obj = FAUST_SLOT_PENDING; + fnew->nxt = NULL; + fnew->cnt = 0ul; + *pffactory = fnew; + return fnew; + } + while (ffactory->nxt != NULL) + ffactory = ffactory->nxt; + fnew = (faustobj *)csound->Calloc(csound, sizeof(faustobj)); + fnew->obj = FAUST_SLOT_PENDING; + fnew->nxt = NULL; + fnew->cnt = ffactory->cnt + 1ul; + ffactory->nxt = fnew; + return fnew; +} + +/* Resolve a faustcompile handle to its registry entry, waiting for an + asynchronous compilation to publish the factory. Returns NULL (after + issuing an error) on failure. */ +static faustobj *resolve_factory(CSOUND *csound, int32_t handle) { + faustobj **fobjp, *fobj = NULL; + int timout = 0; + int failed = (handle < 0); + + if (!failed) { + fobjp = (faustobj **)csound->QueryGlobalVariable(csound, "::factory"); + if (fobjp == NULL) { + csound->InitError(csound, "%s", Str("no factory available\n")); + return NULL; + } + for (;;) { + { + std::lock_guard guard(faust_registry_mutex); + fobj = *fobjp; + while (fobj != NULL && (int32_t)fobj->cnt != handle) + fobj = fobj->nxt; + if (fobj == NULL || fobj->obj != FAUST_SLOT_PENDING) + break; + } + csound->Sleep(1); + if (++timout > 1000) { + csound->InitError(csound, "%s", Str("Faust code was not ready. Try compiling it \n" + "in a separate instrument prior to running it here\n")); + return NULL; + } + } + if (fobj == NULL) { + csound->InitError(csound, Str("factory not found %d\n"), handle); + return NULL; + } + failed = (fobj->obj == FAUST_SLOT_FAILED); + } + if (failed) { + csound->InitError(csound, "%s", Str("Faust code did not compile properly.\n" + "Check above messages for Faust compiler errors\n")); + return NULL; + } + return fobj; +} + int32_t init_faustcompile(CSOUND *csound, faustcompile *p) { faustcompile_state *st = (faustcompile_state *)csound->Malloc(csound, sizeof(faustcompile_state)); st->csound = csound; st->factory = NULL; st->thread = NULL; + st->node = NULL; /* Snapshot everything the asynchronous thread needs, so that it never touches the per-note opcode memory (which is freed when this instrument - ends). */ + ends). The thread publishes its result into st->node, never into the + output pointer. */ hdata *data = (hdata *)csound->Malloc(csound, sizeof(hdata)); data->csound = csound; data->code = csound->Strdup(csound, p->code->data); @@ -470,14 +525,23 @@ int32_t init_faustcompile(CSOUND *csound, faustcompile *p) { data->target = NULL; if (p->INOCOUNT > 4 && p->target != NULL && p->target->data != NULL) data->target = csound->Strdup(csound, p->target->data); - data->hptr = p->hptr; data->state = st; - *p->hptr = -1.0; + /* Reserve the registry slot and publish the handle while the opcode is + still alive, so that no write to per-note memory happens after the + instrument ends. */ + { + std::lock_guard guard(faust_registry_mutex); + st->node = reserve_factory_slot(csound); + } + *p->hptr = FL(st->node->cnt); if(!(int)*p->async) { /* synchronous compilation, run on the calling thread (large stack) */ init_faustcompile_thread(data); + /* Preserve the historical output convention for synchronous failure. */ + if (st->node->obj == FAUST_SLOT_FAILED) + *p->hptr = -2.0; } else { st->thread = csound->CreateThread(init_faustcompile_thread_runner, data); } @@ -532,41 +596,15 @@ int32_t delete_faustdsp(CSOUND *csound, void *p) { int32_t init_faustdsp(CSOUND *csound, faustdsp *p) { int32_t factory; - faustobj *fobj, **fobjp, **pfdsp, *fdsp; + faustobj *fobj, **pfdsp, *fdsp; llvm_dsp *dsp; controls *ctls = new controls(); const char *varname = "::dsp"; - int timout = 0; - while (*((cs_float *)p->code) == -1.0) { - csound->Sleep(1); - timout++; - if(timout > 1000) { - return csound->InitError( - csound, "%s", Str("Faust code was not ready. Try compiling it \n" - "in a separate instrument prior to running it here\n")); - } - } factory = (int32_t)*((cs_float *)p->code); - - if (factory == -2) - return csound->InitError( - csound, "%s", Str("Faust code did not compile properly.\n" - "Check above messages for Faust compiler errors\n")); - - fobjp = (faustobj **)csound->QueryGlobalVariable(csound, "::factory"); - if (fobjp == NULL) - return csound->InitError(csound, "%s", Str("no factory available\n")); - fobj = NULL; - { - std::lock_guard guard(faust_registry_mutex); - fobj = *fobjp; - while (fobj != NULL && (int32_t)fobj->cnt != factory) - fobj = fobj->nxt; - } + fobj = resolve_factory(csound, factory); if (fobj == NULL) - return csound->InitError(csound, Str("factory not found %d\n"), - (int32_t)factory); + return CSOUND_ERROR; dsp = ((llvm_dsp_factory *)fobj->obj)->createDSPInstance(); if (dsp == NULL) @@ -781,42 +819,15 @@ int32_t delete_faustgen(CSOUND *csound, void *p) { int32_t init_faustaudio(CSOUND *csound, faustgen *p) { int32_t factory; const OPARMS *parms = csound->GetOParms(csound); - faustobj *fobj, **fobjp, **pfdsp, *fdsp; + faustobj *fobj, **pfdsp, *fdsp; llvm_dsp *dsp; controls *ctls = new controls(); const char *varname = "::dsp"; - int timout = 0; - while (*((cs_float *)p->code) == -1.0) { - csound->Sleep(1); - timout++; - if(timout > 1000) { - return csound->InitError( - csound, "%s", Str("Faust code was not ready. Try compiling it \n" - "in a separate instrument prior to running it here\n")); - } - } - factory = (int32_t)*((cs_float *)p->code); - - if (factory == -2) - return csound->InitError( - csound, "%s", Str("Faust code did not compile properly.\n" - "Check above messages for Faust compiler errors\n")); - - fobjp = (faustobj **)csound->QueryGlobalVariable(csound, "::factory"); - if (fobjp == NULL) - return csound->InitError(csound, "%s", Str("no factory available\n")); - fobj = NULL; - { - std::lock_guard guard(faust_registry_mutex); - fobj = *fobjp; - while (fobj != NULL && (int32_t)fobj->cnt != factory) - fobj = fobj->nxt; - } + fobj = resolve_factory(csound, factory); if (fobj == NULL) - return csound->InitError(csound, Str("factory not found %d\n"), - (int32_t)factory); + return CSOUND_ERROR; dsp = ((llvm_dsp_factory *)fobj->obj)->createDSPInstance(); if (dsp == NULL)