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

Enable LSAN support for 32bit architecture #403

Closed
ramosian-glider opened this issue Sep 1, 2015 · 8 comments
Closed

Enable LSAN support for 32bit architecture #403

ramosian-glider opened this issue Sep 1, 2015 · 8 comments

Comments

@ramosian-glider
Copy link
Member

Originally reported on Google Code with ID 403

I am working on trying to increase the support of leak sanitizer on 32 bit architectures.
My aim is to bring up the leak identification rate, by reducing the number of false
negatives we are currently getting from junk data looking like mid pointers. On a 32
bit architecture as the address space is smaller, there is a high probability that
an allocation of 4 bytes of random junk can look like a mid pointer, leading to missed
leaks. 

Valgrind memcheck tries to relieve this problem by reporting any allocation with no
start pointer and one or more interior pointer as possibly lost, but this approach
has issues as it can often reports too many false positives; leading debuggers to often
ignore the possibly lost report. Dr. Memory goes one stage further by using heuristics
to remove some of these false positives from the reported possibly leaks (http://drmemory.org/docs/page_leaks.html)

The first stage of my attached working patch is simply the addition of "possible leaks"
on 32 bit archs (using preprocessors).
Next I would like to work on a way of removing as many false positives from the reported
"possible leaks" as possible.

We could do this by using heuristics like the ones used in dr. memory, but I know valgrind
memcheck dismissed this guessing approach as looking "scary" (https://bugs.kde.org/show_bug.cgi?id=280271).


Any ideas or thoughts would be greatly appreciated.

My testing is being done on an arm 32 bit device so my patches work on top of the following
ARM patch (https://code.google.com/p/address-sanitizer/issues/detail?id=294)

REFERENCE ISSUES: 
Port LSan to arm https://code.google.com/p/address-sanitizer/issues/detail?id=294
Leak sanitizer support for Android on arm64 https://code.google.com/p/address-sanitizer/issues/detail?id=379
https://code.google.com/p/valgrind-variant/wiki/LeakCheckingOn32bits

Reported by taylorr4 on 2015-08-11 04:45:05


- _Attachment: [32bit.patch](https://storage.googleapis.com/google-code-attachments/address-sanitizer/issue-403/comment-0/32bit.patch)_
@ramosian-glider
Copy link
Member Author

This has come up before, including the bug you mentioned above,
and it is problematic for the reasons you mostly explained. :)  
We are deliberately not enabling lsan on 32-bit --
it will have too high maintenance cost for us and we are not willing to pay it, sorry.

If you are still enthusiastic, keep updating this bug with more details/patches, 
maybe someone else will find them useful. 

Reported by konstantin.s.serebryany on 2015-08-11 05:09:27

@ramosian-glider
Copy link
Member Author

As this keeps popping up, perhaps add a descriptive message to #error in lsan header?

Reported by tetra2005 on 2015-08-16 20:57:19

@minfrin
Copy link

minfrin commented Dec 18, 2016

Lack of 32 bit support is a real shame, as it makes troubleshooting and debugging on the raspberry pi extremely difficult. This is particularly true of code that touches RPI hardware that is difficult to run on other 64 bit machines.

Given that for many people an RPi is their learning computer, it would be very useful for LeakSanitizer to be part of that learning experience.

@kcc
Copy link
Contributor

kcc commented Dec 28, 2016

That's a fair point.
The reasons why we don't want to enable lsan on 32-bit remain,
but if there is a volunteer who would like to enable and then maintain the 32-bit lsan --
I'll review the changes.

@chefmax
Copy link

chefmax commented Dec 28, 2016

Yay!
We have some experimental patches for x86 and arm for our GCC based toolchain, but it would be nice to have them upstream. So I can volunteer for contributing and maintaining LSan for these 32 bit arches. I'm going to start posting them on February if everything would be fine.

jyknight pushed a commit to jyknight/llvm-monorepo-old1 that referenced this issue Jan 23, 2017
People keep asking LSan to be available on 32 bit targets (e.g. google/sanitizers#403)
despite the fact that false negative ratio might be huge (up to 85%). This happens for big real world applications
that may contain random binary data (e.g. browser), but for smaller apps situation is not so terrible and LSan still might be useful.
This patch adds initial support for x86 Linux (disabled by default), ARM32 is in TODO list.
We used this patch (well, ported to GCC) on our 32 bit mobile emulators and it worked pretty fine
thus I'm posting it here to initiate further discussion.

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

llvm-svn=292775
@minfrin
Copy link

minfrin commented May 19, 2017

ARM32 is in TODO list

Quick ping - any progress on ARM32 for the Raspberry Pi? Back struggling with yet more Pi bugs, and the lack of memory checking is a real blocker for any useful work. Happy to rebuild my gcc toolchain if arm32 support is included?

@chefmax
Copy link

chefmax commented May 23, 2017

Sorry, missed this one.
LSan for ARM Linux was enabled in clang some time ago, see https://reviews.llvm.org/rL299923.
But please note, that to make it work, you'll need to recompile your software with -marm -fno-omit-frame-pointer options.

@eugenis
Copy link
Contributor

eugenis commented Jun 27, 2017

Sounds like this is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants