Last Comment Bug 856671 - Crash in TiledLayerBuffer.EmptyUpdate unit test (gtest)
: Crash in TiledLayerBuffer.EmptyUpdate unit test (gtest)
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Layers (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla23
Assigned To: Vasil Dimov [:vd]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-04-01 10:07 PDT by Vasil Dimov [:vd]
Modified: 2013-04-02 11:46 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (953 bytes, patch)
2013-04-01 10:13 PDT, Vasil Dimov [:vd]
b56girard: review+
Details | Diff | Splinter Review
Fix (954 bytes, patch)
2013-04-01 12:17 PDT, Vasil Dimov [:vd]
vd: review+
Details | Diff | Splinter Review

Description Vasil Dimov [:vd] 2013-04-01 10:07:14 PDT
The unit test TiledLayerBuffer.EmptyUpdate, defined in gfx/layers/TestTiledLayerBuffer.cpp has the following two problems:

To reproduce run:
firefox -unittest
or
GTEST_FILTER=TiledLayerBuffer.EmptyUpdate firefox -unittest

Problem 1:
----------

gRectPoolTlsIndex.init() (gRectPoolTlsIndex is defined as a global variable in nsRegion.cpp) is never called and thus gRectPoolTlsIndex.inited remains false when gRectPoolTlsIndex.get() is called later which leads to an assertion failure:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff01bcb23 in mozilla::ThreadLocal<RgnRectMemoryAllocator*>::get (
    this=0x7ffff52949b0 <gRectPoolTlsIndex>)
    at ../../dist/include/mozilla/ThreadLocal.h:118
118	  MOZ_ASSERT(initialized());
(gdb) bt
#0  0x00007ffff01bcb23 in mozilla::ThreadLocal<RgnRectMemoryAllocator*>::get (
    this=0x7ffff52949b0 <gRectPoolTlsIndex>)
    at ../../dist/include/mozilla/ThreadLocal.h:118
#1  0x00007ffff01b773b in nsRegion::RgnRect::operator new ()
    at /home/vd/m/mozilla-central/gfx/src/nsRegion.cpp:215
#2  0x00007ffff01b7990 in nsRegion::SetToElements (this=0x7fffffffcff0, aCount=1)
    at /home/vd/m/mozilla-central/gfx/src/nsRegion.cpp:278
#3  0x00007ffff01b88a3 in nsRegion::Copy (this=0x7fffffffcff0, aRect=...)
    at /home/vd/m/mozilla-central/gfx/src/nsRegion.cpp:606
#4  0x00007ffff01b6aa9 in nsRegion::nsRegion (this=0x7fffffffcff0, aRect=...)
    at /home/vd/m/mozilla-central/gfx/src/nsRegion.h:73
#5  0x00007ffff01d438c in nsIntRegion::nsIntRegion (this=0x7fffffffcff0, aRect=...)
    at ../../dist/include/nsRegion.h:285
#6  0x00007ffff220346b in mozilla::layers::TiledLayerBuffer_EmptyUpdate_Test::TestBody (this=0x7ffff6c22170)
    at /home/vd/m/mozilla-central/gfx/layers/TestTiledLayerBuffer.cpp:71
...
(gdb) f 0
#0  0x00007ffff01bcb23 in mozilla::ThreadLocal<RgnRectMemoryAllocator*>::get (
    this=0x7ffff52949b0 <gRectPoolTlsIndex>)
    at ../../dist/include/mozilla/ThreadLocal.h:118
118	  MOZ_ASSERT(initialized());
(gdb) p inited
$1 = false

The solution could be to call nsRegion::InitStatic() (which calls gRectPoolTlsIndex.init()) beforehand.

Problem 2:
----------

Can only get here if Problem 1 is solved because it crashes earlier otherwise.

TestTiledLayerTile (in TestTiledLayerBuffer.cpp) is defined in such a way that any object of that type will always be equal to any other object of that type. This is because the 'value' member is initialized to 0 and never changed afterwards and the operator==() method just compares the 'value' members of the two objects being compared. So TiledLayerBuffer::IsPlaceholder() will always return true because it compares two such objects. TiledLayerBuffer::Update() contains an assert that IsPlaceholder() does not return true. That assert fails:

###!!! ABORT: index out of range: '!IsPlaceholder(newTile)', file /home/vd/m/mozilla-central/gfx/layers/TiledLayerBuffer.h, line 420

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff5426551 in mozalloc_abort (
    msg=0x7fffffffc970 "###!!! ABORT: index out of range: '!IsPlaceholder(newTile)', file /home/vd/m/mozilla-central/gfx/layers/TiledLayerBuffer.h, line 420")
    at /home/vd/m/mozilla-central/memory/mozalloc/mozalloc_abort.cpp:30
30	    MOZ_CRASH();
(gdb) bt
#0  0x00007ffff5426551 in mozalloc_abort (
    msg=0x7fffffffc970 "###!!! ABORT: index out of range: '!IsPlaceholder(newTile)', file /home/vd/m/mozilla-central/gfx/layers/TiledLayerBuffer.h, line 420")
    at /home/vd/m/mozilla-central/memory/mozalloc/mozalloc_abort.cpp:30
#1  0x00007ffff209f914 in Abort (
    aMsg=0x7fffffffc970 "###!!! ABORT: index out of range: '!IsPlaceholder(newTile)', file /home/vd/m/mozilla-central/gfx/layers/TiledLayerBuffer.h, line 420")
    at /home/vd/m/mozilla-central/xpcom/base/nsDebugImpl.cpp:430
#2  0x00007ffff209f81f in NS_DebugBreak (aSeverity=3, 
    aStr=0x7ffff3d991e7 "index out of range", 
    aExpr=0x7ffff3d991fa "!IsPlaceholder(newTile)", 
    aFile=0x7ffff3d98fe8 "/home/vd/m/mozilla-central/gfx/layers/TiledLayerBuffer.h", aLine=420) at /home/vd/m/mozilla-central/xpcom/base/nsDebugImpl.cpp:387
#3  0x00007ffff220413b in mozilla::layers::TiledLayerBuffer<mozilla::layers::TestTiledLayerBuffer, mozilla::layers::TestTiledLayerTile>::Update (this=0x7fffffffd030, 
    aNewValidRegion=..., aPaintRegion=...)
    at /home/vd/m/mozilla-central/gfx/layers/TiledLayerBuffer.h:420
#4  0x00007ffff220304f in mozilla::layers::TestTiledLayerBuffer::TestUpdate (
    this=0x7fffffffd030, aNewValidRegion=..., aPaintRegion=...)
    at /home/vd/m/mozilla-central/gfx/layers/TestTiledLayerBuffer.cpp:53
#5  0x00007ffff2203490 in mozilla::layers::TiledLayerBuffer_EmptyUpdate_Test::TestBody (this=0x7ffff6c22170)
    at /home/vd/m/mozilla-central/gfx/layers/TestTiledLayerBuffer.cpp:73
...
(gdb) f 3
#3  0x00007ffff220413b in mozilla::layers::TiledLayerBuffer<mozilla::layers::TestTiledLayerBuffer, mozilla::layers::TestTiledLayerTile>::Update (this=0x7fffffffd030, 
    aNewValidRegion=..., aPaintRegion=...)
    at /home/vd/m/mozilla-central/gfx/layers/TiledLayerBuffer.h:420
420	      NS_ABORT_IF_FALSE(!IsPlaceholder(newTile), "index out of range");
(gdb) p newTile.value
$1 = 0
(gdb) p AsDerived().GetPlaceholderTile().value
$2 = 0
(gdb) 

The solution could be to change the test's GetPlaceholderTile() method to return an object with a 'value' member set to something other than 0.

The tree I used is mozilla-central as of 126538:2f63e2f90d5c from Mar 28 2013.
Comment 1 Vasil Dimov [:vd] 2013-04-01 10:13:05 PDT
Created attachment 731931 [details] [diff] [review]
Fix

Fix the problems as suggested in the bug description.
Comment 2 Benoit Girard (:BenWa) 2013-04-01 10:31:40 PDT
Comment on attachment 731931 [details] [diff] [review]
Fix

Review of attachment 731931 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch.

::: gfx/layers/TestTiledLayerBuffer.cpp
@@ +29,4 @@
>  
>  public:
>    TestTiledLayerTile GetPlaceholderTile() const {
> +    return TestTiledLayerTile(1);

Lets use -1 instead
Comment 3 Vasil Dimov [:vd] 2013-04-01 12:17:20 PDT
Created attachment 731979 [details] [diff] [review]
Fix

Fix, use -1 as suggested by Benoit.
Comment 4 Milan Sreckovic [:milan] 2013-04-01 12:21:44 PDT
(In reply to Vasil Dimov [:vd] from comment #3)
> Created attachment 731979 [details] [diff] [review]
> Fix
> 
> Fix, use -1 as suggested by Benoit.

Thanks for the fix, thanks for the review! Now I don't have to keep disabling the test in the makefile :-)
Comment 6 Ryan VanderMeulen [:RyanVM] 2013-04-02 11:46:59 PDT
https://hg.mozilla.org/mozilla-central/rev/06c5bffd4437

Note You need to log in before you can comment on or make changes to this bug.