Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

memleak:support third party alloctions by definitive symbols prefix #4812

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

judeng
Copy link
Contributor

@judeng judeng commented Nov 27, 2023

The third-party allocations, such as jellmalloc that is usually statically linked into redis using the "je_" prefix, so we can specify the prefix string of these symbols to detect memory leaks in softwares using third-party allocs.

@yonghong-song
Copy link
Collaborator

There is a merge conflict. Could you rebase and resubmit?

pid=pid)
bpf.attach_uprobe(name=obj, sym="munmap", fn_name="munmap_enter",
attach_probes("free", need_uretprobe=False)
attach_probes("munmap", can_fail=True, need_uretprobe=False) # failed on jemalloc
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the above change break other libraries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't break other libraries. For the C library, the actual implementation of free is exactly the same as the original
bpf.attach_uprobe(name=obj, sym=“free”,fn_name="free_enter",pid=pid)

For the jemalloc/tcmalloc library, the implementation of free is
bpf.attach_uprobe(name=obj, sym=“je_free”,fn_name="free_enter",pid=pid)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I checked the change again. Yes, there is indeed no functionality change.

@judeng
Copy link
Contributor Author

judeng commented Dec 4, 2023

@yonghong-song thanks, last week I dealt with the conflict by manually merging the master branch, now I do not see any conflict, do I need to rebase?
And the CI have been stocking a long time, what should I do?https://github.com/iovisor/bcc/actions/runs/7001293144/job/19269436679?pr=4812

@judeng
Copy link
Contributor Author

judeng commented Dec 4, 2023

I rebase and force push this branch, not sure if this behavior is appropriate @yonghong-song

@judeng judeng requested a review from yonghong-song December 6, 2023 03:03
@judeng
Copy link
Contributor Author

judeng commented Dec 7, 2023

CI still failed, but the error looks like irrelative with this pr:

29/41 Test #29: py_test_tools_smoke ..............***Failed  294.37 sec
Traceback (most recent call last):
  File "/bcc/tools/argdist.py", line 757, in run
    self._main_loop()
  File "/bcc/tools/argdist.py", line 747, in _main_loop
    exit()
  File "/usr/lib/python3.8/_sitebuiltins.py", line 26, in __call__
    raise SystemExit(code)
SystemExit: None
..........................cannot attach kprobe, probe entry may not exist
cannot attach kprobe, probe entry may not exist
.......Killed
.WARNING: 2 stack traces lost and could not be displayed.
.........Traceback (most recent call last):
  File "/bcc/tools/offwaketime.py", line 400, in <module>
    print("    %s" % b.ksym(addr).decode('utf-8', 'replace'))
OSError: [Errno 9] Bad file descriptor
F...........Traceback (most recent call last):
  File "/bcc/tools/sslsniff.py", line 336, in <module>
    attach_nss("nspr4")
  File "/bcc/tools/sslsniff.py", line 306, in attach_nss
    b.attach_uprobe(name=lib, sym="PR_Write",
  File "/bcc/build/src/python/bcc-python3/bcc/__init__.py", line 1380, in attach_uprobe
    (path, addr) = BPF._check_path_symbol(name, sym, addr, pid, sym_off)
  File "/bcc/build/src/python/bcc-python3/bcc/__init__.py", line 978, in _check_path_symbol
    raise Exception("could not determine address of symbol %s in %s"
Exception: could not determine address of symbol PR_Write in nspr4
.............................
======================================================================
FAIL: test_offwaketime (__main__.SmokeTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/bcc/tests/python/test_tools_smoke.py", line 271, in test_offwaketime
    self.run_with_duration("offwaketime.py 1")
  File "/bcc/tests/python/test_tools_smoke.py", line 40, in run_with_duration
    self.assertEqual(0,     # clean exit
AssertionError: 0 != 1

@yonghong-song
Copy link
Collaborator

Right, CI failure is not related to your patch.

@yonghong-song yonghong-song merged commit 0ae3b12 into iovisor:master Dec 7, 2023
5 of 12 checks passed
@judeng judeng deleted the redis-memleak branch December 7, 2023 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants