fix(amd64-386): sample oscillator hard crash

The sample-based oscillators converted the samplepos to an integer
and did samplepos < loop_end comparison to check if we are past
looping. Unfortunately, the < comparison was done in signed math.
Normally, this should never happen, but if the x87 FPU stack
overflowed exactly at right position, we then got 0x80000000 in
samplepos, which is equal to -2147483648. Thus, we considered that
sample is not looping and read the sample table at position
-2147483648, well out of bound. TL;DR changing jl to jb makes sure
we always wrap within to sample table, no matter what.

Fixes #149.

CHANGELOG.md

@@ -10,6 +10,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   the command line tools.
 
 ### Fixed
+- Sample-based oscillators could hard crash if a x87 stack overflow happened
+  when calculating the current position in the sample ([#149][i149])
 - Numeric updown widget calculated dp-to-px conversion incorrectly, resulting in
   wrong scaling ([#150][i150])
 - Empty patch should not crash the native synth ([#148][i148])
@@ -215,4 +217,5 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 [i145]: https://github.com/vsariola/sointu/issues/145
 [i146]: https://github.com/vsariola/sointu/issues/146
 [i148]: https://github.com/vsariola/sointu/issues/148
+[i149]: https://github.com/vsariola/sointu/issues/149
 [i150]: https://github.com/vsariola/sointu/issues/150

tests/CMakeLists.txt

@@ -181,3 +181,8 @@ target_compile_definitions(test_render_samples PUBLIC TEST_HEADER="test_render_s
 add_executable(test_render_samples_api test_render_samples_api.c)
 target_link_libraries(test_render_samples_api ${STATICLIB})
 add_test(test_render_samples_api test_render_samples_api)
+
+add_executable(test_oscillator_crash test_oscillator_crash.c)
+target_link_libraries(test_oscillator_crash ${STATICLIB})
+add_test(test_oscillator_crash test_oscillator_crash)
+

tests/test_oscillator_crash.c

@@ -0,0 +1,55 @@
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+#include <sointu.h>
+
+#define BPM 100
+#define SAMPLE_RATE 44100
+#define LENGTH_IN_ROWS 16
+#define SAMPLES_PER_ROW SAMPLE_RATE * 4 * 60 / (BPM * 16)
+const int su_max_samples = SAMPLES_PER_ROW * LENGTH_IN_ROWS;
+
+int main(int argc, char* argv[])
+{
+    Synth* synth;
+    float* buffer;
+    // The patch is invalid and overflows the stack. This should still exit cleanly, but used to hard crash.
+    // See: https://github.com/vsariola/sointu/issues/149
+    const unsigned char opcodes[] = { SU_OSCILLATOR_ID + 1, // STEREO                                     
+                                     SU_ADVANCE_ID };
+    const unsigned char operands[] = { 69, 74, 0, 0, 82, 128, 128 };
+    int errcode;
+    int time;
+    int samples;
+    int totalrendered;
+    int retval;
+    // initialize Synth
+    synth = (Synth*)malloc(sizeof(Synth));
+    memset(synth, 0, sizeof(Synth));
+    memcpy(synth->Opcodes, opcodes, sizeof(opcodes));
+    memcpy(synth->Operands, operands, sizeof(operands));
+    synth->NumVoices = 3;
+    synth->Polyphony = 6;
+    synth->RandSeed = 1;
+    synth->SampleOffsets[0].Start = 91507;
+    synth->SampleOffsets[0].LoopStart = 5448;
+    synth->SampleOffsets[0].LoopLength = 563;
+    // initialize Buffer
+    buffer = (float*)malloc(2 * sizeof(float) * su_max_samples);
+    // triger first voice
+    synth->SynthWrk.Voices[0].Note = 64;
+    synth->SynthWrk.Voices[0].Sustain = 1;
+    totalrendered = 0;
+    samples = su_max_samples;
+    time = INT32_MAX;
+    retval = 0;    
+    errcode = su_render(synth, buffer, &samples, &time);
+    if (errcode != 0x1041) {
+        retval = 1;
+        printf("su_render should have return errcode 0x1401, got 0x%08x\n", errcode);
+    }       
+    free(synth);
+    free(buffer);
+    return retval;
+}

vm/compiler/templates/amd64-386/sources.asm

@@ -338,7 +338,7 @@ su_oscillat_gate_bit:                           ; stack: 0/1, let's call it x
     pop     {{.DX}}                                             ; edx is now the sample number
     movzx   ebx, word [{{.DI}} + 4]    ; ecx = loopstart
     sub     edx, ebx                                        ; if sample number < loop start
-    jl      su_oscillat_sample_not_looping                  ;   then we're not looping yet
+    jb      su_oscillat_sample_not_looping                  ;   then we're not looping yet
     mov     eax, edx                                        ; eax = sample number
     movzx   ecx, word [{{.DI}} + 6]   ; edi is now the loop length
     xor     edx, edx                                        ; div wants edx to be empty