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.

Sign up for GitHub

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

Jump to bottom

LeakSanitizer and AddressSanitizer detect false leaks after fork() with threads. #836

Closed
vitalybuka opened this issue Jul 19, 2017 · 5 comments
Closed

LeakSanitizer and AddressSanitizer detect false leaks after fork() with threads. #836

vitalybuka opened this issue Jul 19, 2017 · 5 comments
Assignees
@vitalybuka

Comments

@vitalybuka
Copy link
Contributor

vitalybuka commented Jul 19, 2017 •
edited
Loading

Sanitizer detects leaks as not yet released blocks of memory which are not referenced by other live memory. After fork() sanitizer sees allocated memory, however if it is references only by TLS or stack of the parent, child can't see that as it has no those threads.

Probably can be fixed by reusing information from ThreadRegistry.
Still according #255 we don't support forking with threads.

WontFix?
@vitalybuka
Copy link
Contributor Author

bug in v8 fuzzer as example: https://bugs.chromium.org/p/chromium/issues/detail?id=740361

@vitalybuka
Copy link
Contributor Author

Should we to the user if we forking process with thread?

@vitalybuka vitalybuka changed the title LeakSanitizer and AddressSanitizer false-positive reports after fork() with threads. LeakSanitizer and AddressSanitizer detect false leaks after fork() with threads. Jul 19, 2017
@chefmax
Copy link

chefmax commented Jul 20, 2017

I tried to fix fork safety of ASan/MSan allocators some time ago (https://reviews.llvm.org/D33325) but the patch appeared to be incomplete. I hope I can try another approach (with intercepting pthread_atfork and acquiring necessary locks during ASan initialization) in near future (probably on the next week).

@vitalybuka
Copy link
Contributor Author

vitalybuka commented Jul 20, 2017 •
edited
Loading

Is it worth it?
Another perspective on these leaks that they are real leaks.
We have forker process with allocated objects. However nothing in new process will release these objects, and nothing there will use them as only threads in child had access there.
Same probably is true for locks.

@chefmax
Copy link

chefmax commented Jul 20, 2017

I'm not sure about this. My initial motivation about fork safety was to make sanitizer allocator more "compliant" with other modern allocators (Glibc, tcmalloc, jemalloc) that are fork safe. This would allow us to avoid "sudden" deadlocks with sanitizers (some user code still relies on allocator fork safety). But I'm not sure whether this should be fixed in sanitizer allocator, perhaps the right thing to do is to fix the code itself.

dtzWill pushed a commit to llvm-mirror/compiler-rt that referenced this issue Jun 5, 2018
@vitalybuka
[lsan] Do not check for leaks in the forked process
7f47a0b 
Summary:
If calling process had threads then forked process will fail to detect
references from them.

Fixes google/sanitizers#836

Reviewers: alekseyshl

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D47751

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@334036 91177308-0d34-0410-b5e6-96231b3b80d8
jyknight pushed a commit to jyknight/llvm-monorepo that referenced this issue Jun 5, 2018
@vitalybuka
[lsan] Do not check for leaks in the forked process
1625ae0 
Summary:
If calling process had threads then forked process will fail to detect
references from them.

Fixes google/sanitizers#836

Reviewers: alekseyshl

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D47751

llvm-svn=334036
@vitalybuka vitalybuka self-assigned this Jun 5, 2018
krytarowski pushed a commit to plusun/compiler-rt that referenced this issue Jun 7, 2018
@vitalybuka @krytarowski
[lsan] Do not check for leaks in the forked process
38961de 
Summary:
If calling process had threads then forked process will fail to detect
references from them.

Fixes google/sanitizers#836

Reviewers: alekseyshl

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D47751

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@334036 91177308-0d34-0410-b5e6-96231b3b80d8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vitalybuka vitalybuka

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@vitalybuka @chefmax