1568533 - CaptureCommandList.h Append() template doesn't use aligned pointers on SPARC

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[Petr Sumbera]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

Firefox on Solaris SPARC fails with following stack:

fffffe2a28855f71 libxul.so`mozilla::gfx::DrawTargetCaptureImpl::SetPermitSubpixelAA+0x184(7fe0de14b8980, 1, 7fe0e028d0900, 7fe0de08cea28, 7fe0e0282b4a8, 7fe0de08dd064)
fffffe2a28856041 libxul.so`mozilla::gfx::DrawTargetCaptureImpl::DrawTargetCaptureImpl+0xec(7fe0de14b8980, 7fe0de14b5060, 1900000, 0, 7fe0df798c944, 7fe0de14b4fc0)
fffffe2a288560f1 libxul.so`mozilla::gfx::Factory::CreateCaptureDrawTargetForTarget+0x18(7fe0de14b8980, 1900000, ffffffffffffffff, 0, 1, 7fe0de14b5060)
fffffe2a288561a1 libxul.so`mozilla::layers::RotatedBuffer::BeginCapture+0x6c(7fe0de08d7380, 7, ffffffffffffffff, 2, 3, 7fe0de14b5060)
fffffe2a28856251 libxul.so`mozilla::layers::ContentClient::BeginPaint+0x944(fffffe2a28856f20, 7fe0de638e860, 7fe0de61f1c00, c, 7, fffffe2a28856f38)
fffffe2a28856541 libxul.so`mozilla::layers::ContentClientDoubleBuffered::BeginPaint+0x94(fffffe2a28856f20, 7fe0de638e860, 7fe0de61f1c00, c, 1, c)
fffffe2a28856601 libxul.so`mozilla::layers::ClientPaintedLayer::RenderLayerWithReadback+0x130(7fe0de61f1c00, fffffe2a28857020, fffffe2a28856f20, 1, 7fe0de61f1c00, fffffe2a28856f20)
fffffe2a28856771 libxul.so`non-virtual thunk to mozilla::layers::ClientContainerLayer::RenderLayer+0x1f0(7fe0de61f1b30, 0, 4b53000, fffffe2a28857020, 7fe0de61f1c00, 2)
fffffe2a28856841 libxul.so`mozilla::layers::ClientLayerManager::EndTransactionInternal+0x1b8(7fe0de23b6800, 7fe0df9a2b364, 7fe0de1434000, 0, 0, 7fe0de61f1b30)
fffffe2a28856af1 libxul.so`mozilla::layers::ClientLayerManager::EndTransaction+0x40(7fe0de23b6800, 7fe0df9a2b364, 7fe0de1434000, 0, 1, 7fe0dfc013de8)
fffffe2a28856bb1 libxul.so`nsDisplayList::PaintRoot+0x7a0(7fe0de1436698, 7fe0de1434000, 0, 7fe0de08ce860, 7fe0de3564000, 7fe0de23b6800)
fffffe2a28856d11 libxul.so`nsLayoutUtils::PaintFrame+0x23b0(6b87995daa5533, 7fe0de44ae020, 0, 0, 7fe0de1434000, d)
fffffe2a28859761 libxul.so`mozilla::PresShell::Paint+0xbfc(7fe0de4498000, 0, fffffe2a2885a190, 1, 184, 7fe0de23b6800)
fffffe2a288598e1 libxul.so`nsViewManager::ProcessPendingUpdatesPaint+0x234(7fe0de3519d80, 7fe0de31a6000, 7fe0dfc013de8, 7fe0de4498000, 6b87995d98cb13, 7fe0dfc11da80)
fffffe2a288599b1 libxul.so`nsViewManager::ProcessPendingUpdatesForView+0x36c(7fe0de3519d80, 7fe0dfc11da80, 1, 7fe0de31a6000, 7fe0de3519d80, 0)
fffffe2a28859a81 libxul.so`nsViewManager::ProcessPendingUpdates+0x94(7fe0de3519d80, 0, 0, 7fe0de3519d80, 4, 7fe0de3519d80)
fffffe2a28859b31 libxul.so`nsRefreshDriver::Tick+0x13e4(7fe0de3542800, 0, 7fe0de4498000, 7fe0dfc149b80, fffffe2a2885a448, 1)
fffffe2a28859e31 libxul.so`mozilla::RefreshDriverTimer::Tick+0x2f4(7fe0de4414070, 130, 6b87995c1e19cf, 8, 1, 1)
fffffe2a28859ef1 libxul.so`mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver+0x1c0(7fe0de440f500, 130, 6b87995c1e19cf, 78, 7fe0de4414070, 6b87995c1e19cf)
fffffe2a28859fb1 libxul.so`mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync+0xc0(7fe0de440f500, fffffe2a2885aa08, 0, 80000000, 3fe00000, 2)
fffffe2a2885a0a1 libxul.so`mozilla::layout::VsyncChild::RecvNotify+0x30(7fe0de4414020, fffffe2a2885aa08, a10001, fffffe2a2885aa08, 0, 1)
fffffe2a2885a151 libxul.so`mozilla::layout::PVsyncChild::OnMessageReceived+0xfc(7fe0de4414020, 7fe0de0820fc0, 7fe0e028d0900, 8, 7fe0de0820fc8, fffffe2a2885aa18)
fffffe2a2885a241 libxul.so`mozilla::ipc::PBackgroundChild::OnMessageReceived+0x68(7fe0de31b3800, 7fe0de0820fc0, 7fe0e028d0900, 8, 7fe0e028d2a40, 6)
fffffe2a2885a6e1 libxul.so`mozilla::ipc::MessageChannel::DispatchAsyncMessage+0x6c(7fe0de31b00c8, 7fe0de0820fc0, 7fe0e028d0900, 0, 0, 1)
fffffe2a2885a791 libxul.so`mozilla::ipc::MessageChannel::DispatchMessage+0x3dc(7fe0de31b00c8, 7fe0de0820fc0, 2, ffffffff, 7fe0de48ebf00, 7fe0de350e4d0)
fffffe2a2885a8a1 libxul.so`mozilla::ipc::MessageChannel::RunMessage+0x10c(7fe0de31b00c8, 7fe0de0820f80, fffffe2a2885b200, 0, 6b87995cb2c76e, 7fe0de0820fc0)
fffffe2a2885a971 libxul.so`mozilla::ipc::MessageChannel::MessageTask::Run+0x60(7fe0de0820f80, 7fe0de0820f80, fffffe2a2885b2e8, 7fe0dfc12b250, 7fe0de0820f80, 7fe0de48ebf00)
fffffe2a2885aa21 libxul.so`nsThread::ProcessNextEvent+0x364(7fe0dfc1ca980, 0, fffffe2a2885b3e7, 7fe0dfc19e000, 7fe0de6337048, 0)
fffffe2a2885ab31 libxul.so`NS_ProcessNextEvent+0x40(7fe0dfc1ca980, 0, 1, 7fe0e028d2a68, 28, 0)
fffffe2a2885abf1 libxul.so`mozilla::ipc::MessagePump::Run+0xe8(7fe0e02154c00, fffffe2a2885bba8, 0, 7fe0e02154c20, 7fe0dfc1ca980, 1)
fffffe2a2885aca1 libxul.so`mozilla::ipc::MessagePumpForChildProcess::Run+0x60(7fe0e02154c00, fffffe2a2885bba8, 7fe0df6724e14, 7fe0e028d2a70, 1, fffffe2a2885bba8)
fffffe2a2885ad51 libxul.so`MessageLoop::RunInternal+0x10(fffffe2a2885bba8, fffffe2a2885bba8, fffffe2a2885b770, 0, 0, 7fe0dfc014e10)
fffffe2a2885ae01 libxul.so`MessageLoop::Run+0x38(fffffe2a2885bba8, 7fe0df6724e14, fffffe2a2885b770, 70742e7573655f75, 20, fffffe2a2885b6b0)
fffffe2a2885aed1 libxul.so`nsBaseAppShell::Run+0x24(7fe0de6337040, 7fe0df6970ab0, 7fe0df6724e14, 7a5, 7fe0e035b0000, 7fe0dfc1ca980)
fffffe2a2885af81 libxul.so`XRE_RunAppShell+0x60(80004000, 7fe0e021606e0, 7fe0df6af1cb8, 7fe0de632d780, 22, 0)
fffffe2a2885b041 libxul.so`mozilla::ipc::MessagePumpForChildProcess::Run+0x70(7fe0e02154c00, fffffe2a2885bba8, 0, fffffe2a2885b990, fffffe2a2885b970, 0)
fffffe2a2885b0f1 libxul.so`MessageLoop::RunInternal+0x10(fffffe2a2885bba8, fffffe2a2885bba8, 1, 0, 7fe0df66868e4, 7fe0dfc013de8)
fffffe2a2885b1a1 libxul.so`MessageLoop::Run+0x38(fffffe2a2885bba8, fffffe2a2885b9d8, fffffe2a2885b9c8, 0, 1, fffffe2a2885ba50)
fffffe2a2885b271 libxul.so`XRE_InitChildProcess+0x51c(12, fffffe2a2885c068, fffffe2a2885bec7, 7fe0e02100000, 625, 7fe0e02117800)
fffffe2a2885b561 libxul.so`mozilla::BootstrapImpl::XRE_InitChildProcess+0xc(7fe0e021466a0, 12, fffffe2a2885c068, fffffe2a2885bec7, 7fe0e02114080, 0)
fffffe2a2885b611 content_process_main+0x68(7fe0e021466a0, 12, fffffe2a2885c068, 0, 7fe0e03544e58, 0)
fffffe2a2885b6d1 main+0x100(13, fffffe2a2885c068, fffffe2a2885c108, 100800, 0, 6b8798833e36f3)
fffffe2a2885b781 _start+0x64(0, 0, 0, 0, 0, 0)

It seems that it fails when 'current' pointer is not aligned to 8 bytes in Append() template here:
https://searchfox.org/mozilla-central/source/gfx/2d/CaptureCommandList.h#54

Comment 1

[Petr Sumbera]

Ryan, I don't follow the code. Is it possible to avoid usage of not aligned pointers? Thank you!

Comment 2

[Ryan Hunt [:rhunt] (on leave until early May)]

Yeah it should be possible. This part of the code was from before my time, see [1]. I'll see if I can find time to add alignment.

[1] https://searchfox.org/mozilla-central/rev/40e889be8ff926e32f7567957f4c316f14f6fbef/gfx/2d/DrawTargetCapture.h#155

Comment 3

[Ryan Hunt [:rhunt] (on leave until early May)]

Hmm. Adding alignment here might be a little tricky. We should consider using a proper allocator here that supports alignment. Bug 1471765 had patches that switched this class to use ArenaAllocator (which supports alignment). There was some concerns about size and cache locality, but they might not be real.

Comment 4

[Petr Sumbera]

I was able to fix the issue with following patch. Can you please look at it?

--- a/gfx/2d/CaptureCommandList.h
+++ b/gfx/2d/CaptureCommandList.h
@@ -40,7 +40,7 @@
     static_assert(sizeof(T) + sizeof(uint16_t) + sizeof(uint16_t) <=
                       std::numeric_limits<uint16_t>::max(),
                   "encoding is too small to contain advance");
-    const uint16_t kAdvance = sizeof(T) + sizeof(uint16_t) + sizeof(uint16_t);
+    const uint16_t kAdvance = sizeof(T) + 4 * sizeof(uint16_t);
 
     size_t size = mStorage.size();
     mStorage.resize(size + kAdvance);
@@ -49,7 +49,7 @@
     *(uint16_t*)(current) = kAdvance;
     current += sizeof(uint16_t);
     *(uint16_t*)(current) = ~kAdvance;
-    current += sizeof(uint16_t);
+    current += 3 * sizeof(uint16_t);
 
     T* command = reinterpret_cast<T*>(current);
     mLastCommand = command;
@@ -99,7 +99,7 @@
     }
     DrawingCommand* Get() {
       MOZ_ASSERT(!Done());
-      return reinterpret_cast<DrawingCommand*>(mCurrent + sizeof(uint32_t));
+      return reinterpret_cast<DrawingCommand*>(mCurrent + 2 * sizeof(uint32_t));
     }
 
    private:

Comment 5

[Liviu Seplecan]

For now what can I do is only to set a component, if isn't the proper one, please fell free to change it.
Regards,
Liviu

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jbonisteel, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 7

[Petr Sumbera]

Attachment: Bug 1568533 - make CaptureCommandList.h Append() template use aligned pointers

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:rhunt, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 9

[Ryan Hunt [:rhunt] (on leave until early May)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=457631adf033c49db6843b55d7919021364fe2f6

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d33c1e0b18ab
make CaptureCommandList.h Append() template use aligned pointers r=rhunt

Comment 11

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/d33c1e0b18ab