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

Container-overflow false positive with uninstrumented code #362

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

Container-overflow false positive with uninstrumented code #362

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

Comments

@ramosian-glider
Copy link
Member

Originally reported on Google Code with ID 362

Imagine two modules, both using std::vector, only one of them is instrumented. Push_back
from the non-instrumented module will not unpoison the memory for the newly added element.
Access from the instrumented module will trigger a false report.

Note that these two modules don't need to share a common instance of std::vector. Theoretically,
it is possible for the linker to choose the uninstrumented version of std::vector::push_back
(if it was not inlined) to be called from the instrumented module.

Reported by eugenis@google.com on 2014-12-08 09:26:48

@Myushu
Copy link

Myushu commented May 30, 2016

Hi,

Is there a way to work around this issue? (that's not "recompile all your deps with -fanitize)

Thanks.

@yugr
Copy link

yugr commented May 30, 2016

Is there a way to work around this issue? (that's not "recompile all your deps with -fanitize)

Probably not (and yes, this seems to be a serious limitation of container sanitization).

@kcc
Copy link
Contributor

kcc commented May 30, 2016

@yugr
Copy link

yugr commented May 30, 2016

@kcc shouldn't container annotations be disabled by default then? Because they otherwise prohibit separate sanitization which is a very popular use-case...

@kcc
Copy link
Contributor

kcc commented May 30, 2016

Default values are hard :)
Our major customers would not care about default values because they set those values themselves,
e.g. see here, so we mostly need to worry about smaller customers.

If we make this feature off by default users will never know about it (people generally don't read docs).
We've found enough container overflows to think that this would be very sad.
On the other hand, if we keep the feature on by default, some of the users will be confused by the false positives. Currently asan prints a link to the above wiki explaining the situation, so the confusion is hopefully minimal. And it will be an extra push for Linux distros to finally start shipping asan-itized libraries.

@dvyukov
Copy link
Contributor

dvyukov commented May 31, 2016

Is there a way to work around this issue? (that's not "recompile all your deps with -fanitize)

You can work around the issue by running with ASAN_OPTIONS=detect_container_overflow=0 env var.

But note that AddressSanitizer finds only a subset of bugs. And to use ThreadSanitizer and MemorySanitizer most likely you need to rebuild all you deps.

@vendethiel
Copy link

Thanks for your answers!

@yugr
Copy link

yugr commented May 31, 2016

So close as not-a-bug then?

@morehouse
Copy link
Contributor

WAI.

@yugr
Copy link

yugr commented Jun 6, 2018

FYI GCC disables container instrumentation by default (user needs to define _GLIBCXX_SANITIZE_VECTOR explicitly), probly because they care more about "small" users.

chenyt9 pushed a commit to MotorolaMobilityLLC/system-core that referenced this issue Nov 3, 2021
…tact

 - workround for ASAN to avoid ASAN container heap detact
 - more information refer google/sanitizers#362
 - This is a temp patch, I will close ASAN when stable line

Change-Id: Ib338dd475244b10e6834c435a6754a5dd5147bbe
Reviewed-on: https://gerrit.mot.com/2030323
SME-Granted: SME Approvals Granted
Tested-by: Jira Key
Reviewed-by: Huosheng Liao <liaohs@motorola.com>
Reviewed-by: Yifu Lin <linyf@mt.com>
Submit-Approved: Jira Key
SLTApproved: Zhiyi Cai <caizy@motorola.com>
chenyt9 pushed a commit to MotorolaMobilityLLC/system-core that referenced this issue Oct 11, 2022
…tact

 - workround for ASAN to avoid ASAN container heap detact
 - more information refer google/sanitizers#362
 - This is a temp patch, I will close ASAN when stable line

Change-Id: Ib338dd475244b10e6834c435a6754a5dd5147bbe
Reviewed-on: https://gerrit.mot.com/2030323
SME-Granted: SME Approvals Granted
Tested-by: Jira Key
Reviewed-by: Huosheng Liao <liaohs@motorola.com>
Reviewed-by: Yifu Lin <linyf@mt.com>
Submit-Approved: Jira Key
SLTApproved: Zhiyi Cai <caizy@motorola.com>
(cherry picked from commit 87b5a4d08509ef73cb4f40391a20445e198e586e)
Reviewed-on: https://gerrit.mot.com/2049904
SLTApproved: Slta Waiver
Reviewed-by: Thiago Kunsch Rocha <tkunsch@motorola.com>
Submit-Approved: Thiago Kunsch Rocha <tkunsch@motorola.com>
Tested-by: Thiago Kunsch Rocha <tkunsch@motorola.com>
Reviewed-by: Konstantin Makariev <kmakariev@motorola.com>
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

7 participants