Menu

#213 valgrind massif reports

open
nobody
engine (107)
2
2010-08-26
2007-11-20
No

Posted by Eero Tamminen to the nazghul-devel list today:

I also used Valgrind memcheck on Nazghul again:
valgrind --tool=memcheck --leak-check=full ../../src/nazghul -I ./ -G
~/.haxima

with the Tutorial I got a few warnings which might be worth looking into:
--------------------------------
warn: 'kern_set_frame' is obsolete
warn: 'kern_set_ascii' is obsolete
warn: 'kern_set_cursor' is obsolete
==9752==
==9752== Conditional jump or move depends on uninitialised value(s)
==9752== at 0x804EB6B: Character::setCurrentFaction(int)
(character.cpp:3271)
==9752== by 0x8053964: Character::Character(char*, char*, sprite*,
species*, occ*, int, int, int, int, int, int, int, int, int, int, int, int
(character.cpp:195)
==9752== by 0x80A1605: kern_mk_char(scheme*, cell*) (kern.c:1710)
==9752== by 0x8087CE5: opexe_0(scheme*, scheme_opcodes) (scheme.c:2436)
==9752== by 0x808563A: Eval_Cycle(scheme*, scheme_opcodes)
(scheme.c:4136)
==9752== by 0x8085A19: scheme_load_named_file(scheme*, _IO_FILE*, char
const*) (scheme.c:4543)
==9752== by 0x808B7D5: session_load(char*) (session.c:374)
==9752== by 0x8068661: playRun (play.c:299)
==9752== by 0x805E5B6: main (nazghul.c:363)
...
Putting The Wanderer near (9 9)
Checking (9,9)...OK!
==9752==
==9752== Invalid read of size 4
==9752== at 0x804D60A: Character::getNextArms(int*) (character.cpp:961)
==9752== by 0x804DEC3: Character::getSpeed() (character.cpp:1938)
==9752== by 0x8060C42: Object::getActionPointsPerTurn() (object.c:997)
==9752== by 0x8060DA5: Object::startTurn() (object.c:1075)
==9752== by 0x80508DC: Character::exec() (character.cpp:2380)
==9752== by 0x80672EA: place_exec (place.c:1801)
==9752== by 0x8068853: playRun (play.c:173)
==9752== by 0x805E5B6: main (nazghul.c:363)
==9752== Address 0x47228FC is 4 bytes before a block of size 32 alloc'd
==9752== at 0x4020A55: operator new[](unsigned) (vg_replace_malloc.c:195)
==9752== by 0x8051975: Character::initCommon() (character.cpp:1521)
==9752== by 0x8053A63: Character::Character(char*, char*, sprite*,
species*, occ*, int, int, int, int, int, int, int, int, int, int, int, int)
(character.cpp:230)
==9752== by 0x80A1605: kern_mk_char(scheme*, cell*) (kern.c:1710)
==9752== by 0x8087CE5: opexe_0(scheme*, scheme_opcodes) (scheme.c:2436)
==9752== by 0x808563A: Eval_Cycle(scheme*, scheme_opcodes)
(scheme.c:4136)
==9752== by 0x8085A19: scheme_load_named_file(scheme*, _IO_FILE*, char
const*) (scheme.c:4543)
==9752== by 0x808B7D5: session_load(char*) (session.c:374)
==9752== by 0x8068661: playRun (play.c:299)
==9752== by 0x805E5B6: main (nazghul.c:363)

There are actually a lot of errors like the above one, especially when
readying one's armor.

==9752== Conditional jump or move depends on uninitialised value(s)
==9752== at 0x805C604: map_paint_tile_objects(place*, int, int, int, int,
int) (map.c:805)
==9752== by 0x805B37D: map_render_loop(place*, SDL_Rect*, SDL_Rect*,
unsigned char*, SDL_Rect*, int, int, void (*)(place*, int, int, int, int,
int)) (map.c:921)
==9752== by 0x805B48C: mapPaintPlace(place*, SDL_Rect*, SDL_Rect*,
unsigned char*, SDL_Rect*, int, int) (map.c:944)
==9752== by 0x805CB66: mapUpdate (map.c:1064)
==9752== by 0x80685E1: updateAfterEvent() (play.c:281)
==9752== by 0x8059022: event_handle_aux(int) (event.c:289)
==9752== by 0x807C099: ui_select_target_generic (cmd.c:1644)
==9752== by 0x807C239: select_target_rlcb(place*, int, int, int*, int*,
int, list*, void (*)(place*, int, int, void*), int (*)(place*, int, int,
void*)) (cmd.c:1600)
==9752== by 0x807C6E2: cmdHandle (cmd.c:1773)
==9752== by 0x8082623: ctrl_character_key_handler(KeyHandler*, int, int)
(ctrl.c:1160)
==9752== by 0x8059068: event_handle_aux(int) (event.c:236)
==9752== by 0x807FAE0: ctrl_character_ui(Character*) (ctrl.c:1691)
==9752== by 0x8050C7D: Character::exec() (character.cpp:2555)
==9752== by 0x80672EA: place_exec (place.c:1801)
==9752== by 0x8068853: playRun (play.c:173)
==9752== by 0x805E5B6: main (nazghul.c:363)
--------------------------------

There are also some minor memory leaks once Tutorial is quit
(I quit and started it twice in case that matters):
--------------------------------
==9752== 43,604 (28 direct, 43,576 indirect) bytes in 1 blocks are
definitely lost in loss record 24 of 124
==9752== at 0x4020620: malloc (vg_replace_malloc.c:149)
==9752== by 0x808B675: session_add(session*, void*, void (*)(void*), void
(*)(save*, void*), void (*)(void*)) (session.c:144)
==9752== by 0x809E4F8: kern_mk_sprite(scheme*, cell*) (kern.c:646)
==9752== by 0x8087CE5: opexe_0(scheme*, scheme_opcodes) (scheme.c:2436)
==9752== by 0x808563A: Eval_Cycle(scheme*, scheme_opcodes)
(scheme.c:4136)
==9752== by 0x8085A19: scheme_load_named_file(scheme*, _IO_FILE*, char
const*) (scheme.c:4543)
==9752== by 0x808B7D5: session_load(char*) (session.c:374)
==9752== by 0x8068661: playRun (play.c:299)
==9752== by 0x805E5B6: main (nazghul.c:363)

==9752== 12,732 (9,480 direct, 3,252 indirect) bytes in 474 blocks are
definitely lost in loss record 118 of 124
==9752== at 0x401F95F: calloc (vg_replace_malloc.c:279)
==9752== by 0x808C228: closure_new (closure.c:167)
==9752== by 0x809E01C: kern_mk_vehicle_type(scheme*, cell*) (kern.c:3714)
==9752== by 0x8087CE5: opexe_0(scheme*, scheme_opcodes) (scheme.c:2436)
==9752== by 0x808563A: Eval_Cycle(scheme*, scheme_opcodes)
(scheme.c:4136)
==9752== by 0x8085A19: scheme_load_named_file(scheme*, _IO_FILE*, char
const*) (scheme.c:4543)
==9752== by 0x808B7D5: session_load(char*) (session.c:374)
==9752== by 0x8068661: playRun (play.c:299)
==9752== by 0x805E5B6: main (nazghul.c:363)

==9752== 7,460 (240 direct, 7,220 indirect) bytes in 5 blocks are definitely
lost in loss record 103 of 124
==9752== at 0x4020620: malloc (vg_replace_malloc.c:149)
==9752== by 0x807630C: terrain_map_new (terrain_map.c:37)
==9752== by 0x80A0AA1: kern_mk_map(scheme*, cell*) (kern.c:787)
==9752== by 0x8087CE5: opexe_0(scheme*, scheme_opcodes) (scheme.c:2436)
==9752== by 0x808563A: Eval_Cycle(scheme*, scheme_opcodes)
(scheme.c:4136)
==9752== by 0x8085A19: scheme_load_named_file(scheme*, _IO_FILE*, char
const*) (scheme.c:4543)
==9752== by 0x808B7D5: session_load(char*) (session.c:374)
==9752== by 0x8068661: playRun (play.c:299)
==9752== by 0x805E5B6: main (nazghul.c:363)
--------------------------------

(and some even smaller ones)

Discussion

  • Eero Tamminen

    Eero Tamminen - 2007-11-26

    Logged In: YES
    user_id=846799
    Originator: NO

    With the actual game this leak is a bit larger than in tutorial:
    ==9245== 293,683 (5,520 direct, 288,163 indirect) bytes in 115 blocks are definitely lost in loss record 87 of 97
    ==9245== at 0x4020620: malloc (vg_replace_malloc.c:149)
    ==9245== by 0x807630C: terrain_map_new (terrain_map.c:37)
    ==9245== by 0x80A0AA1: kern_mk_map(scheme*, cell*) (kern.c:787)
    ==9245== by 0x8087CE5: opexe_0(scheme*, scheme_opcodes) (scheme.c:2436)
    ==9245== by 0x808563A: Eval_Cycle(scheme*, scheme_opcodes) (scheme.c:4136)
    ==9245== by 0x8085A19: scheme_load_named_file(scheme*, _IO_FILE*, char const*) (scheme.c:4543)
    ==9245== by 0x808B7D5: session_load(char*) (session.c:374)
    ==9245== by 0x8068661: playRun (play.c:299)
    ==9245== by 0x805E5B6: main (nazghul.c:363)

    And there's one more leak:
    ==9245== 86,740 (2,260 direct, 84,480 indirect) bytes in 113 blocks are definitely lost in loss record 22 of 97
    ==9245== at 0x401F95F: calloc (vg_replace_malloc.c:279)
    ==9245== by 0x80A5C09: node_new (node.c:32)
    ==9245== by 0x8066FB0: place_add_to_turn_list(place*, Object*) (place.c:421)
    ==9245== by 0x806704D: place_add_object (place.c:962)
    ==9245== by 0x8063790: Object::relocate(place*, int, int, int, closure*) (object.c:519)
    ==9245== by 0x809F9E5: kern_mk_place(scheme*, cell*) (kern.c:1141)
    ==9245== by 0x8087CE5: opexe_0(scheme*, scheme_opcodes) (scheme.c:2436)
    ==9245== by 0x808563A: Eval_Cycle(scheme*, scheme_opcodes)
    (scheme.c:4136)
    ==9245== by 0x8085A19: scheme_load_named_file(scheme*, _IO_FILE*, char const*) (scheme.c:4543)
    ==9245== by 0x808B7D5: session_load(char*) (session.c:374)
    ==9245== by 0x8068661: playRun (play.c:299)
    ==9245== by 0x805E5B6: main (nazghul.c:363)

    The good news is that all these even slightly larger leaks come only when the game is quit and you get back to the startup screen[1]. So they are really an issue only if user would play Haxima *really* many times in a row. :-)

    [1] You can check this by this by killing the valgrind/nazghul process with a signal which can be catched by Valgrind, but is not catched by nazghul (such as "kill -USR1 <valgrind PID>") instead of quitting nazghul normally with "q".

    So... I would be more concerned about the invalid read (reading 4 bytes before allocation) which is an obvious error in character.cpp:
    if (armsIndex > 0 && rdyArms[*armsIndex-1] == rdyArms[*armsIndex] &&
    rdyArms[*armsIndex]->getNumHands() == 2)
    (this should be "*armsIndex > 0")

    My guess is that the other character.cpp Valgrind warning comes from this code:
    bool Character::isPlayerPartyMember()
    {
    // If player party has not been created yet it's safe to assume that
    // for now this is not a party member.
    if (! player_party) {
    return false;
    }
    return (class Object*)party == (class Object*)player_party;
    }

    maybe because player_party isn't properly initialized on the first time?

    That leaves the question of what in map.c:805
    /* If the crosshair is active but this tile is not in range
    * then shade the tile. */
    if (Session->crosshair->is_active() &&
    Session->crosshair->isRangeShaded() &&
    ! Session->crosshair->inRange(map_x, map_y))

    Could be uninitialized? I get it when I "Handle" the first Tutorial lever.

     
  • Gordon McNutt

    Gordon McNutt - 2008-01-12
    • priority: 5 --> 8
     
  • Gordon McNutt

    Gordon McNutt - 2008-01-13

    Logged In: YES
    user_id=54873
    Originator: YES

    Fixed a 3k leak in version 1.67 of session.sc, but it doesn't look like any of these.

     
  • Gordon McNutt

    Gordon McNutt - 2008-06-07

    Logged In: YES
    user_id=54873
    Originator: YES

    Fixed some more.

     
  • Gordon McNutt

    Gordon McNutt - 2008-06-07
    • priority: 8 --> 2
     
  • Gordon McNutt

    Gordon McNutt - 2010-08-26
    • labels: --> engine
     

Log in to post a comment.