VST2 chunk save/load possible bug

[sadko4u]

Using Carla built from develop branch, commit 8385b5e

When saving to CARXP file, plugin forms the following BLOB for chunk data:

[TRC][vst.cpp: 316] vst_dispatcher: vst_dispatcher effect=0x562dadf8e780, opcode=23 (effGetChunk), index=0, value=0, ptr=0x7ffd7e8036a8, opt = 0.000
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 915] serialize_state: Serializing port id=bypass
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 915] serialize_state: Serializing port id=mode
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 915] serialize_state: Serializing port id=ramp
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 915] serialize_state: Serializing port id=samp
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 915] serialize_state: Serializing port id=m
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 915] serialize_state: Serializing port id=cm
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 915] serialize_state: Serializing port id=t
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 915] serialize_state: Serializing port id=time
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 915] serialize_state: Serializing port id=dry
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 915] serialize_state: Serializing port id=wet
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 915] serialize_state: Serializing port id=g_out
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 841] dump_vst_bank: Chunk dump:
00000000: 43 63 6e 4b 00 00 01 20 46 42 43 68 00 00 00 01    CcnK... FBCh....
00000010: 6a 61 76 38 00 00 07 d0 00 00 00 00 00 00 00 00    jav8............
00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
00000040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
00000050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
00000060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
00000070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
00000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
00000090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 88    ................
000000a0: 00 00 00 0b 62 79 70 61 73 73 00 00 00 00 00 00    ....bypass......
000000b0: 00 00 09 6d 6f 64 65 00 00 00 00 00 00 00 00 09    ...mode.........
000000c0: 72 61 6d 70 00 00 00 00 00 00 00 00 09 73 61 6d    ramp.........sam
000000d0: 70 00 43 18 00 00 00 00 00 06 6d 00 42 b4 00 00    p.C.......m.B...
000000e0: 00 00 00 07 63 6d 00 42 78 00 00 00 00 00 06 74    ....cm.Bx......t
000000f0: 00 41 a0 00 00 00 00 00 09 74 69 6d 65 00 41 f9    .A.......time.A.
00000100: 99 c4 00 00 00 08 64 72 79 00 00 00 00 00 00 00    ......dry.......
00000110: 00 08 77 65 74 00 3f 80 00 00 00 00 00 0a 67 5f    ..wet.?.......g_
00000120: 6f 75 74 00 3f 80 00 00                            out.?...        
[TRC][vst.cpp: 316] vst_dispatcher: vst_dispatcher effect=0x562dadf8e780, opcode=45 (effGetEffectName), index=0, value=0, ptr=0x7ffd7e8039e0, opt = 0.000
[TRC][vst.cpp: 349] vst_dispatcher: product_string = Delay Compensator Mono
[TRC][vst.cpp: 316] vst_dispatcher: vst_dispatcher effect=0x562dadf8e780, opcode=15 (effEditClose), index=0, value=0, ptr=(nil), opt = 0.000

When trying to load the same CARXP file, we get the following dump:

[TRC][vst.cpp: 316] vst_dispatcher: vst_dispatcher effect=0x55adb885def0, opcode=24 (effSetChunk), index=0, value=88, ptr=0x55adb8a4c980, opt = 0,000
[TRC][/home/sadko/eclipse/lsp-plugins/include/container/vst/wrapper.h: 841] dump_vst_bank: Chunk dump:
00000000: 00 00 00 0b 62 79 70 61 73 73 00 00 00 00 00 00    ....bypass......
00000010: 00 00 09 6d 6f 64 65 00 00 00 00 00 00 00 00 09    ...mode.........
00000020: 72 61 6d 70 00 00 00 00 00 00 00 00 09 73 61 6d    ramp.........sam
00000030: 70 00 43 18 00 00 00 00 00 06 6d 00 42 b4 00 00    p.C.......m.B...
00000040: 00 00 00 07 63 6d 00 42 78 00 00 00 00 00 06 74    ....cm.Bx......t
00000050: 00 41 a0 00 00 00 00 00 09 74 69 6d 65 00 41 f9    .A.......time.A.
00000060: 99 c4 00 00 00 08 64 72 79 00 00 00 00 00 00 00    ......dry.......
00000070: 00 08 77 65 74 00 3f 80 00 00 00 00 00 0a 67 5f    ..wet.?.......g_
00000080: 6f 75 74 00 3f 80 00 00 31 00 00 00 00 00 00 00    out.?...1.......
00000090: 78 ec 9d 42 4a 7f 00 00 78 ec 9d 42 4a 7f 00 00    x..BJ^?..x..BJ^?..
000000a0: 10 74 90 b8 ad 55 00 00 88 ec 9d 42 4a 7f 00 00    .t...U.....BJ^?..
000000b0: 30 00 00 00 00 00 00 00 30 00 00 00 00 00 00 00    0.......0.......
000000c0: 01 00 00 00 01 00 00 00 02 00 00 00 00 00 00 00    ................
000000d0: 18 00 00 00 00 00 00 00 19 00 00 00 00 00 00 00    ................
000000e0: 48 ca a4 b8 ad 55 00 00 31 00 00 00 00 00 00 00    H....U..1.......
000000f0: 01 00 00 00 01 00 00 00 02 00 00 00 00 00 00 00    ................
00000100: 18 00 00 00 00 00 00 00 0f 00 00 00 00 00 00 00    ................
00000110: 30 00 00 00 00 00 00 00 81 00 00 00 00 00 00 00    0...............
00000120: c0 ff 95 b8 ad 55 00 00 e0 21 97 b8 ad 55 00 00    .....U...!...U..
00000130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
00000140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................

And then, the plugin crashes because we improperly determine the size of the chunk and, as a result, try to dump a huge block of memory and finally become SIGSEGV.

Filipe, need your comments here about such behaviour.

[falkTX]

This is happening because the state you are saving matches exactly what the juce plugin wrapper does.
For compatibility reasons (carla can be build with or without juce, default is without), there is a check for juce-like chunks and a subsequent convert of the blob data (basically offsetting the initial bits where data is saved)
See https://github.com/falkTX/Carla/blob/develop/source/backend/plugin/CarlaPluginVST2.cpp#L2563

I understand that is just the way VST banks are saved, I guess you like to make things simpler and save in that format.
There should be a way to detect this though...

[sadko4u]

Hmm, @falkTX, I'm just saving a chunk according the true VST2.x specification:

https://github.com/sadko4u/lsp-plugins/blob/543e17e9de3b8dc9de6e31438d1223d792054626/include/3rdparty/steinberg/vst2.h#L2459-L2530

I know, JUCE is a piece of shit, but not only JUCE plugins do follow specifications when saving data to a chunk...

[falkTX]

I know, and understand that well, but in my testing I did not find a plugin that crashed due to this.
Yours is the first one I see doing that.

One thing I see is that juce never sets the byteSize parameter, so we can use that.
But to be more future proof, I can make it so that saved state uses a custom code not covered by the spec.

[falkTX]

Please try 8e37bc8 and let me know if that fixes it for you

[sadko4u]

@falkTK, I'll look again to official vst2sdk.
Here it is:

And:

So, actually, JUCE violates the SDK rules.

[falkTX]

well, juce actually puts the vst bank thing on top of the plugin state.
It is quite likely that some plugins (that dont use juce, like yours) will end up having double vst bank structure saved as binary blob. first from juce, then the actual plugin state that has vst bank struct.

from what I see, this was all done in juce for convenience. so that a save of the state ends up being as a vst preset format, whatever the plugin uses that internally or not.
or maybe not, with juce who knows...

anyway, as I see in https://github.com/falkTX/Carla/blob/develop/source/modules/juce_audio_processors/format_types/juce_VSTPluginFormat.cpp#L1270 the byteSize is always zero, and in that file that value is never changed to anything else.
so it is a good way to detect.
new saved state in carla will use a different code to make sure there wont be more conflicts in the future. but I have to keep the check for the old, more correct code, for backwards compatibility.

and so... does that work for you? does your plugin state load now?

[sadko4u]

Yes, this hack worked for me. But I'm also currently thinking about adding a workaround because maybe not only Carla will have such issues with VST.
What I want to do, is adding a CcnK and FBCh signatures detection to my state deserialization code.

[falkTX]

maybe not only Carla will have such issues with VST.

oh right, there might be issues in juce based hosts if they try to load a vst preset from you :)

in any case, thanks for confirming, I am closing this ticket.

[sadko4u]

From my side, I've done a couple of hacks, too:
lsp-plugins/lsp-plugins@e51b742
I was forced to implement version 3 serialization data structure for storing proper value of chunk size.