Fixing pvalloc memleak test

[dubeyabhishek]

Request to allocate 30K bytes to pvalloc(), results in allocating 3*64Kb in return. The test matches for leaked amount to be exactly equal to 30K bytes, whereas on system configured with 64Kb pagesize, leaked memory is much more than 30K bytes.

[dubeyabhishek]

@chenhengqi is anything additional needed for this test? can we close this?

[yonghong-song]

I have a question about 'Request to allocate 30K bytes to pvalloc(), results in allocating 3*64Kb in return'. From the 'man pvalloc', we have

The obsolete function pvalloc() is similar to valloc(), but rounds the size of the allocation up to the next multiple of the system page size.

So the allocation size should be multiple of page size. So in your case, it should be 64Kb, right?

The following is an example on x86_64 which has 4KB as the page size,

$ cat memory-leak-pvalloc.c
#include <malloc.h>

int main() {
  void *p = pvalloc(1000);
  return 0;
}
$ clang -fsanitize=address -g memory-leak-pvalloc.c ; ASAN_OPTIONS=detect_leaks=1 ./a.out

=================================================================
==2359109==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 4096 byte(s) in 1 object(s) allocated from:
    #0 0x55bdf258d9b8 in pvalloc /home/yhs/work/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:156:3
    #1 0x55bdf25e7618 in main /home/yhs/tmp/memory-leak-pvalloc.c:4:13
    #2 0x7f971303ad84 in __libc_start_main (/lib64/libc.so.6+0x3ad84) (BuildId: d2e8988d9d5476b5c62a1495da4304dee4d76d27)

SUMMARY: AddressSanitizer: 4096 byte(s) leaked in 1 allocation(s).
$

So requesting 1000 bytes and get 4096 (4KB) bytes.

Could you double check in your env why it is 3 *64KB not 64KB?

[dubeyabhishek]

@yonghong-song

Please find 2 outputs below:

ASAN_OPTIONS=detect_leaks=1 ./a.out

=================================================================
==30996==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 65536 byte(s) in 1 object(s) allocated from:
#0 0x100c1dc0 in pvalloc (/root/a.out+0x100c1dc0) (BuildId: 454ef1b4623142e4662d69e7c6d436de05da99f7)
#1 0x10115458 in main /root/memory-leak.c:5:11
#2 0x7ffff7a31dc0 in __libc_start_call_main (/lib64/glibc-hwcaps/power10/libc.so.6+0x31dc0) (BuildId: fce00888c22b7f6cf8a0bbc86d55a324d3428fed)
#3 0x7ffff7a31f88 in __libc_start_main@GLIBC_2.17 (/lib64/glibc-hwcaps/power10/libc.so.6+0x31f88) (BuildId: fce00888c22b7f6cf8a0bbc86d55a324d3428fed)

SUMMARY: AddressSanitizer: 65536 byte(s) leaked in 1 allocation(s).

---

Failure from "make test" log : 196608 is 3 pages of 64KB

0/38 Testing: py_test_tools_memleak
30/38 Test: py_test_tools_memleak
Command: "/root/bcc/build/tests/wrapper.sh" "py_test_tools_memleak" "sudo" "/root/bcc/tests/python/test_tools_memleak.py"
Directory: /root/bcc/tests/python
"py_test_tools_memleak" start time: Nov 06 09:54 EST
Output:

.....F..

FAIL: test_pvalloc (main.MemleakToolTests)

Traceback (most recent call last):
File "/root/bcc/tests/python/test_tools_memleak.py", line 105, in test_pvalloc
self.assertEqual(cfg.leaking_amount, self.run_leaker("pvalloc"))
AssertionError: 30000 != 196608

---

Ran 8 tests in 34.676s

FAILED (failures=1)
Failed

[rnav]

Enabling tracing in memleak.py shows what is going on:

$ sudo ../tools/memleak.py -tc "/tmp/bcc-test-memleak-rh_1h9jr/leaker_app pvalloc 30000"
Executing '/tmp/bcc-test-memleak-rh_1h9jr/leaker_app pvalloc 30000' and tracing the resulting process.
Attaching to pid 24657, Ctrl+C to quit.

leaking via pvalloc, 0x7fffaa4a0000
(b'leaker_app', 24657, 22, b'....1', 336122.994575, b'alloc entered, size = 30000')
(b'leaker_app', 24657, 22, b'....1', 336122.994608, b'alloc entered, size = 196608')
(b'leaker_app', 24657, 22, b'....1', 336122.994653, b'alloc exited, size = 196608, result = 7fffaa490000')
(b'leaker_app', 24657, 22, b'....1', 336122.994721, b'alloc entered, size = 1024')
(b'leaker_app', 24657, 22, b'....1', 336122.99476, b'alloc exited, size = 1024, result = 1262d02a0')

We see a first probe hit for 30k, followed by another probe hit for 192k. The second one looks to be for mmap and with the below change to memleak.py:

diff --git a/tools/memleak.py b/tools/memleak.py
index e4ca5ac8..f3860073 100755
--- a/tools/memleak.py
+++ b/tools/memleak.py
@@ -467,7 +467,7 @@ if not kernel_trace:
         attach_probes("malloc")
         attach_probes("calloc")
         attach_probes("realloc")
-        attach_probes("mmap")
+        #attach_probes("mmap")
         attach_probes("posix_memalign")
         attach_probes("valloc", can_fail=True) # failed on Android, is deprecated in libc.so from bionic directory
         attach_probes("memalign")

... we see just the one 30k allocation:

$ sudo ../tools/memleak.py -tc "/tmp/bcc-test-memleak-rh_1h9jr/leaker_app pvalloc 30000"
Executing '/tmp/bcc-test-memleak-rh_1h9jr/leaker_app pvalloc 30000' and tracing the resulting process.
Attaching to pid 24691, Ctrl+C to quit.

leaking via pvalloc, 0x7fffb9b30000
(b'leaker_app', 24691, 14, b'....1', 336157.748015, b'alloc entered, size = 30000')
(b'leaker_app', 24691, 14, b'....1', 336157.748083, b'alloc exited, size = 30000, result = 7fffb9b30000')
(b'leaker_app', 24691, 14, b'....1', 336157.74811, b'alloc entered, size = 1024')
(b'leaker_app', 24691, 14, b'....1', 336157.748136, b'alloc exited, size = 1024, result = 15b2502a0')

... and allows the test to pass. Perhaps the invocation of mmap is a powerpc-specific aspect of pvalloc implementation in glibc, or to do with the larger 64k page size used by 64-bit powerpc.

[yonghong-song]

I looked at the test again. From the test case perspective, I think this patch is not needed even for powerpc with 64KB page. Basic the memleak tool does not really know what is the actually allocated memory size by the application/kernel. Sanitizer did more work to find real leaked size, but the memleak tool is pretty simple. It tracks the allocation with ptr and user-specified allocation size. If it does not see the ptr to be freed. It will report a leak with user-specified allocation size.
This happens to all architectures, not just for powerpc.

[rnav]

The patch from @dubeyabhishek still looks relevant to me.

I agree that the behavior is not necessarily related to 64k page size on powerpc, and more to do with how the memleak tool tracks [de-]allocations. However, the tool is also impacted by how libc implements pvalloc. In this case, we do see the assertion failing on powerpc due to the subsequent mmap invoked by pvalloc on powerpc.

The only alternative I can think of is to have the memleak tool ignore subsequent allocations.

[yonghong-song]

The patch from @dubeyabhishek still looks relevant to me.

I agree that the behavior is not necessarily related to 64k page size on powerpc, and more to do with how the memleak tool tracks [de-]allocations. However, the tool is also impacted by how libc implements pvalloc. In this case, we do see the assertion failing on powerpc due to the subsequent mmap invoked by pvalloc on powerpc.

The only alternative I can think of is to have the memleak tool ignore subsequent allocations.

Could you modify the test to guard your change with powerpc architecture? Other arch should still use the original assert. Please also add a comment in the code. Thanks!