Linux crashes in AudioSampleBuffer move assignment operator


#1

Hi everybody,

I finally took the leap to the newest JUCE version (5.2.0 master) from 4.3.0. While the transition went smooth for all other OS, the Linux build is crashing almost instantly.

Unfortunately my debugging skills on Linux are pretty lousy, but I managed to pin down the crash location to the memcpy operation in the move assignment operator of the AudioSampleBuffer (but as always, this could also be a false alarm and the segfault happened much earlier, it’s just very consistent so it might be really the culprit).

I also ran valgrind on the debug build and it complained with “Invalid read of size 8” whenever I call something like this:

internalBuffer = AudioSampleBuffer(1, 0);

(if you’re wondering why I am creating a zero sized audio sample buffer it’s just that I need the channel amount set correctly in the constructor while the actual buffer size will be determined in the prepareToPlay method later on).

Now my question is: has there been any changes in the audio sample buffer code like expecting a certain memory alignment that has been introduced since JUCE 4.3.0? And why could this only happen on Linux?


#2

There have been huge changes since 4.3.0 but we’d really need a decent stack trace to see how you managed to trigger the problem.


#3

This is the stack trace (from valgrind) as soon as it leaves my code:

==29418== Invalid read of size 8
==29418==    at 0x4C326D6: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==29418==    by 0x4953D5: juce::AudioBuffer<float>::operator=(juce::AudioBuffer<float>&&) (juce_AudioSampleBuffer.h:220)
==29418==    by 0x6AF9C3: hise::ModulatorSynth::ModulatorSynth(hise::MainController*, juce::String const&, int) (ModulatorSynth.cpp:59)

The GDB backtrace looks like this:

#0  __memcpy_sse2_unaligned ()
    at ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:38
#1  0x00000000004953fd in juce::AudioBuffer<float>::operator=(juce::AudioBuffer<float>&&) (this=0x7f16880c8d80, 
    other=<unknown type in juce_audio_basics/buffers/juce_AudioSampleBuffer.h:219
#2  0x00000000006cb4f8 in hise::ModulatorSynthVoice::ModulatorSynthVoice (
    this=0x7f16880c8c10, ownerSynth_=0x7f1688006490)
    at ../hi_dsp/modules/ModulatorSynth.h:514

It’s another location, but it also creates a zero sized AudioSampleBuffer and uses the move assignment.

I can confirm that when I change the line

internalBuffer = AudioSampleBuffer(1, 0);

to

internalBuffer = AudioSampleBuffer(1, 32);

it stops crashing. It’s pretty unlikely that I am writing to the buffer before the “real” initialisation (otherwise the assertion in getWritePointer() would have fired long time ago) so I believe it has to do something with weird alignment issues.


#4

I guess that FloatVectorOperations::copy doesn’t expect a zero length. I can add a check to avoid that being called, e.g. changing line 154 to

        if (other.isClear || size == 0)

Is that enough to avoid the issue for you?


#5

(oh, and we’d need to also add a check in the clear() method to avoid calling the FloatVectorOperations::clear in there)


#6

Unfortunately no, still the same issue. But If I just create an empty AudioSampleBuffer at startup and do nothing with it, it doesn’t crash, so I am afraid, the memcpy is segfaulting for more complicated reasons here.

I’ll keep digging, but thanks for the advice.


#7

Weird: moving the initialisation from the constructor body to the initialiser list fixes the crash. This crashes:

Constructor()
{
    internalBuffer = AudioSampleBuffer(2, 0);
}

while this doesn’t:

Constructor():
  internalBuffer(2,0)
{

}

Any clues? Apparently the move operator is not being called in the initialiser list.


#8

Well that’s not a surprise - just constructing the object with a zero size will be fine, it won’t do any kind of copying.

But with the modifications I suggested, it also shouldn’t call any copy methods if the size is zero, so what’s the new call stack where it’s crashing?


#9

The fix you’ve suggested was for the copy constructor, but the crashes appeared in the move constructor along these lines (190+ on the master branch):

        if (numChannels < (int) numElementsInArray (preallocatedChannelSpace))
        {
            channels = preallocatedChannelSpace;
            memcpy (preallocatedChannelSpace, other.channels, sizeof (preallocatedChannelSpace));
        }
        else
        {
            channels = other.channels;
        }

So it was not segfaulting on the actual sample data, but on the preallocated pointer array that points to the data (which gets memcopied only in the move constructor as far as I can see).


#10

I don’t understand… memcpy doesn’t care about alignment, it won’t segfault as long as the data is a valid pointer (?)


#11

Yes the lib call to __memcpy_sse2_unaligned should be hint enough that it’s unaligned :slight_smile:

Still not sure how it can end up with a invalid pointer to a member’s data inside the constructor. Also, while the app didn’t crash, valgrind still goes crazy as soon as the move assignment operator is called:

==9013== Invalid read of size 8
==9013==    at 0x4C326D6: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9013==    by 0x4958B8: juce::AudioBuffer<float>::operator=(juce::AudioBuffer<float>&&) (juce_AudioSampleBuffer.h:219)
==9013==    by 0x6A8D8E: hise::ModulatorChain::ModulatorChain(hise::MainController*, juce::String const&, int, hise::Modulation::Mode, hise::Processor*) (ModulatorChain.cpp:43)

==9013==  Address 0x190832e0 is 0 bytes after a block of size 48 alloc'd
==9013==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9013==    by 0x4ADF49: void juce::HeapBlock<char, true>::malloc<unsigned long>(unsigned long, unsigned long) (juce_HeapBlock.h:226)
==9013==    by 0x4A1D03: juce::AudioBuffer<float>::allocateData() (juce_AudioSampleBuffer.h:1076)
==9013==    by 0x4944B1: juce::AudioBuffer<float>::AudioBuffer(int, int) (juce_AudioSampleBuffer.h:61)
==9013==    by 0x6A8D6E: hise::ModulatorChain::ModulatorChain(hise::MainController*, juce::String const&, int, hise::Modulation::Mode, hise::Processor*) (ModulatorChain.cpp:43)

so it seems that the allocation of the HeapBlock in the allocateData() method is doing something weird. I’d love to just ignore this and think that it’s a false positive from Valgrind (which might be the case if it doesn’t recognise the reinterpret_cast to Type**), but the fact that it was crashing at exact this location before is a bit worrisome.


#12

Can you give us a piece of sample code that reproduces it, so we can see what’s going on ourselves?


#13
  • Simple Console Application
  • JUCE Master 5.2.0
  • Ubuntu 16.04 LTS on a VirtualBox from macOS

This main code:

int main (int argc, char* argv[])
{

    DBG("Creating TestBuffer");
    
    AudioSampleBuffer b;

    DBG("Using move constructor with size 32");

    b = AudioSampleBuffer(2, 32);

    DBG("Using move constructor with size 0");

    b = AudioSampleBuffer(2, 0);

    return 0;
}

produces this valgrind warning:

==4304== Memcheck, a memory error detector
==4304== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==4304== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==4304== Command: build/ValTest
==4304== 
JUCE v5.2.0
Creating TestBuffer
Using move constructor with size 32
Using move constructor with size 0
==4304== Invalid read of size 8
==4304==    at 0x4C326D6: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4304==    by 0x40C9B3: juce::AudioBuffer<float>::operator=(juce::AudioBuffer<float>&&) (juce_AudioSampleBuffer.h:217)
==4304==    by 0x40C522: main (Main.cpp:42)
==4304==  Address 0x18d8d678 is 0 bytes after a block of size 56 alloc'd
==4304==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4304==    by 0x40CD5B: void juce::HeapBlock<char, true>::malloc<unsigned long>(unsigned long, unsigned long) (juce_HeapBlock.h:226)
==4304==    by 0x40CBD9: juce::AudioBuffer<float>::allocateData() (juce_AudioSampleBuffer.h:1070)
==4304==    by 0x40C8BF: juce::AudioBuffer<float>::AudioBuffer(int, int) (juce_AudioSampleBuffer.h:61)
==4304==    by 0x40C509: main (Main.cpp:42)
==4304== 
==4304== Invalid read of size 8
==4304==    at 0x4C326C8: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4304==    by 0x40C9B3: juce::AudioBuffer<float>::operator=(juce::AudioBuffer<float>&&) (juce_AudioSampleBuffer.h:217)
==4304==    by 0x40C522: main (Main.cpp:42)
==4304==  Address 0x18d8d688 is 16 bytes after a block of size 56 alloc'd
==4304==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4304==    by 0x40CD5B: void juce::HeapBlock<char, true>::malloc<unsigned long>(unsigned long, unsigned long) (juce_HeapBlock.h:226)
==4304==    by 0x40CBD9: juce::AudioBuffer<float>::allocateData() (juce_AudioSampleBuffer.h:1070)
==4304==    by 0x40C8BF: juce::AudioBuffer<float>::AudioBuffer(int, int) (juce_AudioSampleBuffer.h:61)
==4304==    by 0x40C509: main (Main.cpp:42)
==4304== 
==4304== 
==4304== HEAP SUMMARY:
==4304==     in use at exit: 122,516 bytes in 246 blocks
==4304==   total heap usage: 471 allocs, 225 frees, 151,451 bytes allocated
==4304== 
==4304== LEAK SUMMARY:
==4304==    definitely lost: 0 bytes in 0 blocks
==4304==    indirectly lost: 0 bytes in 0 blocks
==4304==      possibly lost: 1,720 bytes in 19 blocks
==4304==    still reachable: 120,796 bytes in 227 blocks
==4304==                       of which reachable via heuristic:
==4304==                         newarray           : 1,536 bytes in 16 blocks
==4304==         suppressed: 0 bytes in 0 blocks
==4304== Rerun with --leak-check=full to see details of leaked memory
==4304== 
==4304== For counts of detected and suppressed errors, rerun with: -v
==4304== ERROR SUMMARY: 25 errors from 2 contexts (suppressed: 0 from 0)

You can clearly see that the valgrind error is just firing with a size of 0.

As I said above, it may just be a false positive because the allocateData() method is reinterpreting the heapblock data to Type** and valgrind misses that and assumes an illegal read operation, but better be safe than sorry :slight_smile:


#14

Ah, I see the problem now! Thanks, I’ll push a fix for that in a few minutes…