neuronvs_patch

Stability patch for NeuronVS.vst v1.56 that fixes an intermittent SIGSEGV inside QuickDraw's CopyBits, reported by hosts (e.g. Logic Pro) as a SIGABRT. The bug is triggered when moving the Nuke joystick fast enough.

Tested under Snow Leopard 10.6.8 with Logic Pro 9.1.5, with NeuronVS wrapped with the FXPansion VST-AU adapter.

The bug

The USB hardware-remote servicing thread drives the GUI synchronously:

stDevReadCb -> CRemoteInfo::stStick -> CParam::SetVal -> CParam::NotifyVal
  -> CStickView::stNtf1Cb -> CDisplay::SetInt -> CDisplay::SetText
  -> CDisplayView::drawofl -> CBitmap::drawTransparent -> QD CopyBits

CDisplay::SetText sets an "offscreen dirty" flag and then, if the view is attached to a frame, immediately calls CDisplayView::drawofl(), which builds a draw context over the editor's CFrame and blits through QuickDraw. QuickDraw is not reentrant and its port state is global, so doing this from a non-UI thread races the main thread's own redraw. The attached-frame check is also a TOCTOU against CDisplay::SetFrame(NULL), which the main thread calls when the editor closes and tears down the frame/GWorld. Either way, CopyBits ends up reading a stale/invalid PixMap and dereferences a wild pointer.

Two independent fixes are applied to close this off. __TEXT is mapped 1:1 (vmaddr == file offset) and the bundle has no LC_CODE_SIGNATURE, so every offset below is a literal file offset.

Fix 1: move rendering onto the UI thread

CDisplay::SetText @ 0x44B3A — replace the if (frame) drawofl() tail with view->setDirty(true) (vtable+0x3C, CControl::setDirty). The dirty flag stays set. Any thread may now call SetText safely; it only touches fields, no QuickDraw involved.

CDisplayView::draw @ 0x44CEE — the UI-thread paint entry point. Its first 70 bytes are dead stores (zero-init of two stack temps that are fully overwritten in the blit branch and unused in the drawText branch), so the fix needs no code cave — it's prepended directly:

if (this->dirty_flag) drawofl(this);

drawofl clears the dirty flag, so it renders at most once per change, and draw()'s own setDirty(0) supersedes drawofl's trailing setDirty(1) — no repaint loop.

Net effect: identical visuals, but every QuickDraw call from this path now happens on the UI thread. The remaining USB-thread work is plain field writes.

Fix 2: guard both CopyBits call sites against a stale bitmap

CBitmap::draw and CBitmap::drawTransparent — the exact function named in the crash stack — both call CopyBits directly, passing their own pixel buffer as srcBits:

CopyBits(srcBits, dstBits, srcRect, dstRect, mode, maskRgn)

Classic QuickDraw BitMap/PixMap structs start with a Ptr baseAddr. If the CBitmap's backing GWorld has been purged or torn down, baseAddr is NULL — and CopyBits will happily dereference it and crash. Fix 1 above closes the main thread-affinity race, but doesn't stop a torn-down bitmap from being blitted if this code is ever reached another way, so this fix adds a direct null-check right at the call site.

Call site A — CBitmap::draw @ 0x179B2 (3 bytes, inside the 5-byte call at 0x179B1): the CopyBits call's relative-call operand is repointed from the _CopyBits symbol stub to the guard trampoline below.

Call site B — CBitmap::drawTransparent @ 0x18067 (3 bytes, inside the 5-byte call at 0x18066): same redirect, second CopyBits call site.

Guard trampoline @ 0x94263 (12 bytes) — both calls above set up srcBits in %edx right before pushing it as the first argument. The trampoline lives in what was 13 bytes of dead nop alignment padding immediately after _NASEngineProcess's retl (unreachable, safe to reuse) and reads:

mov  eax, [edx]      ; eax = srcBits->baseAddr
test eax, eax
jle  skip             ; baseAddr <= 0 (NULL/invalid) -> bail, no blit
jmp  _CopyBits         ; tail-call into the real CopyBits
skip:
ret                    ; caller's stack is untouched either way

Because it's a tail jmp rather than a call, the original caller's stack frame and return address are preserved exactly as if CopyBits had been called directly — the only behavioral change is that an invalid source bitmap is silently skipped instead of crashing.

Patch summary

#	Location	Offset	Len	What changes
1	CBitmap::draw	0x179B2	3	call _CopyBits -> call guard trampoline
2	CBitmap::drawTransparent	0x18067	3	call _CopyBits -> call guard trampoline
3	CDisplay::SetText	0x44B3A	24	synchronous drawofl() -> setDirty(true)
4	CDisplayView::draw	0x44CEE	70	prepend dirty-flag check -> drawofl()
5	code cave after _NASEngineProcess	0x94263	12	dead nops -> guard trampoline (target of 1 & 2)

All five regions are verified byte-for-byte before any write; the tool refuses to touch the file if anything doesn't match a known original or already-patched state.

Building

This patch was built under Linux/x86_64 using the osxcross macOS 10.6 cross toolchain:

export PATH=/data/osxcross/target/bin:$PATH
i386-apple-darwin10-gcc   -mmacosx-version-min=10.6 -O2 -o /tmp/np.i386   neuronvs_patch.c
x86_64-apple-darwin10-gcc -mmacosx-version-min=10.6 -O2 -o /tmp/np.x86_64 neuronvs_patch.c
lipo -create -output neuronvs_patch /tmp/np.i386 /tmp/np.x86_64

Produces a universal (i386 + x86_64) binary, neuronvs_patch.

Usage

./neuronvs_patch [VST_PATH_OR_BINARY]

• VST_PATH_OR_BINARY defaults to ./NeuronVS.vst. If it's a directory, the binary is looked up at <path>/Contents/MacOS/NeuronVS; if it's a regular file, it's used directly (so this also works on an already extracted/copied binary).
• The tool patches the file in place if it's found to be original, or reverts it in place if it's found to already be patched — running it twice toggles the fix on and off.
• If any of the 5 regions don't match a known original or patched state, it aborts with a hex diff and writes nothing.

Remove the plugin's cached scan in the host after patching, if it caches plugin binaries.