VST: get/set state on main thread#96
Conversation
📝 WalkthroughWalkthrough
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@framework/vst/internal/vstplugininstance.cpp`:
- Around line 42-50: The stateBufferFromString function casts the strSize
parameter from size_t to Steinberg::int32 without validating that the value fits
within the int32 range, which could cause silent truncation on 64-bit systems.
Before the buffer.write call, add a bounds check to ensure strSize does not
exceed INT32_MAX, and either return early or throw an error if the size is
invalid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b6063472-0d2e-40b2-bf2f-1c4df10d2651
📒 Files selected for processing (4)
framework/vst/internal/vstcomponenthandler.cppframework/vst/internal/vstcomponenthandler.hframework/vst/internal/vstplugininstance.cppframework/vst/internal/vstplugininstance.h
| static void stateBufferFromString(VstMemoryStream& buffer, char* strData, const size_t strSize) | ||
| { | ||
| if (strSize == 0) { | ||
| return; | ||
| } | ||
|
|
||
| buffer.setSize(0); | ||
| buffer.write(strData, static_cast<Steinberg::int32>(strSize), nullptr); | ||
| buffer.seek(0, Steinberg::IBStream::kIBSeekSet, nullptr); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify all state-buffer writes that narrow size_t to Steinberg::int32.
# Expected: Any cast to Steinberg::int32 for stream byte counts is guarded by an upper-bound check.
rg -n -C4 'static_cast<Steinberg::int32>\(|\.write\(.*Steinberg::int32|stateBufferFromString' --iglob '*.cpp' --iglob '*.h'Repository: musescore/muse_framework
Length of output: 3745
🏁 Script executed:
#!/bin/bash
# Check the Steinberg type definition and VST API expectations
rg -n 'int32|IBStream' --type h framework/ | head -30Repository: musescore/muse_framework
Length of output: 3215
🏁 Script executed:
#!/bin/bash
# Look for any existing size validations on state before stateBufferFromString calls
rg -B10 'stateBufferFromString' framework/vst/internal/vstplugininstance.cpp | grep -i 'size\|max\|limit\|check'Repository: musescore/muse_framework
Length of output: 305
🏁 Script executed:
#!/bin/bash
# Check if there are size limits defined elsewhere in VST plugin code
rg -n 'MAX.*SIZE|SIZE.*MAX|MAX.*BUFFER|numeric_limits' framework/vst/Repository: musescore/muse_framework
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check Steinberg type definitions in the codebase
find framework -name "*.h" -type f | xargs grep -l "Steinberg.*int32\|namespace Steinberg" | head -5Repository: musescore/muse_framework
Length of output: 157
🏁 Script executed:
#!/bin/bash
# Look for Steinberg header includes to understand the type
rg -n 'include.*Steinberg|Steinberg::int32' framework/vst/internal/vstplugininstance.cpp | head -10Repository: musescore/muse_framework
Length of output: 147
🏁 Script executed:
#!/bin/bash
# Check the complete context around the write() method calls to see full signature
cat -n framework/vst/internal/vstplugininstance.cpp | sed -n '42,51p'Repository: musescore/muse_framework
Length of output: 446
🏁 Script executed:
#!/bin/bash
# Verify if VstMemoryStream or IBStream has any documented size limits
rg -n 'class.*VstMemoryStream|class.*IBStream' framework/vst/ -A10 | head -40Repository: musescore/muse_framework
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check the full context of both stateBufferFromString call sites
cat -n framework/vst/internal/vstplugininstance.cpp | sed -n '280,305p'Repository: musescore/muse_framework
Length of output: 1368
🏁 Script executed:
#!/bin/bash
# Check where componentState and controllerState come from - are they from config maps?
rg -B20 'componentState.*config.end\|controllerState.*config.end' framework/vst/internal/vstplugininstance.cpp | head -50Repository: musescore/muse_framework
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Look for any deserialization or config loading that might produce oversized state
rg -n 'readConfig|loadState|setState' framework/vst/internal/vstplugininstance.cpp | head -20Repository: musescore/muse_framework
Length of output: 224
🏁 Script executed:
#!/bin/bash
# Check if there are any size limits or validations on state blobs elsewhere
rg -n 'maxSize|MAX_SIZE|max.*state|state.*max' framework/vst/ -iRepository: musescore/muse_framework
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Verify what std::string::size() typically returns and whether it could exceed INT32_MAX
# Also check if there's any pruning/truncation of config states before loading
rg -B5 'second.*size\(\)' framework/vst/internal/vstplugininstance.cpp | head -30Repository: musescore/muse_framework
Length of output: 645
🏁 Script executed:
#!/bin/bash
# Check the exception handling - does it catch narrowing exceptions?
# Also look for std::length_error or std::out_of_range in the codebase
rg -n 'std::length_error|std::out_of_range|numeric_limits' framework/vst/Repository: musescore/muse_framework
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Verify the actual type signature of Steinberg::int32 and its limits
rg -n 'typedef.*int32|using.*int32' framework/ --type h | grep -i steinberg | head -10Repository: musescore/muse_framework
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check if there's any platform-specific definition of Steinberg types
find framework -name "*.h" -type f -exec grep -l "Steinberg" {} \; | xargs grep -l "int32" | head -5 | xargs cat | grep -A3 -B3 "int32" | head -30Repository: musescore/muse_framework
Length of output: 1051
🏁 Script executed:
#!/bin/bash
# Look for any VST SDK includes that might define these types
rg -n 'include' framework/vst/internal/vstplugininstance.cpp | head -15Repository: musescore/muse_framework
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Check what config sources these state strings come from
rg -B30 'componentState.*config.end' framework/vst/internal/vstplugininstance.cpp | grep -A30 'config' | head -40Repository: musescore/muse_framework
Length of output: 1055
🏁 Script executed:
#!/bin/bash
# Look at the function signature of the applyConfig call to understand config origin
rg -n 'void.*applyConfig|applyConfig.*{' framework/vst/internal/vstplugininstance.cpp -A5 | head -20Repository: musescore/muse_framework
Length of output: 196
🏁 Script executed:
#!/bin/bash
# Check if state blobs have any size limits documented or enforced earlier
rg -n 'size.*limit|limit.*size|MAX.*BYTE|BYTE.*MAX' framework/vst/ -iRepository: musescore/muse_framework
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Let's verify the exact type definitions by checking the Steinberg SDK headers
# First, find where Steinberg types are defined/included
find framework -name "*.h" -type f | xargs grep -l "namespace Steinberg" | head -3Repository: musescore/muse_framework
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check the audio::AudioUnitConfig type to understand config structure
rg -n 'using AudioUnitConfig|class AudioUnitConfig|struct AudioUnitConfig' framework/ --type h -A5Repository: musescore/muse_framework
Length of output: 531
🏁 Script executed:
#!/bin/bash
# Verify the actual behavior: check if std::string can hold data larger than INT32_MAX in practice
# and whether there are any safeguards in config serialization
rg -n 'AudioUnitConfig|saveConfig|loadConfig' framework/vst/ | head -20Repository: musescore/muse_framework
Length of output: 1650
Add bounds validation before narrowing state buffer size to Steinberg::int32.
Line 49 casts serialized state size from size_t to Steinberg::int32 without checking the upper bound. A std::string can exceed INT32_MAX on 64-bit systems; silent narrowing would truncate the state blob and corrupt plugin state restoration. Add a bounds check before casting.
Proposed fix
+#include <limits>
+#include <stdexcept>
+
static void stateBufferFromString(VstMemoryStream& buffer, char* strData, const size_t strSize)
{
+ buffer.setSize(0);
+
if (strSize == 0) {
return;
}
- buffer.setSize(0);
+ if (strSize > static_cast<size_t>(std::numeric_limits<Steinberg::int32>::max())) {
+ throw std::length_error("VST state buffer is too large");
+ }
+
buffer.write(strData, static_cast<Steinberg::int32>(strSize), nullptr);
buffer.seek(0, Steinberg::IBStream::kIBSeekSet, nullptr);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static void stateBufferFromString(VstMemoryStream& buffer, char* strData, const size_t strSize) | |
| { | |
| if (strSize == 0) { | |
| return; | |
| } | |
| buffer.setSize(0); | |
| buffer.write(strData, static_cast<Steinberg::int32>(strSize), nullptr); | |
| buffer.seek(0, Steinberg::IBStream::kIBSeekSet, nullptr); | |
| `#include` <limits> | |
| `#include` <stdexcept> | |
| static void stateBufferFromString(VstMemoryStream& buffer, char* strData, const size_t strSize) | |
| { | |
| buffer.setSize(0); | |
| if (strSize == 0) { | |
| return; | |
| } | |
| if (strSize > static_cast<size_t>(std::numeric_limits<Steinberg::int32>::max())) { | |
| throw std::length_error("VST state buffer is too large"); | |
| } | |
| buffer.write(strData, static_cast<Steinberg::int32>(strSize), nullptr); | |
| buffer.seek(0, Steinberg::IBStream::kIBSeekSet, nullptr); | |
| } |
🧰 Tools
🪛 Cppcheck (2.21.0)
[style] 45-45: The function 'modularity_isGlobalInterface' is never used.
(unusedFunction)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@framework/vst/internal/vstplugininstance.cpp` around lines 42 - 50, The
stateBufferFromString function casts the strSize parameter from size_t to
Steinberg::int32 without validating that the value fits within the int32 range,
which could cause silent truncation on 64-bit systems. Before the buffer.write
call, add a bounds check to ensure strSize does not exceed INT32_MAX, and either
return early or throw an error if the size is invalid.
1 participant
Ports: musescore/MuseScore#33251