Closed
Bug 1448189
Opened 6 years ago
Closed 6 years ago
Startup crash [@ _sk_start_pipeline_sse2]
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox59 | --- | unaffected |
firefox60 | --- | fixed |
firefox61 | --- | fixed |
People
(Reporter: jbeich, Assigned: jbeich)
References
Details
(Keywords: crash, regression)
Attachments
(1 file, 2 obsolete files)
59 bytes,
text/x-review-board-request
|
lsalzman
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
mozilla-unified changeset 998a57bd05ad snapshot built by Clang 5.0.1 on FreeBSD 10.3 i386. Doesn't affect amd64. $ rm -rf objdir/tmp/scratch_user; mkdir -p objdir/tmp/scratch_user $ echo 'user_pref("browser.tabs.remote.autostart", false);' >>objdir/tmp/scratch_user/user.js $ gdb --args objdir/dist/bin/firefox -no-remote -profile objdir/tmp/scratch_user [...] Thread 1 received signal SIGBUS, Bus error. 0x3155e265 in _sk_start_pipeline_sse2 () from objdir/dist/bin/libxul.so (gdb) backtrace #0 0x3155e265 in _sk_start_pipeline_sse2 () from objdir/dist/bin/libxul.so #1 0x31769c03 in SkRasterPipeline::run (this=0xffff74fc, x=0, y=0, w=1, h=1) at gfx/skia/skia/src/jumper/SkJumper.cpp:481 #2 0x31f47a2d in SkRasterPipelineBlitter::Create (dst=..., paint=..., alloc=0xffff7494, shaderPipeline=..., burstCtx=0x0, is_opaque=true, is_constant=true) at gfx/skia/skia/src/core/SkRasterPipelineBlitter.cpp:198 #3 0x31f47295 in SkCreateRasterPipelineBlitter (dst=..., paint=..., ctm=..., alloc=0xffff7494) at gfx/skia/skia/src/core/SkRasterPipelineBlitter.cpp:101 #4 0x3157ba27 in SkBlitter::Choose (device=..., matrix=..., origPaint=..., alloc=0xffff7494, drawCoverage=false) at gfx/skia/skia/src/core/SkBlitter.cpp:1001 #5 0x31b2cee0 in SkAutoBlitterChoose::choose (this=0xffff7490, dst=..., matrix=..., paint=..., drawCoverage=false) at gfx/skia/skia/src/core/SkAutoBlitterChoose.h:34 #6 0x31b23ba8 in SkDraw::drawDevPath (this=0xffff83a8, devPath=..., paint=..., drawCoverage=false, customBlitter=0x0, doFill=true, iData=0x0) at gfx/skia/skia/src/core/SkDraw.cpp:969 #7 0x31b25708 in SkDraw::drawPath (this=0xffff83a8, origSrcPath=..., origPaint=..., prePathMatrix=0x0, pathIsMutable=false, drawCoverage=false, customBlitter=0x0, iData=0x0) at gfx/skia/skia/src/core/SkDraw.cpp:1141 #8 0x3165fcc9 in SkDraw::drawPath (this=0xffff83a8, path=..., paint=..., prePathMatrix=0x0, pathIsMutable=false) at gfx/skia/skia/src/core/SkDraw.h:58 #9 0x318bd0fe in SkBitmapDevice::drawPath (this=0x420f9800, path=..., paint=..., prePathMatrix=0x0, pathIsMutable=false) at gfx/skia/skia/src/core/SkBitmapDevice.cpp:226 #10 0x318e0823 in SkCanvas::onDrawPath (this=0x4206d000, path=..., paint=...) at gfx/skia/skia/src/core/SkCanvas.cpp:2170 #11 0x318dd07f in SkCanvas::drawPath (this=0x4206d000, path=..., paint=...) at gfx/skia/skia/src/core/SkCanvas.cpp:1760 #12 0x2c639811 in mozilla::gfx::DrawTargetSkia::Fill (this=0x41eba580, aPath=0x36032f20, aPattern=..., aOptions=...) at gfx/2d/DrawTargetSkia.cpp:976 #13 0x2cda8dbd in CreateBoxShadow (aDestDrawTarget=0x41c3fa50, aMinSize=..., aCornerRadii=0xffff8fb8, aBlurRadius=..., aShadowColor=..., aMirrorCorners=true, aOutBlurMargin=...) at gfx/thebes/gfxBlur.cpp:544 #14 0x2cd8529e in GetBlur (aDestinationCtx=0x420801c0, aRectSize=..., aBlurRadius=..., aCornerRadii=0xffff8fb8, aShadowColor=..., aMirrorCorners=true, aOutBlurMargin=..., aOutSlice=..., aOutMinSize=...) at gfx/thebes/gfxBlur.cpp:612 #15 0x2cd848cb in gfxAlphaBoxBlur::BlurRectangle (aDestinationCtx=0x420801c0, aRect=..., aCornerRadii=0xffff8fb8, aBlurStdDev=..., aShadowColor=..., aDirtyRect=..., aSkipRect=...) at gfx/thebes/gfxBlur.cpp:963 #16 0x30a6e9e0 in nsContextBoxBlur::BlurRectangle (aDestinationCtx=0x420801c0, aRect=..., aAppUnitsPerDevPixel=60, aCornerRadii=0xffff8fb8, aBlurRadius=240, aShadowColor=..., aDirtyRect=..., aSkipRect=...) at layout/painting/nsCSSRendering.cpp:4601 #17 0x30a6d171 in nsCSSRendering::PaintBoxShadowOuter (aPresContext=0x3fb9ae00, aRenderingContext=..., aForFrame=0x41a60ab8, aFrameArea=..., aDirtyRect=..., aOpacity=1) at layout/painting/nsCSSRendering.cpp:1771 #18 0x30aaabf2 in nsDisplayBoxShadowOuter::Paint (this=0x41f8d190, aBuilder=0xffffa628, aCtx=0x420801c0) at layout/painting/nsDisplayList.cpp:5835 #19 0x30a5ecf8 in mozilla::FrameLayerBuilder::PaintItems (this=0x420cd940, aItems=..., aRect=..., aContext=0x420801c0, aBuilder=0xffffa628, aPresContext=0x3fb9ae00, aOffset=..., aXScale=1, aYScale=1, aCommonClipCount=0) at layout/painting/FrameLayerBuilder.cpp:6097 #20 0x30a5f80c in mozilla::FrameLayerBuilder::DrawPaintedLayer (aLayer=0x37481600, aContext=0x420801c0, aRegionToDraw=..., aDirtyRegion=..., aClip=mozilla::layers::DrawRegionClip::DRAW, aRegionToInvalidate=..., aCallbackData=0xffffa628) at layout/painting/FrameLayerBuilder.cpp:6254 #21 0x2cb792a3 in mozilla::layers::ClientPaintedLayer::PaintThebes (this=0x37481600, aReadbackUpdates=0xffff99c0) at gfx/layers/client/ClientPaintedLayer.cpp:158 #22 0x2cb7a30c in mozilla::layers::ClientPaintedLayer::RenderLayerWithReadback (this=0x37481600, aReadback=0xffff9a30) at gfx/layers/client/ClientPaintedLayer.cpp:314 #23 0x2cb7a37b in non-virtual thunk to mozilla::layers::ClientPaintedLayer::RenderLayerWithReadback(mozilla::layers::ReadbackProcessor*) () at objdir/dist/include/mozilla/layers/LayersTypes.h:291 #24 0x2cb9f16b in mozilla::layers::ClientContainerLayer::RenderLayer (this=0x41cefa00) at gfx/layers/client/ClientContainerLayer.h:58 #25 0x2cb9f30e in non-virtual thunk to mozilla::layers::ClientContainerLayer::RenderLayer() () at objdir/dist/include/mozilla/layers/ContentClient.h:257 #26 0x2cb7644a in mozilla::layers::ClientLayerManager::EndTransactionInternal (this=0x37490520, , mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*)>, aCallbackData=0xffffa628) at gfx/layers/client/ClientLayerManager.cpp:359 #27 0x2cb768af in mozilla::layers::ClientLayerManager::EndTransaction (this=0x37490520, aCallback=0x30a5efa0 <mozilla::FrameLayerBuilder::DrawPaintedLayer(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*)>, aCallbackData=0xffffa628, aFlags=mozilla::layers::LayerManager::END_DEFAULT) at gfx/layers/client/ClientLayerManager.cpp:423 #28 0x30a999e2 in nsDisplayList::PaintRoot (this=0xffffa610, aBuilder=0xffffa628, aCtx=0x0, aFlags=13) at layout/painting/nsDisplayList.cpp:2779 #29 0x305bdc2d in nsLayoutUtils::PaintFrame (aRenderingContext=0x0, aFrame=0x40f79010, aDirtyRegion=..., aBackstop=4293388520, aBuilderMode=nsDisplayListBuilderMode::PAINTING, aFlags=(nsLayoutUtils::PaintFrameFlags::PAINT_WIDGET_LAYERS | nsLayoutUtils::PaintFrameFlags::PAINT_EXISTING_TRANSACTION | nsLayoutUtils::PaintFrameFlags::PAINT_NO_COMPOSITE)) at layout/base/nsLayoutUtils.cpp:4016 #30 0x305209a2 in mozilla::PresShell::Paint (this=0x28892800, aViewToPaint=0x3fbfeb80, aDirtyRegion=..., aFlags=1) at layout/base/PresShell.cpp:6435 #31 0x2ffc08bb in nsViewManager::ProcessPendingUpdatesPaint (this=0x400656a0, aWidget=0x3a3a2640) at view/nsViewManager.cpp:480 #32 0x2ffc03a8 in nsViewManager::ProcessPendingUpdatesForView (this=0x400656a0, aView=0x3fbfeb80, aFlushDirtyRegion=true) at view/nsViewManager.cpp:412 #33 0x2ffc17e4 in nsViewManager::ProcessPendingUpdates (this=0x400656a0) at view/nsViewManager.cpp:1102 #34 0x304cc230 in nsRefreshDriver::Tick (this=0x3a21c600, aNowEpoch=1521761518002803, aNowTime=...) at layout/base/nsRefreshDriver.cpp:2064 #35 0x304d48ae in mozilla::RefreshDriverTimer::TickDriver (driver=0x3a21c600, jsnow=1521761518002803, now=...) at layout/base/nsRefreshDriver.cpp:341 #36 0x304d458f in mozilla::RefreshDriverTimer::TickRefreshDrivers (this=0x3a20d1c0, aJsNow=1521761518002803, aNow=..., aDrivers=...) at layout/base/nsRefreshDriver.cpp:311 #37 0x304d43a6 in mozilla::RefreshDriverTimer::Tick (this=0x3a20d1c0, jsnow=1521761518002803, now=...) at layout/base/nsRefreshDriver.cpp:333 #38 0x304d786f in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers (this=0x3a20d1c0, aTimeStamp=...) at layout/base/nsRefreshDriver.cpp:774 #39 0x304d6874 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver ( this=0x3a207920, aVsyncTimestamp=...) at layout/base/nsRefreshDriver.cpp:687 #40 0x304d2dd8 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run ( this=0x3cc0e180) at layout/base/nsRefreshDriver.cpp:533 #41 0x2aaf42fe in nsThread::ProcessNextEvent (this=0x2884a5e0, aMayWait=false, aResult=0xffffcc61) at xpcom/threads/nsThread.cpp:1040 #42 0x2ab1f054 in NS_ProcessNextEvent (aThread=0x2884a5e0, aMayWait=false) at xpcom/threads/nsThreadUtils.cpp:517 #43 0x2b6b5b1e in mozilla::ipc::MessagePump::Run (this=0x288ccbb0, aDelegate=0x2880f480) at ipc/glue/MessagePump.cpp:97 #44 0x2b5ad481 in MessageLoop::RunInternal (this=0x2880f480) at ipc/chromium/src/base/message_loop.cc:326 #45 0x2b5ad406 in MessageLoop::RunHandler (this=0x2880f480) at ipc/chromium/src/base/message_loop.cc:319 #46 0x2b5ad3bb in MessageLoop::Run (this=0x2880f480) at ipc/chromium/src/base/message_loop.cc:299 #47 0x3002a8e2 in nsBaseAppShell::Run (this=0x3a13fe20) at widget/nsBaseAppShell.cpp:157 #48 0x32b99de9 in nsAppStartup::Run (this=0x28b9a070) at toolkit/components/startup/nsAppStartup.cpp:290 #49 0x32d1aec7 in XREMain::XRE_mainRun (this=0xffffd480) at toolkit/xre/nsAppRunner.cpp:4727 #50 0x32d1c2ab in XREMain::XRE_main (this=0xffffd480, argc=4, argv=0xffffdafc, aConfig=...) at toolkit/xre/nsAppRunner.cpp:4872 #51 0x32d1cbd7 in XRE_main (argc=4, argv=0xffffdafc, aConfig=...) at toolkit/xre/nsAppRunner.cpp:4964 #52 0x32d35fd2 in mozilla::BootstrapImpl::XRE_main (this=0x288153e0, argc=4, argv=0xffffdafc, aConfig=...) at toolkit/xre/Bootstrap.cpp:49 #53 0x0804e305 in do_main (argc=4, argv=0xffffdafc, envp=0xffffdb10) at browser/app/nsBrowserApp.cpp:231 #54 0x0804dde4 in main (argc=4, argv=0xffffdafc, envp=0xffffdb10) at browser/app/nsBrowserApp.cpp:304 (gdb) disassemble Dump of assembler code for function _sk_start_pipeline_sse2: 0x3155e220 <+0>: push %ebp 0x3155e221 <+1>: mov %esp,%ebp 0x3155e223 <+3>: push %ebx 0x3155e224 <+4>: push %edi 0x3155e225 <+5>: push %esi 0x3155e226 <+6>: sub $0x5c,%esp 0x3155e229 <+9>: mov 0xc(%ebp),%eax 0x3155e22c <+12>: cmp 0x14(%ebp),%eax 0x3155e22f <+15>: jae 0x3155e2d6 <_sk_start_pipeline_sse2+182> 0x3155e235 <+21>: mov 0x10(%ebp),%edx 0x3155e238 <+24>: mov 0x8(%ebp),%ecx 0x3155e23b <+27>: mov 0x18(%ebp),%esi 0x3155e23e <+30>: mov (%esi),%edi 0x3155e240 <+32>: add $0x4,%esi 0x3155e243 <+35>: lea 0x4(%ecx),%ecx 0x3155e246 <+38>: mov %ecx,-0x10(%ebp) 0x3155e249 <+41>: cmp %edx,-0x10(%ebp) 0x3155e24c <+44>: mov 0x8(%ebp),%ecx 0x3155e24f <+47>: mov %ecx,-0x68(%ebp) 0x3155e252 <+50>: mov %eax,-0x14(%ebp) 0x3155e255 <+53>: mov %eax,-0x64(%ebp) 0x3155e258 <+56>: movl $0x0,-0x60(%ebp) 0x3155e25f <+63>: lea -0x58(%ebp),%eax 0x3155e262 <+66>: xorps %xmm0,%xmm0 => 0x3155e265 <+69>: movaps %xmm0,0x30(%eax) 0x3155e269 <+73>: movaps %xmm0,0x20(%eax) 0x3155e26d <+77>: movaps %xmm0,0x10(%eax) 0x3155e271 <+81>: movaps %xmm0,(%eax) 0x3155e274 <+84>: mov %ecx,%eax 0x3155e276 <+86>: lea -0x68(%ebp),%ebx 0x3155e279 <+89>: ja 0x3155e2a4 <_sk_start_pipeline_sse2+132> 0x3155e27b <+91>: sub $0x8,%esp 0x3155e27e <+94>: xorps %xmm0,%xmm0 0x3155e281 <+97>: xorps %xmm1,%xmm1 0x3155e284 <+100>: xorps %xmm2,%xmm2 0x3155e287 <+103>: xorps %xmm3,%xmm3 0x3155e28a <+106>: push %esi 0x3155e28b <+107>: push %ebx 0x3155e28c <+108>: call *%edi 0x3155e28e <+110>: mov 0x10(%ebp),%edx 0x3155e291 <+113>: add $0x10,%esp 0x3155e294 <+116>: mov -0x68(%ebp),%ecx 0x3155e297 <+119>: lea 0x4(%ecx),%eax 0x3155e29a <+122>: mov %eax,-0x68(%ebp) 0x3155e29d <+125>: add $0x8,%ecx 0x3155e2a0 <+128>: cmp %edx,%ecx 0x3155e2a2 <+130>: jbe 0x3155e27b <_sk_start_pipeline_sse2+91> 0x3155e2a4 <+132>: mov %edx,%ecx 0x3155e2a6 <+134>: sub %eax,%ecx 0x3155e2a8 <+136>: je 0x3155e2c9 <_sk_start_pipeline_sse2+169> 0x3155e2aa <+138>: mov %ecx,-0x60(%ebp) 0x3155e2ad <+141>: sub $0x8,%esp 0x3155e2b0 <+144>: xorps %xmm0,%xmm0 0x3155e2b3 <+147>: xorps %xmm1,%xmm1 0x3155e2b6 <+150>: xorps %xmm2,%xmm2 0x3155e2b9 <+153>: xorps %xmm3,%xmm3 0x3155e2bc <+156>: push %esi 0x3155e2bd <+157>: lea -0x68(%ebp),%eax 0x3155e2c0 <+160>: push %eax 0x3155e2c1 <+161>: call *%edi 0x3155e2c3 <+163>: mov 0x10(%ebp),%edx 0x3155e2c6 <+166>: add $0x10,%esp 0x3155e2c9 <+169>: mov -0x14(%ebp),%eax 0x3155e2cc <+172>: inc %eax 0x3155e2cd <+173>: cmp 0x14(%ebp),%eax 0x3155e2d0 <+176>: jne 0x3155e249 <_sk_start_pipeline_sse2+41> 0x3155e2d6 <+182>: add $0x5c,%esp 0x3155e2d9 <+185>: pop %esi 0x3155e2da <+186>: pop %edi 0x3155e2db <+187>: pop %ebx 0x3155e2dc <+188>: pop %ebp 0x3155e2dd <+189>: ret End of assembler dump. (gdb) info registers eax 0xffff6b68 -38040 ecx 0x0 0 edx 0x1 1 ebx 0x35eeb000 904835072 esp 0xffff6b58 0xffff6b58 ebp 0xffff6bc0 0xffff6bc0 esi 0xffff6c18 -37864 edi 0x3155e491 827712657 eip 0x3155e265 0x3155e265 <_sk_start_pipeline_sse2+69> eflags 0x210206 [ PF IF RF ID ] cs 0x33 51 ss 0x3b 59 ds <unavailable> es <unavailable> fs <unavailable> gs <unavailable> https://searchfox.org/mozilla-central/source/gfx/skia/skia/src/jumper/SkJumper_generated.S#36148-36219 https://skia.googlesource.com/skia/+log/chrome/m59..chrome/m66/src/jumper/SkJumper_generated.S
When built by GCC 6.4.0 it also crashes in SSE2 code but unrelated to Skia: https://pastebin.mozilla.org/9080697
Dimitry, does this look related to https://lists.freebsd.org/pipermail/freebsd-current/2012-November/037563.html ? If you want to try reproduce downstream, see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=226476
Flags: needinfo?(dimitry)
Comment 3•6 years ago
|
||
It looks like Skia expects the i386 ABI to respect the fact that the stack is 16 byte aligned. And FreeBSD i386 does not seem to respect this? Is there any way to alter the FreeBSD build flags for Firefox to accommodate this?
Flags: needinfo?(jbeich)
-mstack-alignment=16 should be enough (like in FFmpeg) but only together with -mstackrealign actually helped. Clang defaults to 4-byte alignment but most platforms except BSDs switched to 16-byte, following GCC. https://reviews.llvm.org/diffusion/L/browse/llvm/tags/RELEASE_600/final/lib/Target/X86/X86Subtarget.cpp;328295$243-245
Flags: needinfo?(jbeich)
Attachment #8961659 -
Flags: feedback?(lsalzman)
Comment 5•6 years ago
|
||
(In reply to Jan Beich from comment #4) > Created attachment 8961659 [details] [diff] [review] > workaround > > -mstack-alignment=16 should be enough (like in FFmpeg) but only together > with -mstackrealign actually helped. Clang defaults to 4-byte alignment but > most platforms except BSDs switched to 16-byte, following GCC. You mean, most platforms simply broke their existing SysV ABI, without any regard for backwards compatibility. :) Note that FreeBSD ports also patches ffmpeg to use -mstackrealign, because of the same issue: https://svnweb.freebsd.org/ports/head/multimedia/ffmpeg/files/patch-configure?revision=452570&view=markup
Flags: needinfo?(dimitry)
Comment 6•6 years ago
|
||
(In reply to Dimitry Andric from comment #5) > (In reply to Jan Beich from comment #4) > > Created attachment 8961659 [details] [diff] [review] > > workaround > > > > -mstack-alignment=16 should be enough (like in FFmpeg) but only together > > with -mstackrealign actually helped. Clang defaults to 4-byte alignment but > > most platforms except BSDs switched to 16-byte, following GCC. > > You mean, most platforms simply broke their existing SysV ABI, without any > regard for backwards compatibility. :) That's called "cleaning up old cruft and really fixing past mistakes" ;)
Comment 7•6 years ago
|
||
Comment on attachment 8961659 [details] [diff] [review] workaround I like this as a solution since SkJumper.cpp is the single jumping-off point into the assembly that has the alignment constraints, -mstackrealign neatly bridges us to it without a lot of fuss. Also avoids having to adjust build flags on Firefox as a whole. Ship it!
Attachment #8961659 -
Flags: review+
Attachment #8961659 -
Flags: feedback?(lsalzman)
Attachment #8961659 -
Flags: feedback+
Dimitry, do other Clang platforms like Android or Windows that don't explicitly set stackAlignment = 16 need -mstackrealign as well?
Flags: needinfo?(dimitry)
Comment 9•6 years ago
|
||
(In reply to Jan Beich from comment #8) > Dimitry, do other Clang platforms like Android or Windows that don't > explicitly set stackAlignment = 16 need -mstackrealign as well? The default stackAlignment is indeed set to 4, and is only set to 16 when the target is Darwin, Linux, Solaris or kFreeBSD, or when in 64-bit mode. For Windows 32-bit, the default alignment will also be 4 bytes. I don't know what Microsoft's own compilers do here. For Android, it appears that the target triples are of the form "cpu-linux-android*", e.g. "arm-linux-androideabi" or "i686-linux-android", so in that case it will still be identified as Linux (which it of course is). Therefore its stack alignment will be 16 bytes too, even for 32-bit.
Flags: needinfo?(dimitry)
Assignee | ||
Comment 10•6 years ago
|
||
Comment on attachment 8961659 [details] [diff] [review] workaround https://treeherder.mozilla.org/#/jobs?repo=try&revision=e03a91c548ec3c9f104093e1525037ef10e960de
Assignee | ||
Comment 11•6 years ago
|
||
Thanks. I was confused by isTargetLinux vs. isTargetAndroid. As the previous patch was fine this version only improves wording: - Expand inline comment to note where the target exclusion list originates - Adjust commit message to imply the code calling SSE2 was realigned Keeping r+ from comment 7.
Attachment #8961659 -
Attachment is obsolete: true
Keywords: checkin-needed
Comment 12•6 years ago
|
||
(In reply to Jan Beich from comment #11) > Created attachment 8961977 [details] [diff] [review] > workaround > > Thanks. I was confused by isTargetLinux vs. isTargetAndroid. > > As the previous patch was fine this version only improves wording: > - Expand inline comment to note where the target exclusion list originates > - Adjust commit message to imply the code calling SSE2 was realigned > > Keeping r+ from comment 7. I think it would be simpler here if we just got rid of the OS_TARGET condition. Just always using -mstack-alignment=16 and -mstackrealign if we're doing the x86/clang combo should be harmless.
Keywords: checkin-needed
Comment hidden (mozreview-request) |
Assignee | ||
Comment 14•6 years ago
|
||
Comment on attachment 8962014 [details] Bug 1448189 - Realign stack before using SSE2 in Skia. Changes since attachment 8961977 [details] [diff] [review]: - Addressed comment 11
Attachment #8961977 -
Attachment is obsolete: true
Comment 15•6 years ago
|
||
mozreview-review |
Comment on attachment 8962014 [details] Bug 1448189 - Realign stack before using SSE2 in Skia. https://reviewboard.mozilla.org/r/230856/#review236346
Attachment #8962014 -
Flags: review?(lsalzman) → review+
Comment 16•6 years ago
|
||
FWIW (I'm not a reviewer), Jan's fix looks good to me.
Keywords: checkin-needed
Updated•6 years ago
|
Assignee: nobody → jbeich
Comment 17•6 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/afbeca340d12 Realign stack before using SSE2 in Skia. r=lsalzman
Keywords: checkin-needed
Comment 18•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/afbeca340d12
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment 19•6 years ago
|
||
Pushed by lsalzman@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ec40368b843a follow-up - Don't use -mstackrealign with clang-cl. r=jbeich
Assignee | ||
Comment 20•6 years ago
|
||
Comment on attachment 8962014 [details] Bug 1448189 - Realign stack before using SSE2 in Skia. Approval Request Comment [Feature/Bug causing the regression]: bug 1444506 [User impact if declined]: Firefox crashes on FreeBSD/OpenBSD x86. [Is this code covered by automated tests?]: Yes, crashtests but not on Tier3 platforms. [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: mozilla-central changeset ec40368b843a [Is the change risky?]: No [Why is the change risky/not risky?]: Can only break build or fail crashtests on Android x86 or Linux x86 ASAN in case of a compiler bug. [String changes made/needed]: None
Attachment #8962014 -
Flags: approval-mozilla-beta?
Comment 21•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ec40368b843a
Comment 23•6 years ago
|
||
Comment on attachment 8962014 [details] Bug 1448189 - Realign stack before using SSE2 in Skia. fix stack alignment on bsd with clang, beta60+
Attachment #8962014 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 24•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/a15c367a272e
You need to log in
before you can comment on or make changes to this bug.
Description
•