surge-synthesizer / surge

Synthesizer plug-in (previously released as Vember Audio Surge)
https://surge-synthesizer.github.io/
GNU General Public License v3.0
3.15k stars 400 forks source link

A Conversation about building on PI (was specific ASM error) #1481

Closed Baggypants closed 4 years ago

Baggypants commented 4 years ago

Describe the bug Compiling surge on the raspberry pi using the https://github.com/nemequ/simde library to translate sse2 instructions to NEON fails at

root@zynthian:~/surge# ./build-linux.sh build --project=lv2

Building surge-lv2 with output in build_logs/build_lv2.log
==== Building surge-lv2 (release_x64) ====
Running prebuild commands
python scripts/linux/emit-vector-piggy.py .
ConfigurationXml.S
src/linux/ConfigurationXml.S: Assembler messages:
src/linux/ConfigurationXml.S:8: Error: unrecognized symbol type ""
make[1]: *** [surge-lv2.make:457: obj/x64/Release/surge-lv2/ConfigurationXml.o] Error 1
make: *** [Makefile:46: surge-lv2] Error 2

** Build of lv2 failed**
src/linux/ConfigurationXml.S:8: Error: unrecognized symbol type ""
make[1]: *** [surge-lv2.make:457: obj/x64/Release/surge-lv2/ConfigurationXml.o] Error 1
make: *** [Makefile:46: surge-lv2] Error 2

** Exiting failed lv2 build**
Complete information is in build_logs/build_lv2.log

To Reproduce Steps to reproduce the behavior:

  1. Compile Premake5 for arm
  2. git clone --recursive https://gitlab.com/Baggypants/surge.git forked from surge-synthesizer/surge
  3. ./build-linux.sh build --project=lv2
  4. remove all the -m64 -msse2 from surge-lv2.make
  5. ./build-linux.sh build --project=lv2

Expected behavior A totally fully compiled arm version with no errors!

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

baconpaul commented 4 years ago

so that file is a file we eject from python which contains some built in binary assets as gcc assembly. A few questions

  1. Does the file look like it is assembly in your environment?
  2. will that assembly compile on arm?

If the answer to 1 is no, then the python script has gone wrong; if the answer to 2 is no then we need to modify the python script for arm.

I don't have a pi (heck in fact right now I don't even have a linux vm running! But that's temporary) but am happy to offer suggestions on GitHub as you get it debugged!

baconpaul commented 4 years ago

Oh and am happy to merge changes needed to make it work as long as they are properly isolated from non-ARM builds too

Baggypants commented 4 years ago

Well, I'm on the outer reaches of my headspace building this. I normally package easy C programs for x86_64. I can say the file contents are this:

root@zynthian:~/surge# cat src/linux/ConfigurationXml.S
    .section ".rodata", "a"

configurationXmlStart:
    .globl configurationXmlStart
    .incbin "../../resources/data/configuration.xml"
configurationXmlEnd:
    .global configurationXmlNullTerminator
    .type configurationXmlNullTerminator, @object
configurationXmlNullTerminator:
    .int 0

And that might look like assembler?

baconpaul commented 4 years ago

yeah that is the assembly input. I know literally zero about building assembly sections on raspberry PI. This may be tricky.

does as -o x.o src/linux/ConfigurationXml.S or gcc -c src/linux/ConfigurationXml.S do anything for you?

Baggypants commented 4 years ago

The incbin file is referenced as a relative path .incbin "../../resources/data/configuration.xml" so I needed to be working in src/linux the output for both is as follows

root@zynthian:~/surge/src/linux# as -o x.o ConfigurationXml.S
ConfigurationXml.S: Assembler messages:
ConfigurationXml.S:8: Error: unrecognized symbol type ""
root@zynthian:~/surge/src/linux# gcc -c ConfigurationXml.S
ConfigurationXml.S: Assembler messages:
ConfigurationXml.S:8: Error: unrecognized symbol type ""

It doesn't seem to like line 8? .type configurationXmlNullTerminator, @object

that @object looks suspicious to me.

Baggypants commented 4 years ago

Random googling leads me to https://stackoverflow.com/questions/36778054/error-unrecognized-symbol-type#36778651

In the ARM syntax, @ is a comment marker, so as the assembler is cryptically telling you, it sees an empty symbol type and barfs on the incomplete directive.

Baggypants commented 4 years ago

Changing the @ to a % seems to let me continue the build.

baconpaul commented 4 years ago

OK! Good luck! I’m excited to see how it all comes together!

if you can get it working, I’m happy to merge changes like I said.

You are compiling directly on a pi? Or cross compiling on a linux machine?

Baggypants commented 4 years ago

I'm compiling directly on a Pi 4. Currently I've hit another issue with simde https://github.com/nemequ/simde/issues/71#issuecomment-574898247

baconpaul commented 4 years ago

Oh I'm almost 99% sure that's ancient code we don't need. We don't even use it on the au and in the retests. I would just comment it out, but to be safe just keep it around and put the implementation of ::set and ::restore in an "if ARM" block or some such for now (or just ifdef it out in your local copy).

Baggypants commented 4 years ago

Commenting out does for now. I've 250ish changed lines so far, merging this back in is going to be a mission!

Baggypants commented 4 years ago

FpuState came back to bite me

Creating target/lv2/Release/Surge.lv2
Linking surge-lv2
/usr/bin/ld: obj/x64/Release/surge-lv2/SurgeLv2Wrapper.o: in function `SurgeLv2Wrapper::run(void*, unsigned int)':
SurgeLv2Wrapper.cpp:(.text._ZN15SurgeLv2Wrapper3runEPvj+0x28): undefined reference to `FpuState::set()'
/usr/bin/ld: SurgeLv2Wrapper.cpp:(.text._ZN15SurgeLv2Wrapper3runEPvj+0x164): undefined reference to `FpuState::restore()'
collect2: error: ld returned 1 exit status
make[1]: *** [surge-lv2.make:203: target/lv2/Release/Surge.lv2/Surge.so] Error 1
make: *** [Makefile:46: surge-lv2] Error 2

** Build of lv2 failed**
tinyxmlerror.cpp
collect2: error: ld returned 1 exit status
make[1]: *** [surge-lv2.make:203: target/lv2/Release/Surge.lv2/Surge.so] Error 1
make: *** [Makefile:46: surge-lv2] Error 2

** Exiting failed lv2 build**
Complete information is in build_logs/build_lv2.log
Baggypants commented 4 years ago

Ok, I commented out some stuff in SurgeLv2Wrapper.*

root@zynthian:~/surge# git diff src/lv2/SurgeLv2Wrapper.*
diff --git a/src/lv2/SurgeLv2Wrapper.cpp b/src/lv2/SurgeLv2Wrapper.cpp
index 16b03fe..72fdd6b 100644
--- a/src/lv2/SurgeLv2Wrapper.cpp
+++ b/src/lv2/SurgeLv2Wrapper.cpp
@@ -101,7 +101,7 @@ void SurgeLv2Wrapper::run(LV2_Handle instance, uint32_t sample_count)
    SurgeSynthesizer* s = self->_synthesizer.get();
    double sampleRate = self->_sampleRate;

-   self->_fpuState.set();
+   //self->_fpuState.set();

    for (unsigned pNth = 0; pNth < n_total_params; ++pNth)
    {
@@ -194,7 +194,7 @@ void SurgeLv2Wrapper::run(LV2_Handle instance, uint32_t sample_count)
       event = SurgeLv2::popNextEvent(eventSequence, &eventIter);
    }

-   self->_fpuState.restore();
+   //self->_fpuState.restore();
 }

 void SurgeLv2Wrapper::handleEvent(const LV2_Atom_Event* event)
diff --git a/src/lv2/SurgeLv2Wrapper.h b/src/lv2/SurgeLv2Wrapper.h
index 670cd20..d53e6b4 100644
--- a/src/lv2/SurgeLv2Wrapper.h
+++ b/src/lv2/SurgeLv2Wrapper.h
@@ -1,7 +1,7 @@
 #pragma once
 #include "SurgeSynthesizer.h"
 #include "AllLv2.h"
-#include "util/FpuState.h"
+//#include "util/FpuState.h"
 #include <memory>

 class SurgeLv2Ui;
@@ -74,7 +74,7 @@ private:
    double _sampleRate = 44100.0;

    uint32_t _blockPos = 0;
-   FpuState _fpuState;
+   //FpuState _fpuState;

    double _timePositionSpeed = 0.0;
    double _timePositionTempoBpm = 0.0;

It compiles! Then explodes in the postbuild.

Linking surge-lv2
Running postbuild commands
python scripts/linux/generate-lv2-ttl.py target/lv2/Release/Surge.lv2/Surge.so
make[1]: *** [surge-lv2.make:204: target/lv2/Release/Surge.lv2/Surge.so] Bus error
make[1]: *** Deleting file 'target/lv2/Release/Surge.lv2/Surge.so'
make: *** [Makefile:46: surge-lv2] Error 2

** Build of lv2 failed**
make[1]: *** [surge-lv2.make:204: target/lv2/Release/Surge.lv2/Surge.so] Bus error
make: *** [Makefile:46: surge-lv2] Error 2

** Exiting failed lv2 build**
Complete information is in build_logs/build_lv2.log
baconpaul commented 4 years ago

We can figure out a merge strategy if it works I’m sure

So the python os loading the dll and calling one function. Just run that python command under gdb and see where the error is would be my advice.

baconpaul commented 4 years ago

Also thinking: if you build and run the headless (which uses the cmake build path) you will get the unit tests running; that may be an easier step 0? Loads no ui code so is a far smaller footprint

Baggypants commented 4 years ago

The headless does compile cleanly but doesn't run:

(gdb) run
Starting program: /usr/bin/Surge-Headless/Surge/Surge-Headless
/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_IN.UTF-8)
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".

Program received signal SIGBUS, Bus error.
0x00087dac in simde_mm_set_ps ()
(gdb) bt
#0  0x00087dac in simde_mm_set_ps ()
#1  0x0008af68 in HalfRateFilter::set_coefficients(float*, float*) ()
#2  0x0008b07c in HalfRateFilter::load_coefficients() ()
#3  0x00087f6c in HalfRateFilter::HalfRateFilter(int, bool) ()
#4  0x000c82ec in SurgeSynthesizer::SurgeSynthesizer(HeadlessPluginLayerProxy*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) ()
#5  0x00111cfc in Surge::Headless::createSurge(int) ()
#6  0x0013c490 in ____C_A_T_C_H____T_E_S_T____0() ()
#7  0x0012c86c in Catch::TestInvokerAsFunction::invoke() const ()
#8  0x0012bf0c in Catch::TestCase::invoke() const ()
#9  0x00126b98 in Catch::RunContext::invokeActiveTestCase() ()
#10 0x00126978 in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) ()
#11 0x00125760 in Catch::RunContext::runTest(Catch::TestCase const&) ()
#12 0x00127ef8 in Catch::(anonymous namespace)::TestGroup::execute() ()
#13 0x00129090 in Catch::Session::runInternal() ()
#14 0x00128d7c in Catch::Session::run() ()
#15 0x00188b4c in int Catch::Session::run<char>(int, char const* const*) ()
#16 0x0015c904 in runAllTests(int, char**) ()
#17 0x00110b0c in main ()
baconpaul commented 4 years ago

Ok so that’s good

My guess is that the memory isn’t aligned on your pi because our alignment directives for Linux don’t generate the right thing on your architecture

If you print out the address of that pointer is it at the right spot in memory?

Baggypants commented 4 years ago

I've no idea how to print the address of a pointer or confirm it's at the right spot in memory, sorry.

baconpaul commented 4 years ago

Oh ... well then this could be tricky.

Basically you need to find the arg of the set and make sure it is 16 but aligned by doing a Cout if the &mem. But the step after that involves figuring out alignment directives on the pi which would require sse and c++ debugging skills.

Hmm ... my surge time now is really on the push to 1.6.5 and I don’t even have a pi 4 (but you can get one like 50 bucks or something right?) so not quite sure what to suggest. Wonder if someone on the other thread can jump in and help with some debugging too?

I’m imoressed how far you got by the way. The rack plugins I know build on pi so it is possible.

Baggypants commented 4 years ago

I'll do some reading up. How hard can debugging C++ be? :laughing: Thanks for the time and all the best for 1.6.5. A pi is cheap, my best advice is make sure you get a decent psu.

baconpaul commented 4 years ago

Ha ha! That's a great attitude

So here's the thing I suspect. For SSE2 to work information has to be aligned on 16 byte boundaries. So if you look at src/common/vt_dsp/halfratefilter.h you see it is declared as

class alignas(16) HalfRateFilter
{

this means these things will only align on 16 byte boundaries if your compiler is obeying the alignas directive.

The first item in the structure is "va" so in vt_dsp/halfratefilter.cpp in the constructor put something like

printf( "ADDRESS IS %x %d", &(va[0]), &(va[0] % 16 ) )

the first will be the hex address of va[0] and the second will be zero if you are aligned

alignas is as of c++-11 but on 32 bit systems I've seen it not working even at higher versions. So googling some 'arm alignas' stuff may also help.

HTH!

nemequ commented 4 years ago

I think you're right about alignment being the issue, but honestly I'm having trouble seeing how. The current implementation of simde_mm_set_ps looks pretty safe to me; all we do is assign the arguments to an aligned array, then load that. And it looks like all you do there with the return value is set some __m128 values, which should also be 16-byte aligned.

SIMDe uses a SIMDE_ALIGN macro which has slightly wider support than the C++11 alignof operator:

#if \
  HEDLEY_HAS_ATTRIBUTE(aligned) || \
  HEDLEY_GCC_VERSION_CHECK(2,95,0) || \
  HEDLEY_CRAY_VERSION_CHECK(8,4,0) || \
  HEDLEY_IBM_VERSION_CHECK(11,1,0) || \
  HEDLEY_INTEL_VERSION_CHECK(13,0,0) || \
  HEDLEY_PGI_VERSION_CHECK(19,4,0) || \
  HEDLEY_ARM_VERSION_CHECK(4,1,0) || \
  HEDLEY_TINYC_VERSION_CHECK(0,9,24) || \
  HEDLEY_TI_VERSION_CHECK(8,1,0)
#  define SIMDE_ALIGN(alignment) __attribute__((aligned(alignment)))
#elif defined(_MSC_VER) && !(defined(_M_ARM) && !defined(_M_ARM64))
#  define SIMDE_ALIGN(alignment) __declspec(align(alignment))
#elif defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L)
#  define SIMDE_ALIGN(alignment) _Alignas(alignment)
#elif defined(__cplusplus) && (__cplusplus >= 201103L)
#  define SIMDE_ALIGN(alignment) alignas(alignment)
#else
#  define SIMDE_ALIGN(alignment)
#endif

I also went ahead and added a static assertion to check the alignment of the types, and it didn't cause any failures. I haven't bothered supporting quite as many compilers for that macro (I'll try to come back at some point and do so), but it should work for most compilers.

baconpaul commented 4 years ago

https://community.vcvrack.com/t/surge-a-new-module-collection-from-a-great-digital-synth/3503/28?u=baconpaul

OK that person was able to make surge rack (which is basically the same as surge headless) build on arm so it is possible. Again, I don't know what or how but perhaps there's a lead?

nemequ commented 4 years ago

I believe AArch64 with NEON should work perfectly; that has been compiling and passing all SIMDe tests for a while. Intel and ARM have both been getting a lot more forgiving about unaligned access over time, it's just ARM is probably about 15 years behind Intel, but with AArch64 ARM has largely caught up.

For 32 bit I think clang 9 will work (all SIMDe tests pass on F31), and likely also GCC 9 (the only failures in the SIMDe tests are in AVX and later, and AFAICT surge only requires SSE2).

I've added a lot of NEON acceleration since June, so maybe it was working because it was pretty much exclusively hitting the portable fallbacks; if you don't enable NEON support in the compiler that's what should happen, and I'm pretty sure that will work on any compiler.

So basically the problem is trying to get the NEON-accelerated version working on 32-bit ARM. I'm going to be trying to get all of the tests for that in SIMDe to pass, once that's done there is a pretty good chance surge will just work, but even if not the problem should be much more tractable.

baconpaul commented 4 years ago

so my experience is only with intel - but when I started building surge 1.6 vintage with 32 bit windows, the c++ alignment macros didn't work properly and I got immediate segv. I had to move the clang compiler up to c++-17 in 32 bit with visual studio to get non-coring aligned SSE.

I am well out of my area of expertise here on how to make this work with non-intel platforms though.

You are correct, surge only requires SSE2.

Baggypants commented 4 years ago

I am well out of my area of expertise here

I'm familiar with the feeling. Meanwhile: I had a go at compiling with clang by setting export CXX=/usr/bin/clang++-9 and it throws out.

Building surge-headless with output in build_logs/build_headless.log
mkdir: cannot create directory 'build': File exists
-- The CXX compiler identification is Clang 9.0.0
-- The ASM compiler identification is Clang
-- Found assembler: /usr/bin/clang++-9
-- Check for working CXX compiler: /usr/bin/clang++-9
-- Check for working CXX compiler: /usr/bin/clang++-9 -- broken
CMake Error at /usr/share/cmake-3.13/Modules/CMakeTestCXXCompiler.cmake:45 (message):
  The C++ compiler

    "/usr/bin/clang++-9"

  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: /root/surge/build/CMakeFiles/CMakeTmp

    Run Build Command:"/usr/bin/make" "cmTC_aab62/fast"
    /usr/bin/make -f CMakeFiles/cmTC_aab62.dir/build.make CMakeFiles/cmTC_aab62.dir/build
    make[1]: Entering directory '/root/surge/build/CMakeFiles/CMakeTmp'
    Building CXX object CMakeFiles/cmTC_aab62.dir/testCXXCompiler.cxx.o
    /usr/bin/clang++-9    -mcpu=cortex-a53 -mtune=cortex-a53 -mfpu=neon-fp-armv8 -mneon-for-64bits -mfloat-abi=hard -mlittle-endian -munaligned-access -mvectorize-with-neon-quad -ftree-vectorize    -o CMakeFiles/cmTC_aab62.dir/testCXXCompiler.cxx.o -c /root/surge/build/CMakeFiles/CMakeTmp/testCXXCompiler.cxx
    clang: error: unknown argument: '-mneon-for-64bits'
    clang: error: unknown argument: '-mvectorize-with-neon-quad'
    make[1]: *** [CMakeFiles/cmTC_aab62.dir/build.make:66: CMakeFiles/cmTC_aab62.dir/testCXXCompiler.cxx.o] Error 1
    make[1]: Leaving directory '/root/surge/build/CMakeFiles/CMakeTmp'
    make: *** [Makefile:121: cmTC_aab62/fast] Error 2

  CMake will not be able to correctly generate this project.
Call Stack (most recent call first):
  CMakeLists.txt:2 (project)

-mneon-for-64bits & -mvectorize-with-neon-quad appear to be set in CXXFLAGS environment variable of my shell. If I try and override the variable they get picked up from somewhere else.

baconpaul commented 4 years ago

I wonder if it got cached by a prior cmake run. Try taking it out of your environment, then rm -rf build, then go again? This is a wild guess. Like I said, I only build surge on Ubuntu, windows 10 and macOS 10.15 (now; used to be 14) and our pipelines do the same, just with 10.13 as our macos.

But the software will build with clang - it is our production mac compiler and the one we use the most often in dev (since most of our dev work is mac first)

Baggypants commented 4 years ago

Your suggestions worked and now I have a whole different backtrace from the bus error!

Reading symbols from /usr/bin/Surge-Headless/Surge/Surge-Headless...(no debugging symbols found)...done.
(gdb) run
Starting program: /usr/bin/Surge-Headless/Surge/Surge-Headless
/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_IN.UTF-8)
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".

Program received signal SIGBUS, Bus error.
0x00040044 in InitQuadFilterChainStateToZero(QuadFilterChainState*) ()
(gdb) bt
#0  0x00040044 in InitQuadFilterChainStateToZero(QuadFilterChainState*) ()
#1  0x000e9484 in SurgeSynthesizer::SurgeSynthesizer(HeadlessPluginLayerProxy*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) ()
#2  0x0013186c in Surge::Headless::createSurge(int) ()
#3  0x0015af24 in ____C_A_T_C_H____T_E_S_T____0() ()
#4  0x0014bb50 in Catch::TestInvokerAsFunction::invoke() const ()
#5  0x00146b78 in Catch::TestCase::invoke() const ()
#6  0x00146ad4 in Catch::RunContext::invokeActiveTestCase() ()
#7  0x00145910 in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) ()
#8  0x00144dd0 in Catch::RunContext::runTest(Catch::TestCase const&) ()
#9  0x00148f10 in Catch::(anonymous namespace)::TestGroup::execute() ()
#10 0x001486e4 in Catch::Session::runInternal() ()
#11 0x00148524 in Catch::Session::run() ()
#12 0x0019b588 in int Catch::Session::run<char>(int, char const* const*) ()
#13 0x001734a8 in runAllTests(int, char**) ()
#14 0x0013064c in main ()
baconpaul commented 4 years ago

OK great. So we are really really close now I bet.

The QuadChainFilterState is one of the few places which still uses _aligned_malloc. Look at line 88 of SurgeSynthesizer.cpp.

The very first thing I would try, in your local copy, is to change that _aligned_malloc to malloc; and print out the pointers to make sure they are not null. What I think is happening is the _aligned_malloc is failing and we aren't dealing with that condition. But you are probably OK to use regular malloc for now, at least to see if the core dump moves.

Baggypants commented 4 years ago

I'm afraid I've hit error: needs_more_hand_holding with this one.

** Build of headless failed**
[ 57%] Building CXX object CMakeFiles/surge-headless.dir/src/common/SurgeError.cpp.o
/root/surge/src/common/SurgeSynthesizer.cpp:88:31: error: no matching function for call to 'malloc'
/root/surge/src/common/SurgeSynthesizer.cpp:90:31: error: no matching function for call to 'malloc'
16 warnings and 2 errors generated.
make[3]: *** [CMakeFiles/surge-headless.dir/build.make:564: CMakeFiles/surge-headless.dir/src/common/SurgeSynthesizer.cpp.o] Error 1
make[2]: *** [CMakeFiles/Makefile2:73: CMakeFiles/surge-headless.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:85: CMakeFiles/surge-headless.dir/rule] Error 2
make: *** [Makefile:118: surge-headless] Error 2

I thought it would be as easy as

root@zynthian:~/surge# git diff /root/surge/src/common/SurgeSynthesizer.cpp
diff --git a/src/common/SurgeSynthesizer.cpp b/src/common/SurgeSynthesizer.cpp
index bcac910..bb566b0 100644
--- a/src/common/SurgeSynthesizer.cpp
+++ b/src/common/SurgeSynthesizer.cpp
@@ -85,9 +85,9 @@ SurgeSynthesizer::SurgeSynthesizer(PluginLayer* parent, std::string suppliedData
    }

    FBQ[0] =
-       (QuadFilterChainState*)_aligned_malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
+       (QuadFilterChainState*)malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
    FBQ[1] =
-       (QuadFilterChainState*)_aligned_malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
+       (QuadFilterChainState*)malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);

    for(int i=0; i<(MAX_VOICES >> 2); ++i)
    {

and I've also tried

diff --git a/src/common/SurgeSynthesizer.cpp b/src/common/SurgeSynthesizer.cpp
index bcac910..4489275 100644
--- a/src/common/SurgeSynthesizer.cpp
+++ b/src/common/SurgeSynthesizer.cpp
@@ -85,9 +85,9 @@ SurgeSynthesizer::SurgeSynthesizer(PluginLayer* parent, std::string suppliedData
    }

    FBQ[0] =
-       (QuadFilterChainState*)_aligned_malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
+       (QuadFilterChainState*)_malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
    FBQ[1] =
-       (QuadFilterChainState*)_aligned_malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
+       (QuadFilterChainState*)_malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);

    for(int i=0; i<(MAX_VOICES >> 2); ++i)
    {

which threw

/root/surge/src/common/SurgeSynthesizer.cpp:88:31: error: use of undeclared identifier '_malloc'
/root/surge/src/common/SurgeSynthesizer.cpp:90:31: error: use of undeclared identifier '_malloc'

and

diff --git a/src/common/SurgeSynthesizer.cpp b/src/common/SurgeSynthesizer.cpp
index bcac910..39e552d 100644
--- a/src/common/SurgeSynthesizer.cpp
+++ b/src/common/SurgeSynthesizer.cpp
@@ -85,9 +85,9 @@ SurgeSynthesizer::SurgeSynthesizer(PluginLayer* parent, std::string suppliedData
    }

    FBQ[0] =
-       (QuadFilterChainState*)_aligned_malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
+       (QuadFilterChainState*) malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
    FBQ[1] =
-       (QuadFilterChainState*)_aligned_malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
+       (QuadFilterChainState*) malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);

    for(int i=0; i<(MAX_VOICES >> 2); ++i)
    {

which was the same as the first.

After web searching I thought it might be missing a header somehow and I've tried including #include <stdlib.h> in the SurgeSynthesizer.cpp the SurgeSynthesizer.h and the globals.h as per the example here but it hasn't made a difference.

baconpaul commented 4 years ago

Mallon only takes one argument so remove the ,16

That is

_aligned_malloc(foo, 16)

Becomes

Malloc( foo )

baconpaul commented 4 years ago

Also: “error: needs more handholding” made me laugh! Funny!

Baggypants commented 4 years ago

This results in exactly the same backtrace as https://github.com/surge-synthesizer/surge/issues/1481#issuecomment-575694812

Baggypants commented 4 years ago

I commented out this bit

diff --git a/src/common/SurgeSynthesizer.cpp b/src/common/SurgeSynthesizer.cpp
index bcac910..9aeeec0 100644
--- a/src/common/SurgeSynthesizer.cpp
+++ b/src/common/SurgeSynthesizer.cpp
@@ -85,14 +85,14 @@ SurgeSynthesizer::SurgeSynthesizer(PluginLayer* parent, std::string suppliedData
    }

    FBQ[0] =
-       (QuadFilterChainState*)_aligned_malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
+       (QuadFilterChainState*)malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState));
    FBQ[1] =
-       (QuadFilterChainState*)_aligned_malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
+       (QuadFilterChainState*)malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState));

    for(int i=0; i<(MAX_VOICES >> 2); ++i)
    {
-       InitQuadFilterChainStateToZero(&(FBQ[0][i]));
-       InitQuadFilterChainStateToZero(&(FBQ[1][i]));
+       /*InitQuadFilterChainStateToZero(&(FBQ[0][i]));
+       InitQuadFilterChainStateToZero(&(FBQ[1][i]));*/
    }

and that kind of gets bits of it working.

root@zynthian:~/surge# /usr/bin/Surge-Headless/Surge/Surge-Headless [wav]
Filters: [wav]
===============================================================================
All tests passed (1059 assertions in 2 test cases)
root@zynthian:~/surge# /usr/bin/Surge-Headless/Surge/Surge-Headless [tun]
Filters: [tun]
free(): corrupted unsorted chunks

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Surge-Headless is a Catch v2.9.2 host application.
Run with -? for options

-------------------------------------------------------------------------------
Retune Surge to .scl files
-------------------------------------------------------------------------------
/root/surge/src/headless/UnitTests.cpp:74
...............................................................................

/root/surge/src/headless/UnitTests.cpp:74: FAILED:
due to a fatal error condition:
  SIGABRT - Abort (abnormal termination) signal

===============================================================================
test cases: 1 | 1 failed
assertions: 5 | 4 passed | 1 failed

Aborted
root@zynthian:~/surge# /usr/bin/Surge-Headless/Surge/Surge-Headless [dsp]
Filters: [dsp]
Bus error
root@zynthian:~/surge# /usr/bin/Surge-Headless/Surge/Surge-Headless [osc]
Filters: [osc]
Bus error
root@zynthian:~/surge# /usr/bin/Surge-Headless/Surge/Surge-Headless [dsp]
Filters: [dsp]
Bus error
root@zynthian:~/surge# /usr/bin/Surge-Headless/Surge/Surge-Headless [osc]
Filters: [osc]
Bus error
root@zynthian:~/surge# /usr/bin/Surge-Headless/Surge/Surge-Headless [patch]
Filters: [patch]
===============================================================================
All tests passed (1 assertion in 1 test case)

root@zynthian:~/surge# /usr/bin/Surge-Headless/Surge/Surge-Headless [mod]
Filters: [mod]
===============================================================================
All tests passed (484998 assertions in 1 test case)
Baggypants commented 4 years ago

Because I'm now just randomly hitting things and pushing buttons I ran the [dsp] and [osc] tests in gdb and unsurprisingly the backtrace is about QuadFilterChainState. Probably because I commented out something important.

(gdb) run
Starting program: /usr/bin/Surge-Headless/Surge/Surge-Headless \[osc\]
/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_IN.UTF-8)
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".
Filters: [osc]

Program received signal SIGBUS, Bus error.
0x00073690 in void ProcessFBQuad<7, false, false, false>(QuadFilterChainState&, fbq_global&, float*, float*) ()
(gdb) bt
#0  0x00073690 in void ProcessFBQuad<7, false, false, false>(QuadFilterChainState&, fbq_global&, float*, float*) ()
#1  0x000f399c in SurgeSynthesizer::process() ()
#2  0x00132488 in Surge::Headless::playAsConfigured(SurgeSynthesizer*, std::vector<Surge::Headless::Event, std::allocator<Surge::Headless::Event> > const&, float**, int*, int*) ()
#3  0x0015ecd4 in frequencyForNote(std::shared_ptr<SurgeSynthesizer>, int, int, bool) ()
#4  0x001761bc in ____C_A_T_C_H____T_E_S_T____40()::$_23::operator()(char const*) const ()
#5  0x001711b8 in ____C_A_T_C_H____T_E_S_T____40() ()
#6  0x0014bb08 in Catch::TestInvokerAsFunction::invoke() const ()
#7  0x00146b30 in Catch::TestCase::invoke() const ()
#8  0x00146a8c in Catch::RunContext::invokeActiveTestCase() ()
#9  0x001458c8 in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) ()
#10 0x00144d88 in Catch::RunContext::runTest(Catch::TestCase const&) ()
#11 0x00148ec8 in Catch::(anonymous namespace)::TestGroup::execute() ()
#12 0x0014869c in Catch::Session::runInternal() ()
#13 0x001484dc in Catch::Session::run() ()
#14 0x0019b540 in int Catch::Session::run<char>(int, char const* const*) ()
#15 0x00173460 in runAllTests(int, char**) ()
#16 0x00130604 in main ()
baconpaul commented 4 years ago

With that init commented out, the filters will have unspecified initial state. Probably why you blow up in the filters later on. You need to figure out why that Init is coring you out.

Baggypants commented 4 years ago

I've compiled surge with -g to include debug symbols and now gdb will let me print the memory location of the pointer.

(gdb) run
Starting program: /usr/bin/Surge-Headless/Surge/Surge-Headless
/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_IN.UTF-8)
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".

Program received signal SIGBUS, Bus error.
0x00040044 in InitQuadFilterChainStateToZero (Q=0x54ec98) at /root/surge/src/common/dsp/QuadFilterChain.cpp:434
434             Q->DL[i] = simde_mm_setzero_ps();
(gdb) bt
#0  0x00040044 in InitQuadFilterChainStateToZero (Q=0x54ec98) at /root/surge/src/common/dsp/QuadFilterChain.cpp:434
#1  0x000e9484 in SurgeSynthesizer::SurgeSynthesizer (this=0xb658a020, parent=0x4f59a8, suppliedDataPath="")
    at /root/surge/src/common/SurgeSynthesizer.cpp:94
#2  0x0013186c in Surge::Headless::createSurge (sr=44100) at /root/surge/src/headless/HeadlessUtils.cpp:20
#3  0x0015af24 in ::____C_A_T_C_H____T_E_S_T__ () at /root/surge/src/headless/UnitTests.cpp:24
#4  0x0014bb50 in Catch::TestInvokerAsFunction::invoke (this=0x4f34e0) at /root/surge/libs/catch2/catch2.hpp:13697
#5  0x00146b78 in Catch::TestCase::invoke (this=0x4f8b78) at /root/surge/libs/catch2/catch2.hpp:13590
#6  0x00146ad4 in Catch::RunContext::invokeActiveTestCase (this=0xbeffed40) at /root/surge/libs/catch2/catch2.hpp:12405
#7  0x00145910 in Catch::RunContext::runCurrentTest (this=0xbeffed40, redirectedCout="", redirectedCerr="")
    at /root/surge/libs/catch2/catch2.hpp:12378
#8  0x00144dd0 in Catch::RunContext::runTest (this=0xbeffed40, testCase=...) at /root/surge/libs/catch2/catch2.hpp:12139
#9  0x00148f10 in Catch::(anonymous namespace)::TestGroup::execute (this=0xbeffed38) at /root/surge/libs/catch2/catch2.hpp:12719
#10 0x001486e4 in Catch::Session::runInternal (this=0xbeffef18) at /root/surge/libs/catch2/catch2.hpp:12919
#11 0x00148524 in Catch::Session::run (this=0xbeffef18) at /root/surge/libs/catch2/catch2.hpp:12875
#12 0x0019b588 in Catch::Session::run<char> (this=0xbeffef18, argc=1, argv=0xbefff1a4) at /root/surge/libs/catch2/catch2.hpp:12605
#13 0x001734a8 in runAllTests (argc=1, argv=0xbefff1a4) at /root/surge/src/headless/UnitTests.cpp:1297
#14 0x0013064c in main (argc=1, argv=0xbefff1a4) at /root/surge/src/headless/main.cpp:102
(gdb) list
429         Q->FBlineL = simde_mm_setzero_ps();
430         Q->FBlineR = simde_mm_setzero_ps();
431
432         for(auto i=0; i<BLOCK_SIZE_OS; ++i)
433         {
434             Q->DL[i] = simde_mm_setzero_ps();
435             Q->DR[i] = simde_mm_setzero_ps();
436         }
437
438         Q->OutL = simde_mm_setzero_ps();
(gdb) info locals
i = 0
(gdb) x /s i
0x0:    <error: Cannot access memory at address 0x0>

The location appears to be NULL

Baggypants commented 4 years ago

at frame 1 we have

(gdb) frame 1
#1  0x000e9484 in SurgeSynthesizer::SurgeSynthesizer (this=0xb658a020, parent=0x4f59a8, suppliedDataPath="")
    at /root/surge/src/common/SurgeSynthesizer.cpp:94
94             InitQuadFilterChainStateToZero(&(FBQ[0][i]));
(gdb) list
89         FBQ[1] =
90             (QuadFilterChainState*)_aligned_malloc((MAX_VOICES >> 2) * sizeof(QuadFilterChainState), 16);
91
92         for(int i=0; i<(MAX_VOICES >> 2); ++i)
93         {
94             InitQuadFilterChainStateToZero(&(FBQ[0][i]));
95             InitQuadFilterChainStateToZero(&(FBQ[1][i]));
96         }
97
98
(gdb) info locals
i = 0
patch = <error reading variable>
(gdb) x /s patch
value of type `SurgePatch' requires 924000 bytes, which is more than max-value-size
baconpaul commented 4 years ago

Right. So I worry that this has to do with how we alloc those QuadFilterChainStates - like is the constructor not running properly or some such. Will have to give it a bit of thought for how you should debug. I think having an ARM expert look at the memory allocation there could really help - maybe time to call in the simde person again?

baconpaul commented 4 years ago

Consolidated into #2345