Oilpan: In ASan, poison all unmarked objects before start sweeping

Oilpan: In ASan, poison all unmarked objects before start sweeping

It seems we've disabled the verification at some point (by mistake).
This CL enables the verification again.

The idea is to poison all unmarked objects before start sweeping,
unpoison a specific object before calling the object's finalizer.
By doing this, ASan can detect an error if the finalizer touches
any other on-heap object that die in the same GC cycle.

BUG=420515
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194725

[haraken, 2015-04-28 02:11:17]

haraken@chromium.org changed reviewers:
+ oilpan-reviews@chromium.org, sigbjornf@opera.com

[haraken, 2015-04-28 02:11:17]

PTAL

BTW, what would happen if we poison all (not only unmarked but all) objects on
the heap before start sweeping? Then ASan will detect an error if a finalizer
touches any on-heap object. Would the verification be too strong?

[sof, 2015-04-28 06:12:24]

https://codereview.chromium.org/1110853002/diff/1/Source/platform/heap/Heap.cpp
File Source/platform/heap/Heap.cpp (right):

https://codereview.chromium.org/1110853002/diff/1/Source/platform/heap/Heap.c...
Source/platform/heap/Heap.cpp:586: for (BasePage* page = m_firstPage; page; page
= page->next()) {
Don't you clear m_firstPage just above, so what work does this loop do?

[sof, 2015-04-28 06:17:46]

If sweep happened along with marking previously & we poison on adding to the
freelist, then I suppose this step wasn't needed previously (== before lazy
sweeping.)

Which brings up a point -- why re-poison upon adding unmarked payloads to the
freelist here?

[haraken, 2015-04-28 06:18:35]

https://codereview.chromium.org/1110853002/diff/1/Source/platform/heap/Heap.cpp
File Source/platform/heap/Heap.cpp (right):

https://codereview.chromium.org/1110853002/diff/1/Source/platform/heap/Heap.c...
Source/platform/heap/Heap.cpp:586: for (BasePage* page = m_firstPage; page; page
= page->next()) {
On 2015/04/28 06:12:24, sof wrote:
> Don't you clear m_firstPage just above, so what work does this loop do?

Very good point. I now understand why I observed no asan error in layout tests
:)

Rerunning layout tests with asan locally.

[haraken, 2015-04-28 06:22:42]

On 2015/04/28 06:17:46, sof wrote:
> If sweep happened along with marking previously & we poison on adding to the
> freelist, then I suppose this step wasn't needed previously (== before lazy
> sweeping.)

I guess this step should have existed even without the lazy sweeping. Even
without lazy sweeping, the addition to the free lists happens during the
sweeping. Thus, to make sure that a finalizer of an object does not touch any
objects that die in the same GC cycle, we need to poison all dead objects before
starting the sweeping.


> Which brings up a point -- why re-poison upon adding unmarked payloads to the
> freelist here?

Because we unpoison the object to call its destructor.

[sof, 2015-04-28 06:26:18]

On 2015/04/28 06:22:42, haraken wrote:
> On 2015/04/28 06:17:46, sof wrote:
> > If sweep happened along with marking previously & we poison on adding to the
> > freelist, then I suppose this step wasn't needed previously (== before lazy
> > sweeping.)
> 
> I guess this step should have existed even without the lazy sweeping. Even
> without lazy sweeping, the addition to the free lists happens during the
> sweeping. Thus, to make sure that a finalizer of an object does not touch any
> objects that die in the same GC cycle, we need to poison all dead objects
before
> starting the sweeping.
> 

Yes, quite right. It'll be very interesting to see what this flushes out :)

[sof, 2015-04-28 09:15:25]

lgtm.

https://codereview.chromium.org/1110853002/diff/20001/Source/platform/heap/He...
File Source/platform/heap/Heap.cpp (right):

https://codereview.chromium.org/1110853002/diff/20001/Source/platform/heap/He...
Source/platform/heap/Heap.cpp:1516: // objects are poisoned, ASan will detech an
error if the finalizer
nit: s/detech/detect/

[haraken, 2015-04-28 10:51:35]

>
https://codereview.chromium.org/1110853002/diff/20001/Source/platform/heap/He...
> File Source/platform/heap/Heap.cpp (right):
> 
>
https://codereview.chromium.org/1110853002/diff/20001/Source/platform/heap/He...
> Source/platform/heap/Heap.cpp:1516: // objects are poisoned, ASan will detech
an
> error if the finalizer
> nit: s/detech/detect/

Done. I'll land this after checking that we don't hit the poison in asan builds.

[haraken, 2015-04-29 16:23:31]

On 2015/04/28 10:51:35, haraken wrote:
> >
>
https://codereview.chromium.org/1110853002/diff/20001/Source/platform/heap/He...
> > File Source/platform/heap/Heap.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1110853002/diff/20001/Source/platform/heap/He...
> > Source/platform/heap/Heap.cpp:1516: // objects are poisoned, ASan will
detech
> an
> > error if the finalizer
> > nit: s/detech/detect/
> 
> Done. I'll land this after checking that we don't hit the poison in asan
builds.

This CL detects a lot of crashes in asan builds, which I'll take a close look
tomorrow.

[haraken, 2015-04-30 06:21:04]

Updated the CL. I moved the poisonUnmarkedObjects() from Heap::prepareForSweep()
to ThreadState::preSweep(). Because thread-local weak processing and
pre-finalizers are allowed to touch on-heap objects, the poisoning must be done
after the weak processing and pre-finalizers but before starting the actual
sweeping.

However, I still hit a bunch of crashes with the CL. These look like real errors
which we need to fix...

Here are a couple of examples:

crash log for renderer (pid 23594):
STDOUT: #CRASHED - renderer (pid 23594)
STDERR: =================================================================
STDERR: ==1==ERROR: AddressSanitizer: use-after-poison on address 0x15aa36aeb150
at pc 0x000008c13778 bp 0x7ffdf8644450 sp 0x7ffdf8644448
STDERR: READ of size 8 at 0x15aa36aeb150 thread T0 (content_shell)
STDERR:     #0 0x8c13777 in timerHeap
third_party/WebKit/Source/platform/Timer.h:95:92
STDERR:     #1 0x8c13777 in operator=
third_party/WebKit/Source/platform/Timer.cpp:83:0
STDERR:     #2 0x8c13777 in swap
third_party/WebKit/Source/platform/Timer.cpp:101:0
STDERR:     #3 0x8c13777 in __pop_heap<blink::TimerHeapLessThanFunction &,
blink::TimerHeapIterator>
buildtools/third_party/libc++/trunk/include/algorithm:4912:0
STDERR:     #4 0x8c13777 in pop_heap<blink::TimerHeapIterator,
blink::TimerHeapLessThanFunction>
buildtools/third_party/libc++/trunk/include/algorithm:4928:0
STDERR:     #5 0x8c13777 in heapPopMin
third_party/WebKit/Source/platform/Timer.cpp:309:0
STDERR:     #6 0x8c13777 in heapPop
third_party/WebKit/Source/platform/Timer.cpp:299:0
STDERR:     #7 0x8c13777 in blink::TimerBase::heapDelete()
third_party/WebKit/Source/platform/Timer.cpp:265:0
STDERR:     #8 0x8c13064 in blink::TimerBase::updateHeapIfNeeded(double)
third_party/WebKit/Source/platform/Timer.cpp:357:9
STDERR:     #9 0x8c122ce in blink::TimerBase::setNextFireTime(double)
third_party/WebKit/Source/platform/Timer.cpp:387:9
STDERR:     #10 0x39be3fa in blink::Document::~Document()
third_party/WebKit/Source/core/dom/Document.cpp:603:1
STDERR:     #11 0x1d98c6e in blink::HeapObjectHeader::finalize(unsigned char*,
unsigned long) third_party/WebKit/Source/platform/heap/Heap.cpp:421:9
STDERR:     #12 0x1d9e4f8 in blink::NormalPage::sweep()
third_party/WebKit/Source/platform/heap/Heap.cpp:1524:13
STDERR:     #13 0x1d99e1d in sweepUnsweptPage
third_party/WebKit/Source/platform/heap/Heap.cpp:634:9
STDERR:     #14 0x1d99e1d in blink::BaseHeap::completeSweep()
third_party/WebKit/Source/platform/heap/Heap.cpp:673:0
STDERR:     #15 0x1da6fa3 in blink::ThreadState::completeSweep()
third_party/WebKit/Source/platform/heap/ThreadState.cpp:945:13
STDERR:     #16 0x1dac4e2 in blink::ThreadState::preSweep()
third_party/WebKit/Source/platform/heap/ThreadState.cpp:906:9
STDERR:     #17 0x1da0d22 in ~SafePointScope
third_party/WebKit/Source/platform/heap/SafePoint.h:26:13
STDERR:     #18 0x1da0d22 in ~GCScope
third_party/WebKit/Source/platform/heap/Heap.cpp:400:0
STDERR:     #19 0x1da0d22 in
blink::Heap::collectGarbage(blink::ThreadState::StackState,
blink::ThreadState::GCType, blink::Heap::GCReason)
third_party/WebKit/Source/platform/heap/Heap.cpp:2353:0


crash log for renderer (pid 23801):
STDOUT: #CRASHED - renderer (pid 23801)
STDERR: =================================================================
STDERR: ==1==ERROR: AddressSanitizer: use-after-poison on address 0x7f5e9d8d9c40
at pc 0x000002dcb141 bp 0x7ffd001c68c0 sp 0x7ffd001c68b8
STDERR: WRITE of size 8 at 0x7f5e9d8d9c40 thread T0 (content_shell)
STDERR:     #0 0x2dcb140 in
blink::PersistentBase<blink::ThreadLocalPersistents<(blink::ThreadAffinity)0>,
blink::Persistent<blink::FontSelector,
blink::ThreadLocalPersistents<(blink::ThreadAffinity)0> > >::~PersistentBase()
third_party/WebKit/Source/platform/heap/Handle.h:135:24
STDERR:     #1 0x2dcace4 in ~Persistent
third_party/WebKit/Source/platform/heap/Handle.h:294:5
STDERR:     #2 0x2dcace4 in blink::FontFallbackList::~FontFallbackList()
third_party/WebKit/Source/platform/fonts/FontFallbackList.h:65:0
STDERR:     #3 0x53e80c7 in deref
third_party/WebKit/Source/wtf/RefCounted.h:172:13
STDERR:     #4 0x53e80c7 in derefIfNotNull<blink::FontFallbackList>
third_party/WebKit/Source/wtf/PassRefPtr.h:57:0
STDERR:     #5 0x53e80c7 in ~RefPtr third_party/WebKit/Source/wtf/RefPtr.h:56:0
STDERR:     #6 0x53e80c7 in ~Font
third_party/WebKit/Source/platform/fonts/Font.h:176:0
STDERR:     #7 0x53e80c7 in blink::StyleInheritedData::~StyleInheritedData()
third_party/WebKit/Source/core/style/StyleInheritedData.cpp:41:0
STDERR:     #8 0x2d2e24d in deref
third_party/WebKit/Source/wtf/RefCounted.h:172:13
STDERR:     #9 0x2d2e24d in derefIfNotNull<blink::StyleInheritedData>
third_party/WebKit/Source/wtf/PassRefPtr.h:57:0
STDERR:     #10 0x2d2e24d in ~RefPtr third_party/WebKit/Source/wtf/RefPtr.h:56:0
STDERR:     #11 0x2d2e24d in ~DataRef
third_party/WebKit/Source/core/style/DataRef.h:31:0
STDERR:     #12 0x2d2e24d in blink::ComputedStyle::~ComputedStyle()
third_party/WebKit/Source/core/style/ComputedStyle.h:122:0
STDERR:     #13 0x4ddd89f in deref
third_party/WebKit/Source/wtf/RefCounted.h:172:13
STDERR:     #14 0x4ddd89f in derefIfNotNull<blink::ComputedStyle>
third_party/WebKit/Source/wtf/PassRefPtr.h:57:0
STDERR:     #15 0x4ddd89f in ~RefPtr third_party/WebKit/Source/wtf/RefPtr.h:56:0
STDERR:     #16 0x4ddd89f in
blink::CachedMatchedProperties::~CachedMatchedProperties()
third_party/WebKit/Source/core/css/resolver/MatchedPropertiesCache.h:39:0
STDERR:     #17 0x1d98c6e in blink::HeapObjectHeader::finalize(unsigned char*,
unsigned long) third_party/WebKit/Source/platform/heap/Heap.cpp:421:9
STDERR:     #18 0x1d9e4f8 in blink::NormalPage::sweep()
third_party/WebKit/Source/platform/heap/Heap.cpp:1524:13
STDERR:     #19 0x1d99e1d in sweepUnsweptPage
third_party/WebKit/Source/platform/heap/Heap.cpp:634:9
STDERR:     #20 0x1d99e1d in blink::BaseHeap::completeSweep()
third_party/WebKit/Source/platform/heap/Heap.cpp:673:0
STDERR:     #21 0x1da6e44 in blink::ThreadState::completeSweep()
third_party/WebKit/Source/platform/heap/ThreadState.cpp:945:13


crash log for renderer (pid 24687):
STDOUT: #CRASHED - renderer (pid 24687)
STDERR: =================================================================
STDERR: ==1==ERROR: AddressSanitizer: use-after-poison on address 0x21c66d9b2cb8
at pc 0x000003bb4033 bp 0x7ffea2738b40 sp 0x7ffea2738b38
STDERR: WRITE of size 8 at 0x21c66d9b2cb8 thread T0 (content_shell)
STDERR:     #0 0x3bb4032 in unlink
third_party/WebKit/Source/wtf/LinkedHashSet.h:63:24
STDERR:     #1 0x3bb4032 in ~LinkedHashSetNodeBase
third_party/WebKit/Source/wtf/LinkedHashSet.h:69:0
STDERR:     #2 0x3bb4032 in ~LinkedHashSet
third_party/WebKit/Source/wtf/LinkedHashSet.h:509:0
STDERR:     #3 0x3bb4032 in finalize
third_party/WebKit/Source/wtf/LinkedHashSet.h:184:0
STDERR:     #4 0x3bb4032 in finalizeGarbageCollectedObject
third_party/WebKit/Source/wtf/LinkedHashSet.h:185:0
STDERR:     #5 0x3bb4032 in finalize
third_party/WebKit/Source/platform/heap/GCInfo.h:32:0
STDERR:     #6 0x3bb4032 in
blink::FinalizerTrait<WTF::LinkedHashSet<blink::WeakMember<blink::Element>,
WTF::PtrHash<blink::WeakMember<blink::Element> >,
WTF::HashTraits<blink::WeakMember<blink::Element> >, blink::HeapAllocator>
>::finalize(void*) third_party/WebKit/Source/platform/heap/GCInfo.h:61:0
STDERR:     #7 0x1d98c6e in blink::HeapObjectHeader::finalize(unsigned char*,
unsigned long) third_party/WebKit/Source/platform/heap/Heap.cpp:421:9
STDERR:     #8 0x1d9e4f8 in blink::NormalPage::sweep()
third_party/WebKit/Source/platform/heap/Heap.cpp:1524:13
STDERR:     #9 0x1d99e1d in sweepUnsweptPage
third_party/WebKit/Source/platform/heap/Heap.cpp:634:9
STDERR:     #10 0x1d99e1d in blink::BaseHeap::completeSweep()
third_party/WebKit/Source/platform/heap/Heap.cpp:673:0
STDERR:     #11 0x1da6e44 in blink::ThreadState::completeSweep()
third_party/WebKit/Source/platform/heap/ThreadState.cpp:945:13
STDERR:     #12 0x1dac4e2 in blink::ThreadState::preSweep()
third_party/WebKit/Source/platform/heap/ThreadState.cpp:906:9
STDERR:     #13 0x1da0d22 in ~SafePointScope
third_party/WebKit/Source/platform/heap/SafePoint.h:26:13
STDERR:     #14 0x1da0d22 in ~GCScope
third_party/WebKit/Source/platform/heap/Heap.cpp:400:0
STDERR:     #15 0x1da0d22 in
blink::Heap::collectGarbage(blink::ThreadState::StackState,
blink::ThreadState::GCType, blink::Heap::GCReason)
third_party/WebKit/Source/platform/heap/Heap.cpp:2353:0

[sof1, 2015-04-30 06:40:15]

Den 4/30/2015 08:21, haraken@chromium.org skreiv:
> Updated the CL. I moved the poisonUnmarkedObjects() from
> Heap::prepareForSweep()
> to ThreadState::preSweep(). Because thread-local weak processing and
> pre-finalizers are allowed to touch on-heap objects, the poisoning must
> be done
> after the weak processing and pre-finalizers but before starting the actual
> sweeping.
>
> However, I still hit a bunch of crashes with the CL. These look like
> real errors
> which we need to fix...
>

Great, steady repros of any genuine bug that what would otherwise be 
hard to reproduce when reported.

> Here are a couple of examples:
>
> ...
>
>
> crash log for renderer (pid 23801):
> STDOUT: #CRASHED - renderer (pid 23801)
> STDERR: =================================================================
> STDERR: ==1==ERROR: AddressSanitizer: use-after-poison on address
> 0x7f5e9d8d9c40
> at pc 0x000002dcb141 bp 0x7ffd001c68c0 sp 0x7ffd001c68b8
> STDERR: WRITE of size 8 at 0x7f5e9d8d9c40 thread T0 (content_shell)
> STDERR:     #0 0x2dcb140 in
>
blink::PersistentBase<blink::ThreadLocalPersistents<(blink::ThreadAffinity)0>,
>
> blink::Persistent<blink::FontSelector,
> blink::ThreadLocalPersistents<(blink::ThreadAffinity)0> >
>> ::~PersistentBase()
> third_party/WebKit/Source/platform/heap/Handle.h:135:24
> STDERR:     #1 0x2dcace4 in ~Persistent
> third_party/WebKit/Source/platform/heap/Handle.h:294:5
> STDERR:     #2 0x2dcace4 in blink::FontFallbackList::~FontFallbackList()
> third_party/WebKit/Source/platform/fonts/FontFallbackList.h:65:0

The (previously linked) Persistent is heap allocated?

--sigbjorn

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[haraken, 2015-04-30 07:22:15]

On Thu, Apr 30, 2015 at 3:40 PM, Sigbjorn Finne <sof@opera.com> wrote:

> Den 4/30/2015 08:21, haraken@chromium.org skreiv:
>
>> Updated the CL. I moved the poisonUnmarkedObjects() from
>> Heap::prepareForSweep()
>> to ThreadState::preSweep(). Because thread-local weak processing and
>> pre-finalizers are allowed to touch on-heap objects, the poisoning must
>> be done
>> after the weak processing and pre-finalizers but before starting the
>> actual
>> sweeping.
>>
>> However, I still hit a bunch of crashes with the CL. These look like
>> real errors
>> which we need to fix...
>>
>>
> Great, steady repros of any genuine bug that what would otherwise be hard
> to reproduce when reported.
>

Yes, the good news is this is a great verification system. The bad news is
there are a lot of errors detected... The stack traces I posted are just a
part of the observed crashes.

I'm asking yutak@ to take care of this, but any help is welcomed of course
:)



crash log for renderer (pid 23801):
>> STDOUT: #CRASHED - renderer (pid 23801)
>> STDERR: =================================================================
>> STDERR: ==1==ERROR: AddressSanitizer: use-after-poison on address
>> 0x7f5e9d8d9c40
>> at pc 0x000002dcb141 bp 0x7ffd001c68c0 sp 0x7ffd001c68b8
>> STDERR: WRITE of size 8 at 0x7f5e9d8d9c40 thread T0 (content_shell)
>> STDERR:     #0 0x2dcb140 in
>>
>>
blink::PersistentBase<blink::ThreadLocalPersistents<(blink::ThreadAffinity)0>,
>>
>> blink::Persistent<blink::FontSelector,
>> blink::ThreadLocalPersistents<(blink::ThreadAffinity)0> >
>>
>>> ::~PersistentBase()
>>>
>> third_party/WebKit/Source/platform/heap/Handle.h:135:24
>> STDERR:     #1 0x2dcace4 in ~Persistent
>> third_party/WebKit/Source/platform/heap/Handle.h:294:5
>> STDERR:     #2 0x2dcace4 in blink::FontFallbackList::~FontFallbackList()
>> third_party/WebKit/Source/platform/fonts/FontFallbackList.h:65:0
>>
>
> The (previously linked) Persistent is heap allocated?
>

Hmm, I guess PersistentNode itself is allocated off-heap but I might be
wrong.


-- 
Kentaro Hara, Tokyo, Japan

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[haraken, 2015-04-30 08:05:54]

I added #if 0 to the poisoning code. For our convenience to fix the bugs, shall
we commit PS5?

[sof, 2015-04-30 08:10:46]

On 2015/04/30 08:05:54, haraken wrote:
> I added #if 0 to the poisoning code. For our convenience to fix the bugs,
shall
> we commit PS5?

wfm

[haraken, 2015-04-30 08:16:19]

The CQ bit was checked by haraken@chromium.org

[haraken, 2015-04-30 08:16:20]

The patchset sent to the CQ was uploaded after l-g-t-m from sigbjornf@opera.com
Link to the patchset: https://codereview.chromium.org/1110853002/#ps80001
(title: " ")

[commit-bot: I haz the power, 2015-04-30 08:17:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1110853002/80001

[commit-bot: I haz the power, 2015-04-30 10:43:41]

Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194725

[sof1, 2015-04-30 14:17:41]

Den 4/30/2015 08:21, haraken@chromium.org skreiv:
>
...
>
> Here are a couple of examples:
>
> crash log for renderer (pid 23594):
> STDOUT: #CRASHED - renderer (pid 23594)
> STDERR: =================================================================
> STDERR: ==1==ERROR: AddressSanitizer: use-after-poison on address
> 0x15aa36aeb150
> at pc 0x000008c13778 bp 0x7ffdf8644450 sp 0x7ffdf8644448
> STDERR: READ of size 8 at 0x15aa36aeb150 thread T0 (content_shell)
> STDERR:     #0 0x8c13777 in timerHeap
> third_party/WebKit/Source/platform/Timer.h:95:92
> STDERR:     #1 0x8c13777 in operator=
> third_party/WebKit/Source/platform/Timer.cpp:83:0
> STDERR:     #2 0x8c13777 in swap
> third_party/WebKit/Source/platform/Timer.cpp:101:0
> STDERR:     #3 0x8c13777 in __pop_heap<blink::TimerHeapLessThanFunction &,
> blink::TimerHeapIterator>
> buildtools/third_party/libc++/trunk/include/algorithm:4912:0

Hmm, this might not work out.

While removing a stopping timer when finalizing a heap object, the timer 
implementation will iterate over the timer heap, touching various as-yet 
finalized(&poisoned) TimerBase*s -- which might be referring to part 
objects embedded in other heap objects.

--sigbjorn

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[haraken, 2015-04-30 14:22:41]

>
> While removing a stopping timer when finalizing a heap object, the timer
> implementation will iterate over the timer heap, touching various as-yet
> finalized(&poisoned) TimerBase*s -- which might be referring to part
> objects embedded in other heap objects.


Just to confirm, but this is a safe thing, right? Then can we add the
NO_SANITIZE_ADDERSS macro to the function with a good comment?


On Thu, Apr 30, 2015 at 11:17 PM, Sigbjorn Finne <sof@opera.com> wrote:

> Den 4/30/2015 08:21, haraken@chromium.org skreiv:
>
>>
>>  ...
>
>>
>> Here are a couple of examples:
>>
>> crash log for renderer (pid 23594):
>> STDOUT: #CRASHED - renderer (pid 23594)
>> STDERR: =================================================================
>> STDERR: ==1==ERROR: AddressSanitizer: use-after-poison on address
>> 0x15aa36aeb150
>> at pc 0x000008c13778 bp 0x7ffdf8644450 sp 0x7ffdf8644448
>> STDERR: READ of size 8 at 0x15aa36aeb150 thread T0 (content_shell)
>> STDERR:     #0 0x8c13777 in timerHeap
>> third_party/WebKit/Source/platform/Timer.h:95:92
>> STDERR:     #1 0x8c13777 in operator=
>> third_party/WebKit/Source/platform/Timer.cpp:83:0
>> STDERR:     #2 0x8c13777 in swap
>> third_party/WebKit/Source/platform/Timer.cpp:101:0
>> STDERR:     #3 0x8c13777 in __pop_heap<blink::TimerHeapLessThanFunction &,
>> blink::TimerHeapIterator>
>> buildtools/third_party/libc++/trunk/include/algorithm:4912:0
>>
>
> Hmm, this might not work out.
>
> While removing a stopping timer when finalizing a heap object, the timer
> implementation will iterate over the timer heap, touching various as-yet
> finalized(&poisoned) TimerBase*s -- which might be referring to part
> objects embedded in other heap objects.
>
> --sigbjorn
>



-- 
Kentaro Hara, Tokyo, Japan

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[sof1, 2015-04-30 14:28:07]

Den 4/30/2015 16:22, Kentaro Hara skreiv:
>     While removing a stopping timer when finalizing a heap object, the
>     timer implementation will iterate over the timer heap, touching
>     various as-yet finalized(&poisoned) TimerBase*s -- which might be
>     referring to part objects embedded in other heap objects.
>
>
> Just to confirm, but this is a safe thing, right? Then can we add the
> NO_SANITIZE_ADDERSS macro to the function with a good comment?
>

Not all timers are on the Oilpan heap?

--sigbjorn

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[haraken, 2015-04-30 14:32:07]

>
> Not all timers are on the Oilpan heap?


Hmm, I'm a bit confused. For timers that are not on the Oilpan heap, that
is no problem (i.e., it is just safe to touch them during destructing some
timer on the heap), isn't it?

(FYI, the Blink scheduler team is going to remove the timer heap in the
near future, although I'm not familiar with their plan.)


On Thu, Apr 30, 2015 at 11:27 PM, Sigbjorn Finne <sof@opera.com> wrote:

> Den 4/30/2015 16:22, Kentaro Hara skreiv:
>
>>     While removing a stopping timer when finalizing a heap object, the
>>     timer implementation will iterate over the timer heap, touching
>>     various as-yet finalized(&poisoned) TimerBase*s -- which might be
>>     referring to part objects embedded in other heap objects.
>>
>>
>> Just to confirm, but this is a safe thing, right? Then can we add the
>> NO_SANITIZE_ADDERSS macro to the function with a good comment?
>>
>>
> Not all timers are on the Oilpan heap?
>
> --sigbjorn
>



-- 
Kentaro Hara, Tokyo, Japan

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[sof1, 2015-04-30 14:33:33]

Den 4/30/2015 16:31, Kentaro Hara skreiv:
>     Not all timers are on the Oilpan heap?
>
>
> Hmm, I'm a bit confused. For timers that are not on the Oilpan heap,
> that is no problem (i.e., it is just safe to touch them during
> destructing some timer on the heap), isn't it?
>

You'd be turning off ASan checks involving those non-heap ones, though?

--sigbjorn

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[haraken, 2015-04-30 15:48:51]

Thanks, I understand the problem. Let me think about it a bit more.


On Thu, Apr 30, 2015 at 11:33 PM, Sigbjorn Finne <sof@opera.com> wrote:

> Den 4/30/2015 16:31, Kentaro Hara skreiv:
>
>>     Not all timers are on the Oilpan heap?
>>
>>
>> Hmm, I'm a bit confused. For timers that are not on the Oilpan heap,
>> that is no problem (i.e., it is just safe to touch them during
>> destructing some timer on the heap), isn't it?
>>
>>
> You'd be turning off ASan checks involving those non-heap ones, though?
>
> --sigbjorn
>



-- 
Kentaro Hara, Tokyo, Japan

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.