Bug 428716 - VEX/useful/smchash.c:39:16: error: Memory leak: gb [memleak]
Summary: VEX/useful/smchash.c:39:16: error: Memory leak: gb [memleak]
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Paul Floyd
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-05 07:49 UTC by dcb314
Modified: 2021-02-20 15:45 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
Patch as suggested (530 bytes, patch)
2020-11-07 18:21 UTC, Paul Floyd
Details

Note You need to log in before you can comment on or make changes to this bug.
Description dcb314 2020-11-05 07:49:48 UTC
Source code is

  GuestBytes* gb = malloc(sizeof(GuestBytes));
  assert(gb);

  if (feof(f)) return NULL;

Maybe better code

  if (feof(f)) return NULL;

  GuestBytes* gb = malloc(sizeof(GuestBytes));
  assert(gb);
Comment 1 Paul Floyd 2020-11-07 15:50:57 UTC
Did you come across this just by looking at the code? Or using a static analysis tool? Or a runtime error?

The change looks safe to make.
Comment 2 dcb314 2020-11-07 16:29:30 UTC
(In reply to Paul Floyd from comment #1)
> Did you come across this just by looking at the code? Or using a static
> analysis tool? 

Option 2. Static analyser cppcheck.
Comment 3 Paul Floyd 2020-11-07 18:21:07 UTC
Created attachment 133113 [details]
Patch as suggested
Comment 4 Mark Wielaard 2020-11-07 23:20:16 UTC
(In reply to Paul Floyd from comment #3)
> Created attachment 133113 [details]
> Patch as suggested

Although technically correct I am not sure how useful this file is. I don't know how to build it.
Comment 5 Paul Floyd 2020-11-08 06:58:22 UTC
It's simple to build:

gcc -g -o smchash smchash.c

Running it is a bit more complicated. You need to generate in input file to test first, say something like this

../../vg-in-place  --trace-notabove=10000 --trace-flags=10000000 --vex-guest-chase=no pwd 2>&1 | grep Guest >  hash.in 

Then the test tool can be run

./smchash < hash.in

I don't really understand the stats generated.

Finally, cppcheck is probably being a bit pessimistic. 'read_one' is called from a while loop from apply_to_all that already checks !feof.
Comment 6 Julian Seward 2020-11-08 10:54:22 UTC
I don't think this is worth bothering with.  It's not part of the build.
It's just a program I hacked up to collect stats used to develop the 
hash functions that you see at the bottom of VEX/priv/guest_generic_bb_to_IR.c.
Comment 7 Paul Floyd 2020-11-08 16:43:35 UTC
Oh well. It's already gone in. A bit less noise next time someone runs a static analysis tool on the code.
Comment 8 Mark Wielaard 2021-02-20 15:45:00 UTC
commit 7967aea84b920d304b99fab60a612740854bd877
Author: Paul Floyd <pjfloyd@wanadoo.fr>
Date:   Sun Nov 8 08:00:36 2020 +0100

    Bug 478716 - cppcheck detects potential leak in VEX/useful/smchash.c