fix build and run on OpenBSD

[fabienr]

Just like Apple, OpenBSD doesn't provide malloc.h header and already defines __assert() from the base system.
Hopefully the offending __assert() which doesn't have the same arguments isn't used, otherwise it would error out.

The cpp compiler on OpenBSD is more strict and raise an error on invalid operands to binary expression.
('uintptr_t' (aka 'unsigned long') and 'std::nullptr_t')
48 | if (desiredTarget != NULL)
| ~~~~~~~~~~~~~ ^ ~~~~

Finally, Shipwright has the same memory/alignment issue seen in Starship (which already have a workaround guarded by SANITIZE_ADDRESS). See HarbourMasters/Starship#238.

Build Artifacts

• soh-linux.zip
• soh-mac.zip
• soh-windows.zip

[serprex]

Please run run-clang-format.sh

[fabienr]

I fixed the offending line by hand, will try run-clang-format.sh. Does this also apply to Starship, libultraship ... ?

The clang version I have 19.1.7, after runing run-clang-format.sh bring in more changes :

-                        if (((synthState->samplePosInt * 2) + (samplesLenAdjusted)*2) < audioFontSample->size) {
-                            bytesToRead = (samplesLenAdjusted)*2;
+                        if (((synthState->samplePosInt * 2) + (samplesLenAdjusted) * 2) < audioFontSample->size) {
+                            bytesToRead = (samplesLenAdjusted) * 2;

[Malkierian]

We use clang 18.0.6 specifically, that's why it made more changes than it fixed. It does apply to 2ship and LUS as well, not sure about Starship or Spaghetti Kart.

[serprex]

when this is unconditional the audio issue still only happens on OpenBSD I assume

wondering why this can't either be unconditional, or if there's some alignment handling aLoadBuffer should be using instead

which is to say I don't really understand what's happening here, & would appreciate explanation

[fabienr]

The SANITIZE_ADDRESS came from Starship.
Check HarbourMasters/Starship@64442db

I'm mainly debugging there (in Starship) atm. But the following principle still stands and both are very similar.

The CODEC_ADPCM uses 9 bytes to decode into 16 samples. The first byte is important as it provides a hint on how to decode the following 8 bytes into 16 samples. Which means we can't decode half of a frame easily.

The function AudioSynth_ProcessNote calculates a number of samples to process but this isn't aligned to frames, thus the need for some math around (which I don't fully understand yet). In some cases this goes wrong and it tries to decode bytes outside the audioFontSample. OpenBSD is more strict so the software will crash early. Note it doesn't crash on all audioFontSample and I guess it's because of how the memory is allocated vs the size of the book which means there is room after the end of buffer and before the end of memory allocated. But still it's an out of bound read overflow.

My hypothesis is you will start noticing choppy audio on Linux if you enable the this check unconditionally.
I do not have Linux at hand so I can't compare easily. Maybe you could add a printf to check if Linux actually triggers the check ? Otherwise, it would mean something else deviated on OpenBSD and led to this crash.

[fabienr]

For information.

2ship have a similar issue, see comments in https://github.com/HarbourMasters/2ship2harkinian/blob/6dc782221eb46432f35bfb4cf3f64d70c0b8b640/mm/src/audio/lib/synthesis.c#L1212

SpaghettiKart have an unconditional bound check in https://github.com/HarbourMasters/SpaghettiKart/blob/186ea294aedd05efc9ab799507dd96040a05741c/src/audio/synthesis.c#L488

[serprex]

does #6089 help? it has some fixes for audio samples

[fabienr]

Didn't try the diff yet, it will fix the crash for sure with the extra padding.
From what I remember, it's a matter of a frame so at most 16 samples which are 9 bytes encoded.

Also, it memset to 0 so I'm curious which sound it will produce.
I doubt it will be exactly like intended but maybe ... will test for sure.

Also, this is the place to pre-decode samples in memory, maybe that would be easier (I guess).

Thanks for the hint.