Bug 250345 - devel/valgrind-devel: 3.17.0-GIT valgrind not picking up right malloc on override
Summary: devel/valgrind-devel: 3.17.0-GIT valgrind not picking up right malloc on over...
Status: Closed Works As Intended
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: amd64 Any
: --- Affects Some People
Assignee: Paul Floyd
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-10-14 14:59 UTC by Karnajit Wangkhem
Modified: 2020-10-15 17:55 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (zeising)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Karnajit Wangkhem 2020-10-14 14:59:56 UTC
Below is an example where I am calling __malloc from my own malloc function.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

void *__malloc(unsigned long);
void __free(void *);

void *malloc(size_t sz)
{
  void *ptr = NULL;
  size_t origsz = sz + 4;
  ptr = __malloc(origsz);
  *((int *)ptr) = sz;
  write(1, "malloc called\n", 14);
  return ptr + 4;
}

void myfree(void *ptr)
{
  void *orig_ptr = ptr - 4;
  write(1, "free called\n", 12);
  __free(orig_ptr);
}

int main()
{
  char *str1 = (char *)malloc(100);
  memcpy(str1, "Hello World", 12);
  myfree(str1);

  return 0;
}

Without valgrind the result looks like this

# ./a.out
malloc called
free called

With valgrind its like this

# valgrind ./a.out
==14188== Memcheck, a memory error detector
==14188== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==14188== Using Valgrind-3.17.0.GIT and LibVEX; rerun with -h for copyright info
==14188== Command: ./a.out
==14188==
==14188== Warning: set address range perms: large range [0x7fffdffff000, 0x7ffffffdf000) (noaccess)
free called
==14188== Invalid free() / delete / delete[] / realloc()
==14188==    at 0x485068E: free (src/paul-floyd-317-fbsd12/valgrind-freebsd/coregrind/m_replacemalloc/vg_replace_malloc.c:611)
==14188==    by 0x20162F: myfree (malloc_free.c:23)
==14188==    by 0x20167E: main (malloc_free.c:30)
==14188==  Address 0x540003c is 4 bytes before a block of size 100 alloc'd
==14188==    at 0x484F4B9: malloc (src/paul-floyd-317-fbsd12/valgrind-freebsd/coregrind/m_replacemalloc/vg_replace_malloc.c:312)
==14188==    by 0x201658: main (malloc_free.c:28)
==14188==
==14188==
==14188== HEAP SUMMARY:
==14188==     in use at exit: 100 bytes in 1 blocks
==14188==   total heap usage: 1 allocs, 1 frees, 100 bytes allocated
==14188==
==14188== LEAK SUMMARY:
==14188==    definitely lost: 100 bytes in 1 blocks
==14188==    indirectly lost: 0 bytes in 0 blocks
==14188==      possibly lost: 0 bytes in 0 blocks
==14188==    still reachable: 0 bytes in 0 blocks
==14188==         suppressed: 0 bytes in 0 blocks
==14188== Rerun with --leak-check=full to see details of leaked memory
==14188==
==14188== For lists of detected and suppressed errors, rerun with: -s
==14188== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Only myfree is called in valgrind env and additional invalid free error came as a result of this. Please verify. overriding reallocf doesn't have this issue under valgrind.
Comment 1 Niclas Zeising freebsd_committer 2020-10-14 15:10:32 UTC
are you using valgrind-devel?
Comment 2 Karnajit Wangkhem 2020-10-14 16:23:50 UTC
(In reply to Niclas Zeising from comment #1)

Yes. in freebsd 12.1, I have installed the package from pkg install.

But the original issue we observed which building the below branch in our freebsd 12 env

https://ssd-git.juniper.net/tools-tot/valgrind-freebsd/-/tree/paul-floyd-317-fbsd12
Comment 3 Niclas Zeising freebsd_committer 2020-10-14 16:40:27 UTC
I don't know what that repo is, and I can't access it.
valgrind-devel is not maintained by me.
Comment 4 Paul Floyd 2020-10-14 20:09:29 UTC
First, what I think this should do.

Obviously your malloc should be called first. Valgrind shouldn't recognize
this malloc. Then __malloc should be called. Since libc malloc is a weak alias
for __malloc as per

paulf> nm /usr/lib/debug/lib/libc.so.7.debug  | grep 000000000011e990
000000000011e990 T __malloc
000000000011e990 W malloc

then Valgrind should see the call to __malloc as a call to malloc.

Then there is the memcpy and the call to myfree which calls __free, again a weak alias:

paulf> nm /usr/lib/debug/lib/libc.so.7.debug  | grep 0000000000122730
0000000000122730 T __free
0000000000122730 W free

So Valgrind should see this, and report no error.

If you *do* want to replace malloc and free inside your executable, you should use 

valgrind --soname-synonyms=somalloc=NONE ./a.out



(see https://www.valgrind.org/docs/manual/manual-core.html#manual-core.rareopts for more details)

The problem seems to be that Valgrind seems to be doing this by default when it shouldn't.

If I run with --trace-redir=yes then I see 

--PID--     0x002012e0 (malloc              ) R-> (1001.0) 0x0484f310 malloc
--PID--     0x0497a990 (malloc              ) R-> (1001.0) 0x0484f310 malloc
--PID-- REDIR: 0x2012e0 (NONE:malloc) redirected to 0x484f310 (malloc)

which is wrong

As a quick hack, if I comment out line 312 of vg_replace_malloc.c (in my repo version), then it seems to work OK and I get

paulf> ../valgrind/vg-in-place ./250345                           
==8835== Memcheck, a memory error detector
==8835== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==8835== Using Valgrind-3.17.0.GIT and LibVEX; rerun with -h for copyright info
==8835== Command: ./250345
==8835== 
malloc called
free called
==8835== 

and the redirs are

--PID--     0x0497a990 (malloc              ) R-> (1001.0) 0x0484f090 malloc
--PID-- REDIR: 0x497a990 (libc.so.7:malloc) redirected to 0x484f090 (malloc)

which is correct.


For reallocf, this is probably an oversight as the code contains


 REALLOC(VG_Z_LIBC_SONAME, realloc);
 REALLOC(SO_SYN_MALLOC,    realloc);
 REALLOCF(VG_Z_LIBC_SONAME, reallocf);

(note the missing SO_SYN_MALLOC).
Comment 5 Paul Floyd 2020-10-14 20:14:55 UTC
(note that the above hack will cause other errors, so it is not a proper fix)
Comment 6 Paul Floyd 2020-10-15 06:06:23 UTC
I also see this behaviour on Linux. The testcase needs modifying slightly since __free and __malloc are static functions in glibc, but __libc_malloc and __libc_free are global aliases similar to __free and __malloc in FreeBSD libc.

On Fedora 32 with the packaged version of the latest official Valgrind (3.16.1) I get

paulf> valgrind -q ./250345
free called
==6047== Invalid free() / delete / delete[] / realloc()
==6047==    at 0x483B9F5: free (vg_replace_malloc.c:538)
==6047==    by 0x4011FF: myfree (250345.c:23)
==6047==    by 0x40124E: main (250345.c:30)
==6047==  Address 0x4a4803c is 4 bytes before a block of size 100 alloc'd
==6047==    at 0x483A809: malloc (vg_replace_malloc.c:307)
==6047==    by 0x401228: main (250345.c:28)
==6047==
Comment 7 Karnajit Wangkhem 2020-10-15 09:06:08 UTC
(In reply to Paul Floyd from comment #5)

Thanks for providing the detailed info. If commenting SO_SYN_* effects only the usage of --soname-synonyms=, I will time being check out this hack.
Comment 8 Paul Floyd 2020-10-15 09:58:37 UTC
(In reply to Karnajit Wangkhem from comment #7)

The real answer is to use the soname synonyms with some bogus value.

For instance

paulf> valgrind --soname-synonyms=somalloc=undefined ./250345
==1730== Memcheck, a memory error detector
==1730== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1730== Using Valgrind-3.17.0.GIT and LibVEX; rerun with -h for copyright info
==1730== Command: ./250345
==1730==
malloc called
free called

This is on Linux, I'll check that this also works on FreeBSD this evening.

I need to fix the missing  REALLOCF(SO_SYN_MALLOC,    reallocf);
but otherwise this works as designed (subject to confirmation).
Comment 9 Karnajit Wangkhem 2020-10-15 10:24:54 UTC
(In reply to Paul Floyd from comment #8)

Thanks I will use the workaround instead.

> I need to fix the missing  REALLOCF(SO_SYN_MALLOC,    reallocf);
but otherwise this works as designed (subject to confirmation).

Atleast with available valgrind 3.11 on ubuntu 16, the issue doesn't seem to exist. Is it possible that later releases had this behaviour change.

$ valgrind --version
valgrind-3.11.0

$ valgrind ./a.out
==40477== Memcheck, a memory error detector
==40477== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==40477== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==40477== Command: ./a.out
==40477==
malloc called
free called
==40477==
==40477== HEAP SUMMARY:
==40477==     in use at exit: 0 bytes in 0 blocks
==40477==   total heap usage: 1 allocs, 1 frees, 104 bytes allocated
==40477==
==40477== All heap blocks were freed -- no leaks are possible
==40477==
==40477== For counts of detected and suppressed errors, rerun with: -v
==40477== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Comment 10 Paul Floyd 2020-10-15 13:22:21 UTC
The soname synonyms feature was added in May 2012 so it has been in since 3.8.0.

The default changed in Release 3.12.0 (20 October 2016)

Here's an extract from the README


* ==================== OTHER CHANGES ====================

* Replacement/wrapping of malloc/new related functions is now done not just
  for system libraries by default, but for any globally defined malloc/new
  related function (both in shared libraries and statically linked alternative
  malloc implementations).  The dynamic (runtime) linker is excluded, though.
  To only intercept malloc/new related functions in
  system libraries use --soname-synonyms=somalloc=nouserintercepts (where
  "nouserintercepts" can be any non-existing library name).
  This new functionality is not implemented for MacOS X
Comment 11 Paul Floyd 2020-10-15 17:54:34 UTC
Just to confirm, same behaviour with FreeBSD:

paulf> valgrind --soname-synonyms=somalloc=undefined ./250345
==1617== Memcheck, a memory error detector
==1617== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1617== Using Valgrind-3.17.0.GIT and LibVEX; rerun with -h for copyright info
==1617== Command: ./250345
==1617== 
malloc called
free called
==1617==