libxmp / xmp-cli

Command-line mod player using libxmp
GNU General Public License v2.0
75 stars 22 forks source link

Memory violation when changing sequence #21

Closed cmatsuoka closed 3 years ago

cmatsuoka commented 3 years ago

When playing a module with only one sequence and pressing >, a memory access violation occurs:

Loading HYPNOSIS.MOD (1 of 1)
Module name  : Hypnosis
Module type  : Protracker clone M.K.
Module length: 37 patterns
Patterns     : 37
Instruments  : 31
Samples      : 31
Channels     : 4 [ 4 b b 4 ]
Duration     : 3min48s
Speed[06] BPM[7D] Pos[00/24] Pat[00/24] Row[14/3F] Chn[04/04]      0:00:02.4
CHANGE TO SEQ 1
==9150== Invalid read of size 4
==9150==    at 0x10CB95: change_sequence (commands.c:109)
==9150==    by 0x10C888: main (main.c:549)
==9150==  Address 0x564e284 is 4 bytes after a block of size 9,296 alloc'd
==9150==    at 0x483ED99: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==9150==    by 0x487C1AD: xmp_create_context (control.c:40)
==9150==    by 0x10BE23: main (main.c:358)
==9150== 
Sequence not changed: only one sequence available                  0:00:02.9
cmatsuoka commented 3 years ago

This is caused by a bug in a workaround for libxmp 4.0.4 or older. Since we currently require libxmp 4.4 or newer, I think we can just drop the workaround.

sezero commented 3 years ago

Is this, in any way, related to https://github.com/libxmp/libxmp/issues/153 ?

cmatsuoka commented 3 years ago

I believe this can be related to crashes when using < or > with libxmp-cli. I couldn't reproduce the crash mentioned in libxmp/libxmp#153 so maybe that's a bug in the Android application. Maybe @LossyDragon could investigate?

Update: just found another strange issue and I'm investigating it.

LossyDragon commented 3 years ago

I believe this can be related to crashes when using < or > with libxmp-cli. I couldn't reproduce the crash mentioned in libxmp/libxmp#153 so maybe that's a bug in the Android application. Maybe @LossyDragon could investigate?

Update: just found another strange issue and I'm investigating it.

When I was investigating the crash last year in libxmp/libxmp#153, everything pointed at libxmp causing the crash. I was also building libxmp with the latest commits at the time.

cmatsuoka commented 3 years ago

Thanks. It was caused by an uninitialized timestamp, fixed in the library.

I'll transfer the xmp-android repository to github.com/libxmp so perhaps that could be a good opportunity to merge your changes -- we'll need a fixed android app anyway.