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 - Timidity, read_config_file, invalid read of size 8
Summary: Timidity, read_config_file, invalid read of size 8
Status: RESOLVED INVALID
Alias: None
Product: SDL_mixer
Classification: Unclassified
Component: misc (show other bugs)
Version: unspecified
Hardware: x86_64 Linux
: P2 normal
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-07 21:53 UTC by Sylvain
Modified: 2013-11-25 04:29 UTC (History)
0 users

See Also:

Attachments
config file (4.55 KB, text/plain)
2013-11-07 21:53 UTC, Sylvain 		Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.