VGMStream randomly crashes when processing a lot of files at once for tags, but only on Intel #191

Closed
opened 2022-01-07 04:21:53 -03:00 by kode54 · 7 comments
kode54 commented 2022-01-07 04:21:53 -03:00 (Migrated from github.com)

Describe
The VGMStream decoder plugin, when fed 8 or more .txtp files at once for tag and properties reading, such as when adding them to the player for the first time, or when reloading their info manually, causes the player to crash. The crash only occurs on Intel machines, not even Apple Silicon machines running the hardened app under Rosetta. It does not happen under debug builds, or under the debugger. The crash will occur under the following call path:

init_vgmstream_from_cogfile -> init_vgmstream_from_STREAMFILE -> init_vgmstream_txtp -> init_vgmstream_from_STREAMFILE -> init_vgmstream_mus_acm -> check_extensions

To Reproduce (delete if not applicable)
Steps to reproduce the behavior:

  1. Launch Cog on an Intel machine
  2. Add a bunch of valid VGMStream .txtp files to the playlist at once
  3. If it hasn't crashed by now, force an info reload for all of them

Expected behavior
The inputs should reload the file properties without error, regardless of how many files are being reloaded at once.

Version information:*

  • macOS version: 11.6.2
  • Cog version: 1700

Additional context
Strangely, this is very hard to debug. Something must be clobbering the stack somewhere.

**Describe** The VGMStream decoder plugin, when fed 8 or more .txtp files at once for tag and properties reading, such as when adding them to the player for the first time, or when reloading their info manually, causes the player to crash. The crash only occurs on Intel machines, not even Apple Silicon machines running the hardened app under Rosetta. It does not happen under debug builds, or under the debugger. The crash will occur under the following call path: init_vgmstream_from_cogfile -> init_vgmstream_from_STREAMFILE -> init_vgmstream_txtp -> init_vgmstream_from_STREAMFILE -> init_vgmstream_mus_acm -> check_extensions **To Reproduce** (delete if not applicable) Steps to reproduce the behavior: 1. Launch Cog on an Intel machine 2. Add a bunch of valid VGMStream .txtp files to the playlist at once 3. If it hasn't crashed by now, force an info reload for all of them **Expected behavior** The inputs should reload the file properties without error, regardless of how many files are being reloaded at once. **Version information:*** - macOS version: 11.6.2 - Cog version: 1700 **Additional context** Strangely, this is very hard to debug. Something must be clobbering the stack somewhere.
kode54 commented 2022-01-07 07:41:01 -03:00 (Migrated from github.com)

Maybe this is because I was calling the playlistLoader loadInfoForEntries directly from the UI thread.

Maybe this is because I was calling the playlistLoader loadInfoForEntries directly from the UI thread.
kode54 commented 2022-01-07 22:07:40 -03:00 (Migrated from github.com)

Nope, it still crashes. And only on Intel. I'm thinking of dropping Intel support sooner rather than later.

Nope, it still crashes. And only on Intel. I'm thinking of dropping Intel support sooner rather than later.
kode54 commented 2022-01-07 22:23:14 -03:00 (Migrated from github.com)

Hmm, this could be a Clang bug in the Intel optimizer, but it's been around since at least Xcode 11 or 12 or so. I'll try reducing the Intel optimization level to -O1 or so and see if that fixes it.

Hmm, this could be a Clang bug in the Intel optimizer, but it's been around since at least Xcode 11 or 12 or so. I'll try reducing the Intel optimization level to -O1 or so and see if that fixes it.
nevack commented 2022-01-07 22:32:22 -03:00 (Migrated from github.com)

I'll try reducing the Intel optimization level to -O1 or so and see if that fixes it.

You mean only for the problematic target? Or the whole project?

> I'll try reducing the Intel optimization level to -O1 or so and see if that fixes it. You mean only for the problematic target? Or the whole project?
kode54 commented 2022-01-07 22:48:02 -03:00 (Migrated from github.com)

Only for the problematic target, which is libvgmstream. The major problem it has, as far as I can tell, is that it's massively inlining and unrolling the giant init loop that calls over 100 different format init functions until one of them succeeds. The loop is designed to call them from a function list table one by one. Apparently it thinks it's perfectly okay to just unroll a bunch of them, so that like 3-4 passes through the breakpoint-able calling function hit the crashing function, which is #73 (starting at 0) in the list. It also makes it impossible to trace what is clobbering the registers that contain the object pointers that are crashing.

Only for the problematic target, which is libvgmstream. The major problem it has, as far as I can tell, is that it's massively inlining and unrolling the giant init loop that calls over 100 different format init functions until one of them succeeds. The loop is designed to call them from a function list table one by one. Apparently it thinks it's perfectly okay to just unroll a bunch of them, so that like 3-4 passes through the breakpoint-able calling function hit the crashing function, which is #73 (starting at 0) in the list. It also makes it impossible to trace what is clobbering the registers that contain the object pointers that are crashing.
kode54 commented 2022-01-07 22:51:38 -03:00 (Migrated from github.com)

Okay, that seems to have fixed it. Pushing the commit.

Okay, that seems to have fixed it. Pushing the commit.
kode54 commented 2022-01-12 22:13:46 -03:00 (Migrated from github.com)

Closing this issue, because changing compile flags should have fixed it. Over inlining broke the ability of the VGMStream input to be called recursively, since several of its inputs are playlists that reference other files that it supports.

Closing this issue, because changing compile flags should have fixed it. Over inlining broke the ability of the VGMStream input to be called recursively, since several of its inputs are playlists that reference other files that it supports.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: chris/Cog#191
No description provided.