Bug 428716

Summary: VEX/useful/smchash.c:39:16: error: Memory leak: gb [memleak]
Product: [Developer tools] valgrind Reporter: dcb314
Component: generalAssignee: Paul Floyd <pjfloyd>
Status: RESOLVED FIXED    
Severity: normal CC: jseward, mark, pjfloyd
Priority: NOR    
Version First Reported In: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: Patch as suggested

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