[FFOS7715 v2.1][Search] monkey test crash at libc.so + 0x22038 (VisitEdges fix)

RESOLVED FIXED in Firefox 37, Firefox OS v2.0M

Status

()

Core
Graphics
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Chaochao Huang, Assigned: jrmuizel)

Tracking

unspecified
mozilla39
ARM
Gonk (Firefox OS)
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-b2g:2.0M+, firefox36 wontfix, firefox37+ fixed, firefox38 fixed, firefox39 fixed, b2g-v2.0M fixed, b2g-v2.1S fixed, b2g-v2.2 fixed, b2g-master fixed)

Details

(Whiteboard: [sprd=398871])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
B2G_OS_Version=2.1.0.0-prerelease
Android_Device=scx15_sp7715ea
Android_Manufacturer=Spreadtrum
ProductName=B2G
Android_Board=scx15_sp7715ea
Android_CPU_ABI=armeabi-v7a
Vendor=Mozilla
InstallTime=1422000361
Notes=GL Layers! EGL? EGL+ GL Context? GL Context+ GL Layers+ 
ReleaseChannel=default
Android_CPU_ABI2=armeabi
Version=34.0
Android_Brand=Spreadtrum
ServerURL=https://crash-reports.mozilla.com/submit?id={3c2e2abc-06d4-11e1-ac3b-374f68613e61}&version=34.0&buildid=20150122152411
Android_Hardware=scx15
useragent_locale=en-US
BuildID=20150122152411
ProductID={3c2e2abc-06d4-11e1-ac3b-374f68613e61}
Android_Version=19(REL)
Android_Model=SP7715A
CrashTime=1422031410
StartupTime=1422022678
ProcessType=content
URL=app://search.gaiamobile.org/manifest.webapp
(Reporter)

Updated

3 years ago
No longer blocks: 1123554
(Reporter)

Comment 1

3 years ago
0  libc.so + 0x22038
1  libxul.so!VisitNextEdgeBetweenRect [nsRegion.cpp : 394 + 0xd]
2  libxul.so!nsRegion::VisitEdges(void (*)(void*, VisitSide, int, int, int, int), void*) [nsRegion.cpp : 452 + 0x13]
3  libxul.so!mozilla::layers::PadDrawTargetOutFromRegion(mozilla::RefPtr<mozilla::gfx::DrawTarget>, nsIntRegion&) [nsRegion.h : 710 + 0xb]
4  libxul.so!mozilla::layers::ClientTiledLayerBuffer::ValidateTile(mozilla::layers::TileClient, nsIntPoint const&, nsIntRegion const&) [TiledContentClient.cpp : 1251 + 0x3]
5  libxul.so!mozilla::layers::TiledLayerBuffer<mozilla::layers::ClientTiledLayerBuffer, mozilla::layers::TileClient>::Update(nsIntRegion const&, nsIntRegion const&) [TiledLayerBuffer.h : 501 + 0x5]
6  libxul.so!mozilla::layers::ClientTiledLayerBuffer::PaintThebes(nsIntRegion const&, nsIntRegion const&, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*) [TiledContentClient.cpp : 959 + 0x9]
7  libxul.so!mozilla::layers::ClientTiledThebesLayer::RenderHighPrecision(nsIntRegion&, nsIntRegion const&, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*) [ClientTiledThebesLayer.cpp : 246 + 0x5]
8  libxul.so!mozilla::layers::ClientTiledThebesLayer::RenderLayer() [ClientTiledThebesLayer.cpp : 426 + 0xf]
9  libxul.so!SkScalerContext::getFontMetrics(SkPaint::FontMetrics*) [SkScalerContext.cpp : 802 + 0x5]
10  libxul.so!mozilla::layers::ClientContainerLayer::RenderLayer() [ClientContainerLayer.h : 69 + 0x7]
11  libxul.so!mozilla::layers::ClientLayerManager::EndTransactionInternal(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) [ClientLayerManager.cpp : 266 + 0x7]
12  libxul.so!mozilla::layers::ClientLayerManager::EndTransaction(void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, mozilla::layers::DrawRegionClip, nsIntRegion const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) [ClientLayerManager.cpp : 296 + 0xb]
13  libxul.so!nsDisplayList::PaintForFrame(nsDisplayListBuilder*, nsRenderingContext*, nsIFrame*, unsigned int) [nsDisplayList.cpp : 1349 + 0xf]
14  libxul.so!nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) [nsDisplayList.cpp : 1194 + 0xd]
15  libxul.so!nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, unsigned int) [nsLayoutUtils.cpp : 3026 + 0x7]
16  libxul.so!PresShell::Paint(nsView*, nsRegion const&, unsigned int) [nsPresShell.cpp : 6224 + 0xd]
17  libxul.so!nsViewManager::ProcessPendingUpdatesPaint(nsIWidget*) [nsViewManager.cpp : 443 + 0x9]
18  libxul.so!nsViewManager::ProcessPendingUpdatesForView(nsView*, bool) [nsViewManager.cpp : 384 + 0x7]
19  libxul.so!nsRefreshDriver::Tick(long long, mozilla::TimeStamp) [nsRefreshDriver.cpp : 1276 + 0x5]
20  libxul.so!mozilla::RefreshDriverTimer::Tick() [nsRefreshDriver.cpp : 171 + 0xb]
21  libxul.so!nsTimerImpl::Fire() [nsTimerImpl.cpp : 618 + 0x5]
22  libxul.so!nsTimerEvent::Run() [nsTimerImpl.cpp : 711 + 0x3]
23  libxul.so!nsThread::ProcessNextEvent(bool, bool*) [nsThread.cpp : 823 + 0x5]
24  libxul.so!NS_ProcessNextEvent(nsIThread*, bool) [nsThreadUtils.cpp : 265 + 0xb]
25  libxul.so!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) [MessagePump.cpp : 99 + 0x7]
26  libxul.so!MessageLoop::RunInternal() [message_loop.cc : 234 + 0x5]
27  libxul.so!MessageLoop::Run() [message_loop.cc : 227 + 0x5]
28  libxul.so!nsBaseAppShell::Run() [nsBaseAppShell.cpp : 164 + 0x7]
29  libxul.so!XRE_RunAppShell [nsEmbedFunctions.cpp : 702 + 0x5]
30  libxul.so!MessageLoop::RunInternal() [message_loop.cc : 234 + 0x5]
31  libxul.so!MessageLoop::Run() [message_loop.cc : 227 + 0x5]
32  libxul.so!XRE_InitChildProcess [nsEmbedFunctions.cpp : 539 + 0x5]
33  plugin-container!content_process_main(int, char**) [plugin-container.cpp : 150 + 0x7]
34  libc.so!__libc_init [libc_init_dynamic.cpp : 112 + 0x7]
35  plugin-container + 0x662
36  linker!set_soinfo_pool_protection [linker.cpp : 291 + 0xb]
37  0xbedcb9ab

Comment 2

3 years ago
Hi vance:

   please help to assign someone to investigate this crash, thanks
Flags: needinfo?(vchen)
ni? Danny and Rex for the follow-up.
blocking-b2g: --- → 2.1S?
Flags: needinfo?(rhung)
Flags: needinfo?(dliang)

Comment 4

3 years ago
Hi, Chaochao,

Please run the following steps to get full SW information for us.
1. git clone https://github.com/Mozilla-TWQA/B2G-flash-tool
2. cd B2G-flash-tool/
3. Connect USB cable to your device and make sure adb works well.
4. Run ./check_version.sh and post all output on the bugzilla.
Flags: needinfo?(rhung) → needinfo?(chaochao.huang)

Comment 5

3 years ago
The information you request are as follows:

~/code/B2G-flash-tool$ ./check_versions.sh 
Gaia-Rev        2055fc40a8bd2af1908979cb45da6b7d1c4ced0b
Gecko-Rev       117b02783b5fb93e122ff8bd5791501dbc966f4f
Build-ID        20150122152411
Version         34.0
Device-Name     scx15_sp7715ea
FW-Release      4.4.2
FW-Incremental  93
FW-Date         Thu Jan 22 15:21:20 CST 2015
Flags: needinfo?(chaochao.huang)

Updated

3 years ago
See Also: → bug 1126644

Comment 6

3 years ago
Same issue is under investigation now, and will post analysis later once it is ready.
Also NI Wenyuan to catch this bug.
Flags: needinfo?(wchi)

Comment 7

3 years ago
Hi Rex,

Yes, this crash looks like bug#1126644.
I am working on it.
Flags: needinfo?(wchi)
Cancel NI due to wchi is working on this.
Flags: needinfo?(dliang)

Comment 9

3 years ago
(In reply to wchi from comment #7)
> Hi Rex,
> 
> Yes, this crash looks like bug#1126644.
> I am working on it.

Assign this case to wchi.

--
Keven
Assignee: nobody → wchi

Comment 10

3 years ago
Hi wchi:

  we sprd colleagues are not authorized to access bug #1126644.  could you please update about this bug's investigate progress, thanks a lot.
Flags: needinfo?(wchi)

Comment 11

3 years ago
Hi yong,

The root cause of bug#1125544 is identified and I am wait for module owner to confirm it.
There is a working fix although it is not reviewed yet.

Would you mind taking the fix and test if it works for this bug#1130978?

Thanks
Flags: needinfo?(wchi)

Comment 13

3 years ago
(In reply to wchi from comment #12)
> Created attachment 8572987 [details] [diff] [review]
> fix.patch

thanks wchi, I will try this patch.

Updated

3 years ago
Duplicate of this bug: 1126644

Comment 15

3 years ago
Created attachment 8574486 [details] [diff] [review]
Fix VisitEdges by Jeff
Attachment #8572987 - Attachment is obsolete: true

Comment 16

3 years ago
Hi yong,

please try the patch in comment#15. It will be the formal one.
thanks

Comment 17

3 years ago
Hi Jeff,

Would you like to take this bug and bug#1126644 ?
Thanks.
Flags: needinfo?(jmuizelaar)

Comment 18

3 years ago
(In reply to wchi from comment #16)
> Hi yong,
> 
> please try the patch in comment#15. It will be the formal one.
> thanks

thanks wchi.

Hi chaochao:

  Please help me to try this patch on our 2.1s branch, thanks.
Flags: needinfo?(chaochao.huang)
(Assignee)

Updated

3 years ago
Assignee: wchi → jmuizelaar
Flags: needinfo?(jmuizelaar)
Carrying over Jeff's comment from bug 1126644.

> Comment on attachment 8574028 [details] [diff]
> Fix VisitEdges
> 
> The code is broken because in the else case of VisitNextEdgeBetweenRect we
> assume that r2->x1 < r1->x1. This is not always the case. 
> 
> The fix is to have VisitNextEdgeBetweenRect return whether there's an
> overlap.
> The calling code can than adjust x1 appropriately if r1 != r1_end && r2 !=
> r2_end.
> 
> If you have questions about how this works, ask.
> 
> Ps. I'll remove the 'cout << '
Comment on attachment 8574486 [details] [diff] [review]
Fix VisitEdges by Jeff

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

::: gfx/src/nsRegion.cpp
@@ +394,5 @@
>      // we handle the corners by just extending the top and bottom edges
>      visit(closure, side, x1, y, r1->x2+1, y);
>      r1++;
>      x1 = r2->x1 - 1;
> +    return false;

Remove the assignment to x1 here and add a comment saying "we're not sure if the next rect to process will be r1 or r2; the caller function will decide and update x1" (or something to that effect)

::: gfx/tests/gtest/TestRegion.cpp
@@ +543,5 @@
> +    // vertically separated
> +    nsRegion r(nsRect(4, 1, 61, 49));
> +    r.Or(r, nsRect(115, 1, 99, 49));
> +    r.Or(r, nsRect(115, 49, 99, 1));
> +    r.Or(r, nsRect(12, 50, 11, 5));

This test is not easy to follow. Can you make a more simplified one (perhaps in addition to this one)? I think something like this should demonstrate the problem, assuming I understand it correctly:

nsRegion r(nsRect(0, 0, 50, 50));
r.Or(nsRect(100, 0, 50, 50));
r.Or(nsRect(200, 50, 50, 50));

(i.e.:

+-----+     +-----+           
|     |     |     |           
|     |     |     |           
+-----+     +-----+    +-----+
                       |     |
                       |     |
                       +-----+
)
Attachment #8574486 - Flags: review+
(Assignee)

Comment 21

3 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #20)
> Comment on attachment 8574486 [details] [diff] [review]
> Fix VisitEdges by Jeff
> 
> Review of attachment 8574486 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/src/nsRegion.cpp
> @@ +394,5 @@
> >      // we handle the corners by just extending the top and bottom edges
> >      visit(closure, side, x1, y, r1->x2+1, y);
> >      r1++;
> >      x1 = r2->x1 - 1;
> > +    return false;
> 
> Remove the assignment to x1 here and add a comment saying "we're not sure if
> the next rect to process will be r1 or r2; the caller function will decide
> and update x1" (or something to that effect)


We need the assignment so that x1 is properly set when the loop condition in the caller is false.
Hm, ok. I guess the comment should say something like "we assign x1 assuming r2->x1 < r1->x1. However the caller may know better and if so, will update x1 as needed".

Updated

3 years ago
blocking-b2g: 2.1S? → 2.1S+
status-b2g-v2.0M: --- → affected

Comment 23

3 years ago
Due to duplicated case, mark 2.0M as affected.

--
Keven
(Reporter)

Comment 24

3 years ago
(In reply to yong.ren from comment #18)

Yong ge,

I have applied it to 2.1s branch.
Flags: needinfo?(chaochao.huang)
https://hg.mozilla.org/mozilla-central/rev/cb71097c142a
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Uplift? I'm hitting crashes in Fennec 36.0.2 by following STR from bug 1138335 comment 0 on a Nexus 4 - just loading the demo and sitting there a bit will cause crashes. For example:

https://crash-stats.mozilla.com/report/index/07ba7b19-ab6c-4f6d-8fee-62f6e2150317
Flags: needinfo?(jmuizelaar)
Component: Gaia::Search → Graphics
Product: Firefox OS → Core
Target Milestone: --- → mozilla39
(Assignee)

Comment 27

3 years ago
Comment on attachment 8574486 [details] [diff] [review]
Fix VisitEdges by Jeff

Approval Request Comment
[Feature/regressing bug #]: 1023473
[User impact if declined]: Crashes on particular content
[Describe test coverage new/current, TreeHerder]: This code has decent unit test coverage.
[Risks and why]: This code is only used on B2G and Android and it fixes an obvious a pretty obvious crash. It should have no risk on desktop
Flags: needinfo?(jmuizelaar)
Attachment #8574486 - Flags: approval-mozilla-beta?
Attachment #8574486 - Flags: approval-mozilla-aurora?

Updated

3 years ago
status-b2g-v2.1S: --- → affected
I still see some crashes reported on 39 in the last week and one crash in the last 3 days but that could be clients that haven't yet updated. The final mobile Beta gtb is Monday. I'll check on crash stats again on Monday and make a call about taking the fix in Beta 8.
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #28)
> I still see some crashes reported on 39 in the last week and one crash in
> the last 3 days but that could be clients that haven't yet updated.

You could probably check the build ids on the crash reports to see if that's the case. That being said I applied this patch on top of the current mozilla-beta, built it [1], and ran through the STR. 37.0b6 crashes reliably with the STR but the build with this patch did not crash on the same STR. So I think the fix is good and would encourage you to take the uplift.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ce38b5e2141
Comment on attachment 8574486 [details] [diff] [review]
Fix VisitEdges by Jeff

Checking build numbers shows them all to be older builds. (Good suggestion kats!) Also, thanks to kats for verifying by applying the patch on Beta. With that verification, I think we're good to take this fix for Beta 8. Beta+ Aurora+
Attachment #8574486 - Flags: approval-mozilla-beta?
Attachment #8574486 - Flags: approval-mozilla-beta+
Attachment #8574486 - Flags: approval-mozilla-aurora?
Attachment #8574486 - Flags: approval-mozilla-aurora+
status-firefox37: --- → affected
status-firefox38: --- → affected
tracking-firefox37: --- → +
https://hg.mozilla.org/releases/mozilla-aurora/rev/7d22e62ca127
status-firefox38: affected → fixed
Flags: in-testsuite+
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/fb9ae74a783a
status-b2g-v2.2: --- → fixed
status-b2g-master: --- → fixed
Bug 1145206 suggests that this may issue may be affecting 36 as well.
status-firefox36: --- → affected
Duplicate of this bug: 1145206
status-firefox36: affected → wontfix

Comment 37

3 years ago
Hi Kai-Zhen,
Can you also help to check whether the patch is okay for 2.0M?
Thanks!
blocking-b2g: 2.1S+ → 2.0M+
Flags: needinfo?(kli)
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/dabfeaf25fc1
status-b2g-v2.0M: affected → fixed
Flags: needinfo?(kli)

Updated

3 years ago
Blocks: 1054172
Too many bugs with very similar titles.
Summary: [FFOS7715 v2.1][Search] monkey test crash at libc.so + 0x22038 → [FFOS7715 v2.1][Search] monkey test crash at libc.so + 0x22038 (VisitEdges fix)
You need to log in before you can comment on or make changes to this bug.