Skip to content

fix build and run on OpenBSD #6050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
serprex merged 5 commits into from
Jan 15, 2026
+3 −3
Merged

fix build and run on OpenBSD #6050

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0e6003e
On OpenBSD the game crashes. Check sample size and adjust aligned acc…
fabienr Dec 28, 2025
6b652b7
fix build on OpenBSD
fabienr Dec 28, 2025
5fd2b24
fix invalid operands to binary expression
fabienr Dec 28, 2025
4105163
please clang-format
fabienr Dec 30, 2025
04a6925
Revert "On OpenBSD the game crashes. Check sample size and adjust ali…
fabienr Jan 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion soh/include/functions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ u32 Locale_IsRegionNative(void);
void _assert(const char* exp, const char* file, s32 line);
#elif defined(__linux__)
void __assert(const char* exp, const char* file, s32 line) __THROW;
#elif !defined(__APPLE__) && !defined(__SWITCH__)
#elif !defined(__APPLE__) && !defined(__SWITCH__) && !defined(__OpenBSD__)
void __assert(const char* exp, const char* file, s32 line);
#endif
#if defined(__APPLE__) && defined(NDEBUG)
Expand Down
2 changes: 1 addition & 1 deletion soh/soh/GbiWrap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ extern "C" void gSPSegment(void* value, int segNum, uintptr_t target) {
if (res) {
uintptr_t desiredTarget = (uintptr_t)ResourceMgr_LoadIfDListByName(imgData);

if (desiredTarget != NULL)
if (desiredTarget)
target = desiredTarget;
}

Expand Down
2 changes: 1 addition & 1 deletion soh/src/buffers/heaps.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include "z64.h"
#include <assert.h>
#ifndef __APPLE__
#if !defined(__APPLE__) && !defined(__OpenBSD__)
#include <malloc.h>
#endif
#include <stdlib.h>
Expand Down
8 changes: 8 additions & 0 deletions soh/src/code/audio_synthesis.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,14 @@ Acmd* AudioSynth_ProcessNote(s32 noteIndex, NoteSubEu* noteSubEu, NoteSynthesisS
aligned = ALIGN16((nFramesToDecode * frameSize) + 16);
addr = DMEM_COMPRESSED_ADPCM_DATA - aligned;

#if __SANITIZE_ADDRESS__ || defined(__OpenBSD__)
Copy link
Contributor

@serprex serprex Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@fabienr fabienr Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@fabienr fabienr Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@serprex serprex Jan 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@fabienr fabienr Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@fabienr fabienr Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I revert SANITIZE_ADDRESS stuff.

Recently I realised the aLoadBufferImpl use ROUND_DOWN_16.

Shipwright/soh/soh/mixer.c

Line 103 in a92f07e

memcpy(BUF_U8(dest_addr), source_addr, ROUND_DOWN_16(nbytes));

All games should at least align to 16 when allocating ADPCM bytecode.
I have no idea why there is that constraint on alignment.

From my ears it looks better now (less choppy) but still not correct (noisy, feels like choppy).

There is also skipInitialSamples which may push the buffer too far. I have no idea why it exists at start.
The extra 32 padding in AudioSampleFactory.cpp may not be needed.

Shipwright/soh/src/code/audio_synthesis.c

Line 883 in a92f07e

frameIndex = (synthState->samplePosInt + skipInitialSamples - nFirstFrameSamplesToIgnore) / 16;

From my ears it doesn't seem better with the bellow change.

-                    frameIndex = (synthState->samplePosInt + skipInitialSamples - nFirstFrameSamplesToIgnore) / 16;
+                    frameIndex = synthState->samplePosInt / 16;

I think I need to record and compare on another OS.
I mean I need to narrow why the sound isn't as good as it should be (compared to emulation).

Copy link
Contributor

@serprex serprex Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, these basic fixes are good, the audio stuff is better in its own PR when there's a better handle as to what's going on

uintptr_t actualAddrLoaded = sampleData - sampleDataStartPad;
uintptr_t offset = actualAddrLoaded - (uintptr_t)sampleAddr;
if (offset + aligned > audioFontSample->size) {
aligned -= (offset + aligned - audioFontSample->size);
}
#endif

aLoadBuffer(cmd++, sampleData - sampleDataStartPad, addr, aligned);
} else {
nSamplesToDecode = 0;
Expand Down
Loading