Skip to content
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.

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

[raudio] Segfault when loading (relatively) large files as Wave. #1423

Closed
3 tasks done
pedromundo opened this issue Nov 2, 2020 · 21 comments
Closed
3 tasks done

[raudio] Segfault when loading (relatively) large files as Wave. #1423

pedromundo opened this issue Nov 2, 2020 · 21 comments

Comments

@pedromundo
Copy link

pedromundo commented Nov 2, 2020

  • I tested it on latest raylib version from master branch.
  • I checked there is no similar issue already reported.
  • My code has no errors or misuse of raylib (I hope?).

Issue description

When loading multiple-minute .mp3 files as Wave's and calling the GetWaveData(Wave) method the application segfaults with no further explanation, I understand that we're supposed to use the Music type for longer files, but bear with me as I have a (not-so-)weird use case, and there are some things about this which lead me to believe that it should/could actually work:

  • It seems to be related to virtual memory size, as when running the application with either lldb or enabling address sanitizer (both of which set the virtual memory size to obscene values) then the application works flawlessly.
  • I've used a 3-minute (3.9MB) .mp3 file for my own testing. I've noticed that some of the audio metadata extracted by raylib seems to be different from what another audio tool (ocenaudio) says. Ocenaudio says the file is 16-bit, while raylib says it is 32-bit, ocenaudio reports 8008125 samples, while raylib reports 16017408 (which divided by two would be 8008704, so still a bit different). I have not actually ran the numbers to check which count is correct, but I think this might help explain the issue somehow.

If this is recognized as an actual bug, I would be willing to try and contribute a PR to fix the issue, given some basic guidance.

Environment

  • macOS Catalina 10.15.5
  • Intel HD Graphics 3000 512 MB
  • OpenGL Version: 3.3 INTEL-10.4.14
  • Using AppleClang 12.0.0.12000032
  • Reproduced with both raylib from master and the pre-built 3.0.0 binaries.

Code Example

#include "raylib.h"

int main(int, char**) {
  InitWindow(400, 200, "unseen");
  InitAudioDevice();
  Wave bgm_wave = LoadWave("resources/bgm/tmnt.mp3");

  float* bgm_wave_data = GetWaveData(bgm_wave);

  // Main game loop
  while (!WindowShouldClose())
  {
    TraceLog(LOG_INFO, "Wave size: %u %f", bgm_wave.sampleCount, bgm_wave_data[10]);
    BeginDrawing();
    ClearBackground(GetColor(0x052c46ff));
    DrawText("I hope you will never see this.", 100, 100, 10, RAYWHITE);
    EndDrawing();
  }

  UnloadWave(bgm_wave);

  CloseWindow();
  CloseAudioDevice();

  return 0;
}

PS: in the 3.0.0 release the Apple crash reporter could track the error down to https://github.com/raysan5/raylib/blob/3.0.0/src/raudio.c#L1024 which on master seems to be https://github.com/raysan5/raylib/blob/master/src/raudio.c#L1091

@raysan5
Copy link
Owner

raysan5 commented Nov 3, 2020

@pedromundo Thanks for the detailed issue!

Actually it seems it could be related to the differences in size reported, so the crash, typical from an array out-of-bounds...

Also note that when loading an MP3 as a Wave, samples are converted to float automatically, so no worries about the 16bit vs 32bit.

raylib is using dr_mp3 (drmp3_open_memory_and_read_f32()) to read the file data, maybe issue is on that function... I'll just let it know to @mackron, maybe he is aware of this issue.

@mackron
Copy link
Contributor

mackron commented Nov 3, 2020

Is this crash happening with all files, or just that one file? It's on 30MB or so, so would have thought it would be a virtual memory thing. I'll check dr_mp3 when I get a chance.

@pedromundo
Copy link
Author

pedromundo commented Nov 3, 2020

I've tested it with 3 multiple-minute mp3 files (~4, 5, 7 MB in size) and it seems to crash in the same way. With a tiny mp3 file (162KB) it is fine.

@pedromundo
Copy link
Author

Did a small test with dr_mp3.h (from master) and everything seems to work just fine calling both f32 and s16 versions of drmp3_open_memory_and_read_pcm_frames_X. LoadFileData was just copy-pasted from raudio.c@master (the joys of 'simple' software).

Compiled this as:
clang++ -Ofast -fpie main.cc --std=c++17

#include <cstdio>
#include <cstdlib>
#include <iostream>

#define DR_MP3_IMPLEMENTATION
#include "dr_mp3.h"

// Copied from raylib.
static unsigned char *LoadFileData(const char *fileName,
                                   unsigned int *bytesRead) {
  unsigned char *data = NULL;
  *bytesRead = 0;

  if (fileName != NULL) {
    FILE *file = fopen(fileName, "rb");

    if (file != NULL) {
      // WARNING: On binary streams SEEK_END could not be found,
      // using fseek() and ftell() could not work in some (rare) cases
      fseek(file, 0, SEEK_END);
      int size = ftell(file);
      fseek(file, 0, SEEK_SET);

      if (size > 0) {
        data = (unsigned char *)malloc(size * sizeof(unsigned char));

        // NOTE: fread() returns number of read elements instead of bytes, so we
        // read [1 byte, size elements]
        unsigned int count =
            (unsigned int)fread(data, sizeof(unsigned char), size, file);
        *bytesRead = count;

        if (count != size)
          std::printf("FILEIO: [%s] File partially loaded\n", fileName);
        else
          std::printf("FILEIO: [%s] File loaded successfully\n", fileName);
      } else
        std::printf("FILEIO: [%s] Failed to read file\n", fileName);

      fclose(file);
    } else
      std::printf("FILEIO: [%s] Failed to open file\n", fileName);
  } else
    std::printf("FILEIO: File name provided is not valid\n");

  return data;
}

int main(int, char **) {
  char const *mp3FileName = "./tmnt.mp3";
  unsigned int mp3FileSize;
  unsigned char *mp3FileData = LoadFileData(mp3FileName, &mp3FileSize);
  drmp3_config mp3Config = {0};
  unsigned long long int totalFrameCount = 0;
  float *waveData = drmp3_open_memory_and_read_pcm_frames_f32(
      mp3FileData, mp3FileSize, &mp3Config, &totalFrameCount, NULL);
  for (unsigned long long int i = 0; i < totalFrameCount; ++i) {
    // very spammy, but makes sure we run through the data, can be output to file.
    // std::cout << i << " " << waveData[i] << "\n";
  }
  std::cout.flush();
}

@raysan5
Copy link
Owner

raysan5 commented Nov 15, 2020

@pedromundo It seems the issue could be related to LoadMP3() in raudio module, did you check it?

Actually, there is a difference, you are using drmp3_open_memory_and_read_pcm_frames_f32() while the function is using drmp3_open_memory_and_read_f32(), maybe that's the issue...

@mackron
Copy link
Contributor

mackron commented Nov 15, 2020

drmp3_open_memory_and_read_f32() was renamed to drmp3_open_memory_and_read_pcm_frames_f32() over a year ago. You should probably upgrade dr_mp3 and try again (might have some API changes to deal with, though).

@raysan5
Copy link
Owner

raysan5 commented Nov 15, 2020

@mackron doing it now! :D

raysan5 added a commit that referenced this issue Nov 15, 2020
miniaudio -> v0.10.25
dr_wav -> v0.12.14
dr_mp3 -> v0.6.19
dr_flac -> v0.12.22
@pedromundo
Copy link
Author

I was gonna take a shot and see how far I'd get "fixing" it myself, but it appears to be taken care of? :)

@raysan5
Copy link
Owner

raysan5 commented Nov 16, 2020

@pedromundo Did you try the fix? Does it work now?

@pedromundo
Copy link
Author

Didn't try it yet, will do it later today!

@pedromundo
Copy link
Author

pedromundo commented Nov 17, 2020

The issue does not appear to be fixed yet.

I get the same behavior as before, where with either ASAN or lldb (which increase the virtual memory size) it just works.

Oddly enough, Raylib now reports 16019712 total samples for the same file I used in my earlier testing, where the reported size was 16017408.

The apple crash reporter tracks it down to GetWaveData, but I don't know much else right now.

Could this be some weird macOS thing?

EDIT: just realized something that is even weirder, if I keep trying to launch the application it will work roughly 1/5 of the time...

@raysan5
Copy link
Owner

raysan5 commented Nov 17, 2020

@pedromundo Can you confirm LoadWave() works? Now it uses almost same code than the one you posted for your test, LoadMP3() was reviewed and dr_mp3 was updated to latest version...

Maybe that part works and the actual problem is in GetWaveData()?

@pedromundo
Copy link
Author

Indeed, I added some extra tracing to my application and the application is fine as far as LoadWave() is called, then it crashes the next line where I call GetWaveData().

As that function looks very simple, I can try recompiling raylib with address sanitizer and checking if anything useful comes up.

@pedromundo
Copy link
Author

That went well:

INFO: Loaded wave data.
=================================================================
==1754==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x00011fe7d800 at pc 0x00010b82a42e bp 0x7ffee4510790 sp 0x7ffee4510788
READ of size 4 at 0x00011fe7d800 thread T0
    #0 0x10b82a42d in GetWaveData+0x44d (raydro:x86_64+0x10013b42d)
    #1 0x10b6f6570 in main+0x110 (raydro:x86_64+0x100007570)
    #2 0x7fff73265cc8 in start+0x0 (libdyld.dylib:x86_64+0x1acc8)

0x00011fe7d800 is located 0 bytes to the right of 67108864-byte region [0x00011be7d800,0x00011fe7d800)
allocated by thread T0 here:
    #0 0x10ba090f2 in wrap_realloc+0xa2 (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x460f2)
    #1 0x10b865ee8 in drmp3__realloc_default+0x8 (raydro:x86_64+0x100176ee8)
    #2 0x10b867393 in drmp3__realloc_from_callbacks+0x63 (raydro:x86_64+0x100178393)
    #3 0x10b82367b in drmp3__full_read_and_close_f32+0x22b (raydro:x86_64+0x10013467b)
    #4 0x10b824097 in drmp3_open_memory_and_read_pcm_frames_f32+0x127 (raydro:x86_64+0x100135097)
    #5 0x10b826ae6 in LoadMP3+0x126 (raydro:x86_64+0x100137ae6)
    #6 0x10b8261a2 in LoadWaveFromMemory+0x162 (raydro:x86_64+0x1001371a2)
    #7 0x10b825f95 in LoadWave+0x135 (raydro:x86_64+0x100136f95)
    #8 0x10b6f653c in main+0xdc (raydro:x86_64+0x10000753c)
    #9 0x7fff73265cc8 in start+0x0 (libdyld.dylib:x86_64+0x1acc8)

SUMMARY: AddressSanitizer: heap-buffer-overflow (raydro:x86_64+0x10013b42d) in GetWaveData+0x44d
Shadow bytes around the buggy address:
  0x100023fcfab0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100023fcfac0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100023fcfad0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100023fcfae0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100023fcfaf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x100023fcfb00:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100023fcfb10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100023fcfb20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100023fcfb30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100023fcfb40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x100023fcfb50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==1754==ABORTING
Abort trap: 6

@chriscamacho
Copy link
Contributor

@pedromundo ya gotta love libasan, if you add -g you should get line numbers on the back trace, compile both raylib and your app with libasan and -g

I use the following flags for asan btw

-fsanitize=address,leak,undefined,pointer-compare,pointer-subtract -fno-omit-frame-pointer

@pedromundo
Copy link
Author

@chriscamacho yeah, I've taken a liking to ASAN (and TSAN) as they've really helped me pull through some annoying issues in an embedded environment recently.

In any case, here's another ASAN report (which should have all the goodies):

INFO: Loaded wave data.
=================================================================
==3124==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x000121dce800 at pc 0x00010c16caa8 bp 0x7ffee3daf510 sp 0x7ffee3daf508
READ of size 4 at 0x000121dce800 thread T0
    #0 0x10c16caa7 in GetWaveData raudio.c:1091
    #1 0x10be57aea in main main.cc:52
    #2 0x7fff6fbdccc8 in start+0x0 (libdyld.dylib:x86_64+0x1acc8)

0x000121dce800 is located 0 bytes to the right of 67108864-byte region [0x00011ddce800,0x000121dce800)
allocated by thread T0 here:
    #0 0x10c58a0f2 in wrap_realloc+0xa2 (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x460f2)
    #1 0x10c224c98 in drmp3__realloc_default dr_mp3.h:2495
    #2 0x10c2289d4 in drmp3__realloc_from_callbacks dr_mp3.h:2530
    #3 0x10c160fe5 in drmp3__full_read_and_close_f32 dr_mp3.h:4169
    #4 0x10c162127 in drmp3_open_memory_and_read_pcm_frames_f32 dr_mp3.h:4298
    #5 0x10c16634a in LoadMP3 raudio.c:2034
    #6 0x10c165843 in LoadWaveFromMemory raudio.c:727
    #7 0x10c1655a5 in LoadWave raudio.c:700
    #8 0x10be57ab2 in main main.cc:40
    #9 0x7fff6fbdccc8 in start+0x0 (libdyld.dylib:x86_64+0x1acc8)

SUMMARY: AddressSanitizer: heap-buffer-overflow raudio.c:1091 in GetWaveData
Shadow bytes around the buggy address:
  0x1000243b9cb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000243b9cc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000243b9cd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000243b9ce0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1000243b9cf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x1000243b9d00:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1000243b9d10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1000243b9d20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1000243b9d30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1000243b9d40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1000243b9d50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==3124==ABORTING
Abort trap: 6

@chriscamacho
Copy link
Contributor

just out of interest, can you replicate the issue from a C application (I assume main.cc is c++ ??) ?

@raysan5
Copy link
Owner

raysan5 commented Nov 19, 2020

@pedromundo After some investigation I detected a big issue with raylib! In several functions I was mixing frameCount with sampleCount! raylib stores sampleCount = frameCount*channels... more info in commit 7f6cd93.

Now, it should work.

@pedromundo
Copy link
Author

@raysan5 oof, that’s good to hear! I’ll update all the things and take a look tomorrow.

@pedromundo
Copy link
Author

After updating to 4eae763 I can confirm that it now works as expected, thanks!

@raysan5
Copy link
Owner

raysan5 commented Nov 20, 2020

@pedromundo nice! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants