Page MenuHomePhabricator

m.ostapenko (Maxim Ostapenko)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 16 2014, 10:32 PM (273 w, 6 d)

Recent Activity

Dec 4 2018

m.ostapenko added a comment to D55156: [asan] Split -asan-use-private-alias to -asan-use-odr-indicator.

So I am not sure if there is any value in using aliases without indicators.

Dec 4 2018, 12:42 AM

Dec 3 2018

m.ostapenko accepted D55146: [asan] Reduce binary size by using unnamed private aliases.

I think this should be fine, aliases don't need names at all.

Dec 3 2018, 6:56 AM
m.ostapenko added a comment to D55156: [asan] Split -asan-use-private-alias to -asan-use-odr-indicator.

Hm, I'd rather separate private aliases and odr indicators because they are separate (although connected) features:

Dec 3 2018, 6:54 AM

Sep 5 2018

m.ostapenko accepted D50180: [Sanitizers] Libasan failed to be build with -mthumb and -fno-omit-frame-pointer by GCC.

lgtm

Sep 5 2018, 10:45 AM
m.ostapenko updated subscribers of D50180: [Sanitizers] Libasan failed to be build with -mthumb and -fno-omit-frame-pointer by GCC.

Oh, sorry, forgot about this :(. Please rebase and upload new version.
BTW, please don't forget to add 'llvm-commits' to subscribers so your patch will be visible in ML.

Sep 5 2018, 2:10 AM

Aug 13 2018

m.ostapenko accepted D50180: [Sanitizers] Libasan failed to be build with -mthumb and -fno-omit-frame-pointer by GCC.

Looks good. Will commit this tomorrow if nobody objects.

Aug 13 2018, 12:15 PM

Aug 2 2018

m.ostapenko added inline comments to D50180: [Sanitizers] Libasan failed to be build with -mthumb and -fno-omit-frame-pointer by GCC.
Aug 2 2018, 6:11 AM
m.ostapenko accepted D50180: [Sanitizers] Libasan failed to be build with -mthumb and -fno-omit-frame-pointer by GCC.

Looks good, but let's wait for code owners to accept this as well.

Aug 2 2018, 5:21 AM

Mar 6 2018

m.ostapenko added a comment to D16403: Add scope information to CFG.

Now, the cfg for this example:

void test_for_implicit_scope() {
  for (A a; A b = a;)
    A c;
}

looks like this:

Mar 6 2018, 4:01 AM · Restricted Project
m.ostapenko updated the diff for D16403: Add scope information to CFG.

Rebased and updated. Fix the issue with ordering between ScopeEnds and implicit destructors.

Mar 6 2018, 3:58 AM · Restricted Project

Feb 20 2018

m.ostapenko added a comment to D16403: Add scope information to CFG.
In D16403#1001466, @NoQ wrote:

I poked Devin offline and we agreed that the overall approach is good to go. Maxim, thank you for picking it up!

We still don't have scopes for segments of code that don't have any variables in them, so i guess it's not yet in the shape where it is super useful for loops, but it's already useful for finding use of stale stack variables, which was the whole point originally, so i think this should definitely land soon.

In D16403#992452, @NoQ wrote:

Am i understanding correctly that while destroying a you can still use the storage of b safely? Or should a go out of scope before b gets destroyed?

AFAIU, when we destroying a we can still use the storage of b because nothing can be allocated into b's storage between calling destructors for b and a. So, imho the sequence should look like:

[B4.5].~A() (Implicit destructor)
[B5.3].~A() (Implicit destructor)
CFGScopeEnd(b)
CFGScopeEnd(a)

Thought about it a bit more and this still doesn't look correct to me. Like, a.~A() is a function call. It can do a lot of unexpected stuff to registers and stack space before jumping into the function. And given that a and b are in different scopes (a is in loop scope, b is in single iteration scope), storage of b is not protected from such stuff during call to the destructor of a. So there's definitely something fishy here. I guess scope ends and destructors would have to be properly interleaved in all cases of exiting multiple scopes.

Feb 20 2018, 9:39 AM · Restricted Project
m.ostapenko added a comment to D16403: Add scope information to CFG.
In D16403#1011218, @NoQ wrote:
In D16403#992454, @NoQ wrote:

@szepet: so i see that LoopExit goes in the beginning of the cleanup block after the loop, while various ScopeEnds go after the LoopExit. Would loop unrolling be significantly broken if you simply subscribe to ScopeEnd instead of LoopExit and avoid cleaning up the loop state until destructors are processed? I might not be remembering correctly - is LoopExit only used for cleanup, or do we have more work to be done here?

I guess your following comment just answers this:

In D16403#1001466, @NoQ wrote:

We still don't have scopes for segments of code that don't have any variables in them, so i guess it's not yet in the shape where it is super useful for loops, but it's already useful for finding use of stale stack variables, which was the whole point originally, so i think this should definitely land soon.

It could be, however, we would lose cases like:

int i = 0;
int a[32];
for(i = 0;i<32;++i) {a[i] = i;}

Since there is no variable which has the scope of the loop, ScopeEnd would be not enough. Sure, we could remove this case, however, the aim is to extend the loop-patterns for completely unrolling. Another thing that there are the patches which would enhance the covered cases by LoopExit (D39398) and add the LoopEntrance to the CFG(D41150) as well. So, at this point, I feel like it would be a huge step back not to use these elements. (Sorry Maxim that we are discussing this here^^)

Yeah, i mean, like, if we change the scope markers to also appear even when no variables are present in the scope, then it would be possible to replace loop markers with some of the scope markers, right?

Feb 20 2018, 9:39 AM · Restricted Project

Feb 1 2018

m.ostapenko updated the diff for D16403: Add scope information to CFG.

Fix scope ends order (as discussed above) and adjust a testcase.

Feb 1 2018, 8:56 AM · Restricted Project

Jan 31 2018

m.ostapenko added a comment to D16403: Add scope information to CFG.
In D16403#992452, @NoQ wrote:

Thank you, this explanation looks very reasonable.

All right, so right after the termination of the loop we have

[B1]
1: ForStmt (LoopExit)
2: [B4.5].~A() (Implicit destructor)
3: [B5.3].~A() (Implicit destructor)
4: CFGScopeEnd(a)
5: CFGScopeEnd(b)

... where [B4.5] is A b = a; and [B5.3] is A a;. Am i understanding correctly that while destroying a you can still use the storage of b safely? Or should a go out of scope before b gets destroyed?

Jan 31 2018, 9:44 AM · Restricted Project

Jan 29 2018

m.ostapenko added a comment to D16403: Add scope information to CFG.

Actually upload the files.

Jan 29 2018, 8:34 AM · Restricted Project
m.ostapenko added a comment to D16403: Add scope information to CFG.

Hi Artem,

Jan 29 2018, 8:32 AM · Restricted Project

Jan 23 2018

m.ostapenko updated the diff for D16403: Add scope information to CFG.

Rebased and ping.

Jan 23 2018, 9:03 AM · Restricted Project

Jan 19 2018

m.ostapenko created D42303: [lsan] Respect log_path option in standalone LSan.
Jan 19 2018, 8:45 AM · Restricted Project

Jan 17 2018

m.ostapenko updated the diff for D16403: Add scope information to CFG.

Some code cleanup + updated test case.

Jan 17 2018, 7:37 AM · Restricted Project

Jan 16 2018

m.ostapenko updated the diff for D16403: Add scope information to CFG.

Hi Devin,

Jan 16 2018, 10:35 AM · Restricted Project

Dec 18 2017

m.ostapenko added a comment to D40951: [ASan] Add interceptor for printf_chk.

Hi, I've committed this on behalf of Denis. @denis13 Will ping you if some buildbots complain about this change.

Dec 18 2017, 7:34 AM · Restricted Project

Nov 30 2017

m.ostapenko added reviewers for D40349: [LSan] New experimental flag for background leak checking before exit.: kcc, vitalybuka, alekseyshl.

Adding maintainers + some comments.

Nov 30 2017, 8:08 AM

Sep 19 2017

m.ostapenko updated subscribers of D38026: Make LSan compliant with recovery mode when running on top of ASan.
Sep 19 2017, 2:09 AM

Aug 24 2017

m.ostapenko updated the diff for D16403: Add scope information to CFG.

Ping^4

Aug 24 2017, 6:45 AM · Restricted Project

Aug 16 2017

m.ostapenko updated the diff for D16403: Add scope information to CFG.

Ping^3

Aug 16 2017, 3:11 AM · Restricted Project

Aug 9 2017

m.ostapenko updated the diff for D16403: Add scope information to CFG.

Rebased and ping.

Aug 9 2017, 12:48 AM · Restricted Project

Aug 2 2017

m.ostapenko updated subscribers of D35409: Add new ASAN_OPTION: sleep_after_init..
Aug 2 2017, 2:30 AM

Jul 28 2017

m.ostapenko updated the diff for D16403: Add scope information to CFG.

Updated some comments. Could someone take a look please?

Jul 28 2017, 7:08 AM · Restricted Project

Jul 24 2017

m.ostapenko updated the diff for D16403: Add scope information to CFG.

Rebased and removed a bunch of stale changes. Also added a check for goto's: if we see GotoStmt and have cfg-scopes == true, make badCFG = true and retry without scopes enabled. This check will be removed once GotoStmt will become supported.

Jul 24 2017, 6:15 AM · Restricted Project

Jul 13 2017

m.ostapenko added a comment to D16403: Add scope information to CFG.
In D16403#808104, @NoQ wrote:

I think the remaining switch-related code seems to be about C++17 switch condition variables, i.e. switch (int x = ...)(?)

Jul 13 2017, 9:03 AM · Restricted Project
m.ostapenko updated the diff for D16403: Add scope information to CFG.

Updating the diff. I've dropped SwitchStmt support, let's implement in separately as well as GotoStmt.

Jul 13 2017, 4:39 AM · Restricted Project

Jul 10 2017

m.ostapenko added a comment to D16403: Add scope information to CFG.

Hi Artem, I'm sorry for a long delay (nasty corporate issues).

Jul 10 2017, 3:15 AM · Restricted Project

Jun 26 2017

m.ostapenko added a project to D34100: fix the bug of opening the file with the bytes can't be decoded to 'utf-8': Restricted Project.
Jun 26 2017, 5:54 AM · Restricted Project

Jun 23 2017

m.ostapenko updated the diff for D16403: Add scope information to CFG.

So, updating the diff. This is still a very experimental version and any feedback would be greatly appreciated.
Current patch should support basic {If, While, For, Compound, Switch}Stmts as well as their interactions with {Break, Continue, Return}Stmts.
GotoStmt and CXXForRangeStmt are not supported at this moment.

Jun 23 2017, 5:43 AM · Restricted Project
m.ostapenko commandeered D16403: Add scope information to CFG.
Jun 23 2017, 4:55 AM · Restricted Project

Jun 22 2017

m.ostapenko added a comment to D16403: Add scope information to CFG.

unless someone has objections (@bshastry ?) I'll commandeer this revision and post updated patch shortly (it still needs some polishing though).

Jun 22 2017, 8:52 AM · Restricted Project

Jun 14 2017

m.ostapenko added a comment to D34210: Add __has_feature(leak_sanitizer).

Currently, the way that we tell users to gate on sanitizer-specific behavior is with __has_feature(foo_sanitizer), as far as I know, it's the only way to do so. LSan provides several API functions for users, ie __lsan_ignore_object. If a user program wants to use these API functions in their program, they need a way to check that LSan is enabled at compilation time (even if LSan doesn't actually modify the compile step). I'm not sure of a better or more consistent way to allow that to happen.

Jun 14 2017, 11:14 AM

Jun 13 2017

m.ostapenko added inline comments to D34149: [ASAN] ASAN is not properly calling libbacktrace to symbolize program.
Jun 13 2017, 11:06 AM · Restricted Project
m.ostapenko updated subscribers of D34149: [ASAN] ASAN is not properly calling libbacktrace to symbolize program.
Jun 13 2017, 9:05 AM · Restricted Project

Jun 8 2017

m.ostapenko added a comment to D33325: [sanitizer] Avoid possible deadlock in child process after fork.

Oh, sorry.

This change is causing problems on Linux. The immediate issue is Tcl, which creates new threads from a pthread_atfork() child handler. The handler is called from libc's fork(), and pthread_create tries to acquire the StackDepot lock before it is released in the fork interceptor.

Won't this deadlock with MSan as well? AFAIK it also locks StackDepot in fork interceptor...

Maybe it does not use StackDepot in pthread_create?

Tcl is broken, of course:

If a fork() call in a multi-threaded process leads to a child fork handler calling any function that is not async-signal-safe, the behavior is undefined.

But in general, even malloc() called from pthread_atfork() will deadlock with this change.
It looks like the right way to do this is call pthread_atfork() as early as possible to setup lock/unlock handlers for the allocator and the stack depot.

Perhaps we can do it from asan_init? From man:
"The order of calls to pthread_atfork() is significant. The parent and child fork handlers shall be called in the order in which they were established by calls to pthread_atfork(). The prepare fork handlers shall be called in the opposite order."

So in this case our unlock routines will be called before others. This still can deadlock when preloading asan to noinstrumented binary though.

Sounds like that's the best we can do. If we are lucky, sanitizer initialization will happen early enough due to ENSURE_ASAN_INIT in one of the interceptors.

Jun 8 2017, 9:14 AM · Restricted Project

Jun 7 2017

m.ostapenko added inline comments to D33784: Bug 33206 - Sanitizer CHECK failed: ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (1036, 1024)) with preload.
Jun 7 2017, 11:45 AM · Restricted Project

Jun 6 2017

m.ostapenko added a comment to D33784: Bug 33206 - Sanitizer CHECK failed: ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (1036, 1024)) with preload.

A crazy thought - what happens if we add DF_1_INITFIRST to the asan library?
-Wl,-z,initfirst

Jun 6 2017, 12:13 PM · Restricted Project
m.ostapenko added a comment to D33784: Bug 33206 - Sanitizer CHECK failed: ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (1036, 1024)) with preload.

Please add a testcase. For me, following commands work:
$ cat foo.c

#include <stdlib.h>
Jun 6 2017, 9:50 AM · Restricted Project
m.ostapenko created D33941: [Driver] Add test to cover case when LSan is not supported.
Jun 6 2017, 7:09 AM

Jun 5 2017

m.ostapenko added a comment to D33325: [sanitizer] Avoid possible deadlock in child process after fork.

This change is causing problems on Linux. The immediate issue is Tcl, which creates new threads from a pthread_atfork() child handler. The handler is called from libc's fork(), and pthread_create tries to acquire the StackDepot lock before it is released in the fork interceptor.

Jun 5 2017, 2:57 PM · Restricted Project
m.ostapenko added inline comments to D33784: Bug 33206 - Sanitizer CHECK failed: ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (1036, 1024)) with preload.
Jun 5 2017, 2:05 PM · Restricted Project

Jun 2 2017

m.ostapenko added a comment to D33784: Bug 33206 - Sanitizer CHECK failed: ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (1036, 1024)) with preload.

Yeah... that's weird.
Can we simply do ENSURE_ASAN_INITED(); thing in the malloc interceptor?

Jun 2 2017, 6:32 AM · Restricted Project
m.ostapenko updated subscribers of D33784: Bug 33206 - Sanitizer CHECK failed: ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (1036, 1024)) with preload.
Jun 2 2017, 12:24 AM · Restricted Project

Jun 1 2017

m.ostapenko added a comment to D33784: Bug 33206 - Sanitizer CHECK failed: ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (1036, 1024)) with preload.

Are you saying that LD_PRELOAD-ed library constructors are run after some other library constructors, and that other library is not a dependency of the LD_PRELOAD-ed one? I've never seen that. Is that on linux/glibc?

Jun 1 2017, 11:23 AM · Restricted Project
m.ostapenko added a comment to D33784: Bug 33206 - Sanitizer CHECK failed: ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (1036, 1024)) with preload.

Let me share more background.
The problem occurs in such rare cases when we have one instrumented library and uninstrumented binary with lots of dependencies from other (uninstrumented) shared libs. In this case we can use LD_PRELOAD trick to preload shared ASan runtime to uninstrumented binary, but since rtld actually doesn't specify order of constructors execution, we can end up with overflow in static alloc_memory_for_dlsym pool if some constructor has malloc with some big size and this constructor is called before __asan_init. The right solution would be just build the executable with ASan, but unfortunately sometimes developers can't do this.

Jun 1 2017, 11:11 AM · Restricted Project

May 31 2017

m.ostapenko added a project to D33712: Bug 33221 [UBSAN] segfault with -fsanitize=undefined: Restricted Project.
May 31 2017, 1:36 AM · Restricted Project
m.ostapenko updated subscribers of D33712: Bug 33221 [UBSAN] segfault with -fsanitize=undefined.

Please add a testcase (you can take it from Vedant's patch in PR).

May 31 2017, 1:35 AM · Restricted Project

May 30 2017

m.ostapenko updated the diff for D33325: [sanitizer] Avoid possible deadlock in child process after fork.

Rebased. Gentle reminder.

May 30 2017, 7:23 AM · Restricted Project

May 24 2017

m.ostapenko updated the diff for D33325: [sanitizer] Avoid possible deadlock in child process after fork.

Removing TSan part. The attached testcase still hangs with TSan, but since locking allocator is problematic let's leave TSan alone.

May 24 2017, 9:08 AM · Restricted Project

May 23 2017

m.ostapenko updated the diff for D33325: [sanitizer] Avoid possible deadlock in child process after fork.

Addressing Dmitry's comments.

May 23 2017, 8:50 AM · Restricted Project
m.ostapenko added a comment to D33325: [sanitizer] Avoid possible deadlock in child process after fork.

I don't completely understand this. allocator_fork_no_hang.cc is broken and deserves deadlocking. Why are we trying to fix this program only under sanitizers rather than detecting and reporting the bug? Sanitizers are meant to be less forgiving than production environment. Why are we trying to conceal the bug?

May 23 2017, 7:01 AM · Restricted Project
m.ostapenko updated the diff for D33325: [sanitizer] Avoid possible deadlock in child process after fork.

Updating. Reproduced the deadlock with TSan and LSan on adjusted testcase. Adding Dmitry since the patch now affects TSan code as well.

May 23 2017, 5:49 AM · Restricted Project
m.ostapenko added inline comments to D33325: [sanitizer] Avoid possible deadlock in child process after fork.
May 23 2017, 4:58 AM · Restricted Project

May 22 2017

m.ostapenko updated the diff for D33325: [sanitizer] Avoid possible deadlock in child process after fork.

Updating. Lock allocator on fork for {A, M, T}San and moving testcase to sanitizer_common. I've also managed to reproduce the deadlock with MSan, but still not with TSan.

May 22 2017, 9:28 AM · Restricted Project

May 19 2017

m.ostapenko added a comment to D33325: [sanitizer] Avoid possible deadlock in child process after fork.

Good catch. This is a problem for MSan as well, is not it? Do you mind fixing it in both tools?

May 19 2017, 1:58 PM · Restricted Project
m.ostapenko updated the diff for D33325: [sanitizer] Avoid possible deadlock in child process after fork.

Updating. When adapting test/msan/fork.cc for ASan I've noticed that deadlock can also happen if the whole allocator is locked when we calling fork. Corresponding backtrace looks like this:

#0  atomic_exchange<__sanitizer::atomic_uint32_t> () at /home/max/src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:68
#1  Lock () at /home/max/src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_linux.cc:541
#2  0x000000000041e9f9 in GenericScopedLock () at /home/max/src/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_mutex.h:187
#3  GetFromAllocator () at /home/max/src/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_allocator_primary64.h:134
#4  0x000000000041e99b in Refill () at /home/max/src/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_allocator_local_cache.h:108
#5  0x000000000041e550 in Allocate () at /home/max/src/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_allocator_local_cache.h:50
#6  0x000000000041e443 in Allocate () at /home/max/src/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_allocator_combined.h:68
#7  0x000000000041b6cb in Allocate () at /home/max/src/llvm/projects/compiler-rt/lib/asan/asan_allocator.cc:407
#8  0x00000000004b9148 in __interceptor_malloc () at /home/max/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:67
#9  0x00000000004e878d in child () at fork.cc:47
#10 0x00000000004e8968 in test () at fork.cc:67
#11 0x00000000004e8a38 in main () at fork.cc:78

Thus we need to lock allocator as well.

May 19 2017, 7:22 AM · Restricted Project

May 18 2017

m.ostapenko added a comment to D33325: [sanitizer] Avoid possible deadlock in child process after fork.

Look at test/msan/fork.cc, it should be possible to replace copy_uninit_thread2 with an allocation loop.

May 18 2017, 11:40 AM · Restricted Project
m.ostapenko added a comment to D33325: [sanitizer] Avoid possible deadlock in child process after fork.
In D33325#758753, @kcc wrote:

is a test possible?

May 18 2017, 11:10 AM · Restricted Project
m.ostapenko created D33325: [sanitizer] Avoid possible deadlock in child process after fork.
May 18 2017, 10:46 AM · Restricted Project
m.ostapenko updated subscribers of D31266: [sancov] fixing too aggressive instrumentation elimination.
May 18 2017, 12:29 AM

May 2 2017

m.ostapenko updated the diff for D32589: [sanitizer] Intercept mcheck and mprobe on Linux.

Add guards against old Glibc versions to testcase.

May 2 2017, 10:54 AM · Restricted Project

Apr 27 2017

m.ostapenko updated the diff for D32589: [sanitizer] Intercept mcheck and mprobe on Linux.

Remove INIT_MCHECK_MPROBE.

Apr 27 2017, 11:49 PM · Restricted Project
m.ostapenko updated the diff for D32589: [sanitizer] Intercept mcheck and mprobe on Linux.

Add mcheck_pedantic.

Apr 27 2017, 11:40 AM · Restricted Project
m.ostapenko added a comment to D32589: [sanitizer] Intercept mcheck and mprobe on Linux.

What about mcheck_pedantic and mcheck_check_all?

Apr 27 2017, 11:15 AM · Restricted Project
m.ostapenko updated the diff for D32589: [sanitizer] Intercept mcheck and mprobe on Linux.

Remove enum.

Apr 27 2017, 5:57 AM · Restricted Project
m.ostapenko added inline comments to D32589: [sanitizer] Intercept mcheck and mprobe on Linux.
Apr 27 2017, 5:44 AM · Restricted Project
m.ostapenko created D32589: [sanitizer] Intercept mcheck and mprobe on Linux.
Apr 27 2017, 5:03 AM · Restricted Project

Apr 17 2017

m.ostapenko created D32128: [sanitizer] Don't include <linux/user.h> in sanitizer_stoptheworld_linux_libcdep.cc on ARM Android..
Apr 17 2017, 10:45 AM · Restricted Project

Apr 13 2017

m.ostapenko added a comment to D32007: [lsan] Reenable lsan tests on ARM bots.

Thanks! Watching for buildbots...

Apr 13 2017, 5:14 AM · Restricted Project
m.ostapenko created D32007: [lsan] Reenable lsan tests on ARM bots.
Apr 13 2017, 4:40 AM · Restricted Project

Apr 12 2017

m.ostapenko accepted D31977: Use 0-padding for i386 and arm print format specifiers.

lgtm

Apr 12 2017, 7:34 AM

Apr 11 2017

m.ostapenko added a comment to D29586: [lsan] Enable LSan for arm Linux.

This breaks bootstrap builds. I put the error message in the revert commit description: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20170410/444101.html

Apr 11 2017, 7:48 AM · Restricted Project

Apr 10 2017

m.ostapenko updated the diff for D31760: [lsan] Enable LSan on arm Linux, clang part.

Add armeb and thumbeb for completeness.
Just curious, does anyone use sanitizers in these targets? I see no public buildbots for armeb and thumbeb. Anyway, not a big deal of course.

Apr 10 2017, 11:32 AM · Restricted Project

Apr 7 2017

m.ostapenko updated the diff for D31760: [lsan] Enable LSan on arm Linux, clang part.

Also check for thumb.

Apr 7 2017, 1:03 AM · Restricted Project

Apr 6 2017

m.ostapenko created D31760: [lsan] Enable LSan on arm Linux, clang part.
Apr 6 2017, 6:57 AM · Restricted Project

Apr 5 2017

m.ostapenko added a comment to D29586: [lsan] Enable LSan for arm Linux.

Going back to the issue with interceptor_malloc not saving FP when built with GCC. In clang a function using builtin_frame_address(0) automatically gets a proper frame pointer. As far as I can see, arm-gcc in the android ndk (version 4.9) behaves the same way on a simple test case. Is that some new optimization where a frame address is computed on the fly?

Apr 5 2017, 9:50 AM · Restricted Project
m.ostapenko added inline comments to D30818: [lsan] Don't handle DTLS of thread under destruction.
Apr 5 2017, 8:33 AM · Restricted Project

Apr 4 2017

m.ostapenko updated the diff for D29586: [lsan] Enable LSan for arm Linux.

Updating according to Aleksey's comments.

Apr 4 2017, 7:26 AM · Restricted Project

Apr 3 2017

m.ostapenko updated the diff for D30818: [lsan] Don't handle DTLS of thread under destruction.

Turned out I was wrong again. Looking to verbose log more carefully, one can see that segfault happens when GetRegistersAndSP fails with errno 3 (ESRCH):

==13657==Attached to thread 14860.
==13657==Attached to thread 14868.
==13657==Attached to thread 13349.
==13657==Attached to thread 13357.
==13657==Could not get registers from thread 13349 (errno 3).
==13657==Unable to get registers from thread 13349.
Tracer caught signal 11: addr=0x7ff4b259b000 pc=0x4239d0 sp=0x7ff4b1d99d90
==13657==Process memory map follows:
...
        0x7ff4b1d9b000-0x7ff4b259b000    0x000000000003 (rw)
        0x7ff4b259b000-0x7ff4b259c000    0x000000000000
...
==13657==End of process memory map.
==13657==Detached from thread 14860.
==13657==Detached from thread 14868.
==13657==Could not detach from thread 13349 (errno 3).
==13657==Detached from thread 13357.
==14860==LeakSanitizer has encountered a fatal error.
==14860==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
==14860==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)

Although LSan successfully attached to thread 13349, it seems that this thread was killed by concurrent SIGKILL signal. Thus stack boundaries extracted by GetRegistersAndSP are already invalid and we access "bad" memory (guard page in this case).
According to ptrace manual, when user tries to get information from ptrace stopped thread he should always be ready to handle ESRCH error:

Apr 3 2017, 11:08 AM · Restricted Project
m.ostapenko updated the diff for D29586: [lsan] Enable LSan for arm Linux.

Rebased. Ping.

Apr 3 2017, 6:03 AM · Restricted Project

Mar 30 2017

m.ostapenko updated the diff for D31420: [asan] Move AsanCheckDynamicRTPrereqs check under flag..

Changed flag name.

Mar 30 2017, 2:54 AM · Restricted Project

Mar 29 2017

m.ostapenko accepted D31462: Enable leak detection on linux-i686 by default.

Looks good, thank you.

Mar 29 2017, 7:21 AM
m.ostapenko created D31456: [sanitizer] Move fread and fwrite interceptors to sanitizer_common.
Mar 29 2017, 2:53 AM · Restricted Project
m.ostapenko added a comment to D31430: Disable asan leak tests on i386 linux.

Well, the issue is that currently, we aren't respecting the default flags as set in sanitizer_flags.inc when we run on asan. asan_flags.cc overrides the defaults and replaces them with CAN_SANITIZE_LEAKS. This means that leak checking will always run on asan builds, even when the default is set to disabled. This isn't causing issues for linux-i386, as most of the tests pass even though it's technically supposed to be disabled. However, for Darwin, the tests will all fail, so we need to ensure that the default values are respected.

So why not make LSan on Darwin pass these tests instead of disabling them on Linux?

Because it hasn't been fully implemented yet, and we want build-only coverage to ensure that nobody breaks the build in the meantime.

When the defaults are respected, asan will not have leak detection by default on linux-i386, which means that asan tests requiring leak-detection will fail.

Why would we want to disable LSan on x86 Linux by default? It would lead to inconsistency with other architectures default value that may confuse users.

All x86 builds for lsan are currently disabled by default, due to the very high false-negative rate (I believe that patch blames to you actually).

Mar 29 2017, 1:59 AM

Mar 28 2017

m.ostapenko added a comment to D31430: Disable asan leak tests on i386 linux.

Well, the issue is that currently, we aren't respecting the default flags as set in sanitizer_flags.inc when we run on asan. asan_flags.cc overrides the defaults and replaces them with CAN_SANITIZE_LEAKS. This means that leak checking will always run on asan builds, even when the default is set to disabled. This isn't causing issues for linux-i386, as most of the tests pass even though it's technically supposed to be disabled. However, for Darwin, the tests will all fail, so we need to ensure that the default values are respected.

Mar 28 2017, 12:09 PM
m.ostapenko added a comment to D31430: Disable asan leak tests on i386 linux.

Could you elaborate, what kind of issue are you trying to solve? LSan in ASan seems to work fine on x86 Linux, it looks like a wrong idea to disable tests there.

Mar 28 2017, 11:52 AM
m.ostapenko added inline comments to D31420: [asan] Move AsanCheckDynamicRTPrereqs check under flag..
Mar 28 2017, 7:45 AM · Restricted Project
m.ostapenko created D31420: [asan] Move AsanCheckDynamicRTPrereqs check under flag..
Mar 28 2017, 7:07 AM · Restricted Project
m.ostapenko added a comment to D30504: [sanitizer] Bail out with warning if user dlopens shared library with RTLD_DEEPBIND flag.

We have a test in chromium that loads a small so file and calls some of the functions therein. There are no allocations in the so file. The test used to run fine under tsan, now it fails with the error message you added. I'll disable the test under tsan, but maybe others out there where also calling non-allocating so's without problems before this change.

Mar 28 2017, 4:18 AM · Restricted Project

Mar 27 2017

m.ostapenko updated the diff for D29586: [lsan] Enable LSan for arm Linux.

Rebased. Gentle reminder.

Mar 27 2017, 5:39 AM · Restricted Project

Mar 23 2017

m.ostapenko added a comment to D31300: Disable use_tls_dynamic on 32-bit linux.

Shouldn't this be "UNSUPPORTED" instead of "XFAIL"?

Mar 23 2017, 11:25 PM

Mar 22 2017

m.ostapenko added a comment to D30818: [lsan] Don't handle DTLS of thread under destruction.

After more debugging it seems that the issue is even more complicated.

Tracer caught signal 11: addr=0x7fb108da4000 pc=0x423990 sp=0x7fb135ffed90
...
        0x7fb1085a4000-**0x7fb108da4000**	 rw (0x000000000003)

This address does not look accessible to me. Well, that depends on the next mapping.

Mar 22 2017, 2:20 PM · Restricted Project

Mar 21 2017

m.ostapenko added a comment to D30818: [lsan] Don't handle DTLS of thread under destruction.

After more debugging it seems that the issue is even more complicated.

Tracer caught signal 11: addr=0x7fb108da4000 pc=0x423990 sp=0x7fb135ffed90
==9109==Process memory map follows:
	0x000000400000-0x000000441000	/tmp/a.out 0x000000000005
	0x000000641000-0x000000642000	/tmp/a.out 0x000000000001
	0x000000642000-0x000000645000	/tmp/a.out 0x000000000003
...
        0x7fb1085a4000-**0x7fb108da4000**	 rw (0x000000000003)

The faulty address looks fine (addressable and accessible), but the problem seems to be pretty similar to that we saw in fast unwinder some time ago (fixed by Evgeniy with https://reviews.llvm.org/rL219683).
Possible explanation (that we considered when debugging segfault in unwinder) looks like this:

  1. Kernel maps stacks from higher addresses to lower (MAP_GROWSDOWN flag in mmap syscall).
  2. Kernel maps stacks non-atomically (i.e not all ulilmit amount of stack memory become addressable simultaneously, lower pages become available a little bit later than higher).
  3. If we make access to *stack_top (== stack_bot - ulimit) before kernel actually mapped all ulimit range, we'll have segfault.
Mar 21 2017, 10:50 AM · Restricted Project

Mar 20 2017

m.ostapenko added a comment to D30818: [lsan] Don't handle DTLS of thread under destruction.

Ping.

Mar 20 2017, 12:41 PM · Restricted Project

Mar 17 2017

m.ostapenko updated the diff for D29586: [lsan] Enable LSan for arm Linux.

Gentle ping. Is there any chance that LSan for ARM will be accepted?

Mar 17 2017, 11:23 AM · Restricted Project