We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 2221

Summary: Timidity, read_config_file, invalid read of size 8
Product: SDL_mixer Reporter: Sylvain <sylvain.becker>
Component: miscAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED INVALID QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2    
Version: unspecified   
Hardware: x86_64   
OS: Linux   
Attachments: config file

Description Sylvain 2013-11-07 21:53:38 UTC
Created attachment 1413 [details]
config file

valgrind --tool=exp-sgcheck ./main.out

Dont know if it worth fixing. sgcheck is experimental. but it worth a look.
On my linux I got this warning.
I attached the config file /etc/timidity/freepats.cfg


==4734== exp-sgcheck, a stack and global array overrun detector
==4734== NOTE: This is an Experimental-Class Valgrind Tool
==4734== Copyright (C) 2003-2012, and GNU GPL'd, by OpenWorks Ltd et al.
==4734== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==4734== Command: ./main.out
==4734== 
==4734== Invalid read of size 8
==4734==    at 0x55688C2: read_config_file (timidity.c:144)
==4734==    by 0x55692FB: Timidity_Init (timidity.c:289)
==4734==    by 0x555D71C: open_music (music.c:371)
==4734==    by 0x555C73C: Mix_OpenAudio (mixer.c:462)
...
==4734==    by 0x4027A9: main (main.cpp:31)
==4734==  Address 0x576f780 expected vs actual:
==4734==  Expected: global array "drumset" of size 1,040 in object with soname "libSDL2_mixer-2"
==4734==  Actual:   global array "tonebank" of size 1,040 in object with soname "libSDL2_mixer-2"
==4734==  Actual:   is 16 after Expected
Comment 1 Sam Lantinga 2013-11-23 10:13:17 UTC
That's weird.

Line 144 is this code:
    if (i<0 || i>127)
      {
        ctl->cmsg(CMSG_ERROR, VERB_NORMAL,
          "%s: line %d: Tone bank must be between 0 and 127\n",
        name, line);
        return -2;
      }
    if (!tonebank[i])
    ^^^^^^^^^^^^^^^^^

However tonebank is an array of size MAXBANK, which is defined as 130. I'm not sure how you can get an invalid read there.

Any ideas? Can you catch it in the debugger?
Comment 2 Sylvain 2013-11-24 15:34:06 UTC
ok, so indeed is not a bug ...
sgcheck is still experimental. this is a limitation of sgcheck.

http://valgrind.10908.n7.nabble.com/exp-sgcheck-bug-Fwd-ast-developers-Re-valgrind-quot-exp-sgcheck-quot-hit-in-ast-ksh-2012-05-18-s-pat-td30494.html

Quoting :

"Basically exp-sgcheck assumes that once a pointer has been used to
access one array it will always access the same array until the end of
the current function. "

which is exactly the case with pointer "bank" used for drumset and tonebank.
(moreover, I agree with your arguments)
Comment 3 Sam Lantinga 2013-11-25 04:29:43 UTC
Okay, thanks for looking into it.