Bug 144598 - Crash in: SkRect::round()
Summary: Crash in: SkRect::round()
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
7.1.6.2 release
Hardware: x86-64 (AMD64) Windows (All)
: highest critical
Assignee: Not Assigned
URL:
Whiteboard: target:7.3.0 target:7.2.2 target:7.1.7
Keywords:
: 144174 144434 144617 144630 144679 144736 144753 144870 144881 144887 144900 144909 144944 145011 145026 145097 145120 145209 145214 145222 145224 145227 145273 145278 145294 145304 (view as bug list)
Depends on:
Blocks: Skia
  Show dependency treegraph
 
Reported: 2021-09-19 00:32 UTC by r2
Modified: 2021-10-25 03:04 UTC (History)
34 users (show)

See Also:
Crash report or crash signature: ["SkRect::round()"]


Attachments
w/ redacted service tag (1.98 KB, text/plain)
2021-09-19 16:43 UTC, r2
Details

Note You need to log in before you can comment on or make changes to this bug.
Description r2 2021-09-19 00:32:11 UTC
This bug was filed from the crash reporting server and is br-2520800e-6840-4482-8a82-b0944a0ad962.
=========================================
searched / tried several "start LibreOffice in Safe Mode" solutions/procedures to the MANY other-previously reported variations of 'LO problem-s/does not work/open in Windows 10' - mostly ones about un/re-install/booting, selecting "Disable hardware acceleration" option, archiving profile, resetting factory settings/entire user profile suggestions...getting worn-out, & often times just frustrated & LOST, trying so many different things. DOES work in SafeMode but not believing this should be an on-going issue, somethings not right and i don't think this is a long-term solution for me. FWIW: i've tried OpenOffice, it works, but i prefer the stable LO Suite on my mixture of Linux & W10 machines over the Apache Oo.o option.
Comment 1 Julien Nabet 2021-09-19 07:03:18 UTC
Could you give a try at https://wiki.documentfoundation.org/QA/FirstSteps#Graphics-related_issues_.28Skia.29 ?
Comment 2 Mike Kaganski 2021-09-19 07:14:24 UTC
Note that out safe mode disables Skia, while the "Disable hardware acceleration" in the Safe Mode dialog only disables Vulkan, but not Skia. Hence I know cases where in order to run in normal mode, one must add a line to disable Skia manually into registrymodifications:

<item oor:path="/org.openoffice.Office.Common/VCL"><prop oor:name="UseSkia" oor:op="fuse"><value>false</value></prop></item>

Having an option in the safe mode dialog for that would be nice. Also it would be nice to offer to report the configurations that were healed by the disabling. But indeed it's a separate issue.
Comment 3 Mike Kaganski 2021-09-19 07:17:19 UTC
(In reply to Julien Nabet from comment #1)

Heh, I read there:

> If there is other or even "UI Render: Skia/Raster" instead, it means the
> symptoms aren't caused by Skia,

heh. It is *indeed* wrong. Just yesterday I helped a person over IRC who needed the procedure I described in comment 2; and indeed, the problem *was* caused by Skia, even though the problem was not using Vulkan (even the Skia/Raster was preventing LibreOffice from launch).
Comment 4 Mike Kaganski 2021-09-19 07:19:29 UTC
(Sorry for multiple spamming messages. I will try to not add more.)

@r2: please follow the advise from the link in comment 1:

> If the issue is gone, locate <user profile>/cache/skia.log

and do send it here.
Comment 5 Julien Nabet 2021-09-19 08:03:15 UTC
(In reply to Mike Kaganski from comment #3)
> (In reply to Julien Nabet from comment #1)
> 
> Heh, I read there:
> 
> > If there is other or even "UI Render: Skia/Raster" instead, it means the
> > symptoms aren't caused by Skia,
> 
> heh. It is *indeed* wrong. Just yesterday I helped a person over IRC who
> needed the procedure I described in comment 2; and indeed, the problem *was*
> caused by Skia, even though the problem was not using Vulkan (even the
> Skia/Raster was preventing LibreOffice from launch).

With your previous comment, well spotted!
I think we need a quick and easy way to fix this for users instead of modifying registrymodifications.

Heiko/Xisco/Luboš: Should we create a new entry like 'Disable Skia' or should we use "Disable hardware acceleration" to set "UseSkia" at False ?

The second option should be straightforward since it only requires 1 extra line in BackupFileHelper::tryDisableHWAcceleration in comphelper/source/misc/backupfilehelper.cxx) (see https://opengrok.libreoffice.org/xref/core/comphelper/source/misc/backupfilehelper.cxx?r=a1252364&mo=64108&fi=1892#1892)

Luboš: I read in vcl/skia/README that "raster" was "CPU-based drawing (here primarily used for debugging)"
So except debugging for devs, is it necessary to include "raster" Skia in release versions of LO?
I mean, graphical part is dealt by GPUs or if no GPUs, by chipset included in motherboards so no use of CPU here too.
So if we remove "raster" case, no need to change registrymodifications management here, it would be (at least for Windows) either Skia Vulkan or no Skia at all.
Comment 6 Mike Kaganski 2021-09-19 09:33:40 UTC
(In reply to Julien Nabet from comment #5)
> Luboš: I read in vcl/skia/README that "raster" was "CPU-based drawing (here
> primarily used for debugging)"
> So except debugging for devs, is it necessary to include "raster" Skia in
> release versions of LO?

Skia/raster is *very* important.
1. It works where there's no Vulkan. I am writing from the Windows system having no Vulkan support - it uses AMD Radeon HD 7400 Series graphics.
2. It allows to have unified rendering even on those systems.

I don't think we need to remove Skia/Raster in release; but indeed, as a fallback on problematic systems, we need a way to disable it in the safe mode dialog.
Comment 7 Julien Nabet 2021-09-19 09:44:04 UTC Comment hidden (obsolete)
Comment 8 V Stuart Foote 2021-09-19 13:18:49 UTC
For OP, Needinfo is to you. And could you please describe the GPU available to you and perhaps attach the msinfo32.exe 'Components' -> 'Display' panel content.

I am frankly surprised you could get Windows 10 (19043) running on an Intel Core 2 Duo P9600 system, Win10 graphics support for Skia would depend on GPU available.

As noted above, force disable of Skia rendering may be needed to run LO with CPU only rendering.
Comment 9 r2 2021-09-19 16:43:09 UTC
Created attachment 175123 [details]
w/ redacted service tag

per V Stuart Foote request/comment #8
Comment 10 r2 2021-09-19 16:51:16 UTC
(In reply to Julien Nabet from comment #1)
> Could you give a try at
> https://wiki.documentfoundation.org/QA/FirstSteps#Graphics-related_issues_.
> 28Skia.29 ?

it appears my reply-w/attached file/s email was rejected, i think THISS is the meat of the answer:

Version: 7.1.6.2 (x64) / LibreOffice Community
Build ID: 0e133318fcee89abacd6a7d077e292f1145735c3
CPU threads: 2; OS: Windows 10.0 Build 19043; UI render: default; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: threaded
Comment 11 V Stuart Foote 2021-09-19 17:38:48 UTC
(In reply to r2 from comment #9)
> Created attachment 175123 [details]
> w/ redacted service tag
> 
> per V Stuart Foote request/comment #8

OK thanks. 

Well to be expected the nVidia Quadro NVS 160M is suspect showing Direct 3D support only through 10.0 spec and OpenGL only to the 2.1 spec.

Obviously no Vulkan support, but not surprised modern Skia drivers choke on the hardware.

Assuming it exposes its identity to hw poling, the hardware/driver pair needs deny listing from any Skia rendering with immediate fallback to GDI only rendering.

@Lubos, is that feasible?
Comment 12 Mike Kaganski 2021-09-19 18:08:12 UTC Comment hidden (obsolete)
Comment 13 V Stuart Foote 2021-09-19 18:38:03 UTC
The crash report signature is filled with similar --> NEW
Comment 14 r2 2021-09-19 19:21:38 UTC
not quite sure how, or WHO, to reply w/ this SIGNIFICANT discovery - but i think i've found at least a work-around, possibly a partial solution:
i happen to have an identical Dell e6400 w/ w10 & LO & noticed it (msinfo.exe) has  a diff. INF (oem49 v. oem46) file. then, i check the LO ver. & realized it's running 7.0.6.2, which i KNOW is/was working fine...uninstalled the 7.1.6 on the problem sys & installed w/ 7.0.6 - now appears to be working fine. interestingly, BOTH are denying that there's a update version & reporting "LibreOffice 7.0 is up to date."

sorry i didn't think to cross-check my own available recourses but hope this will contribute to resolving whatever the ver. diff. issue / solution might be worth pursuing. Meny Gras!
Comment 15 Mike Kaganski 2021-09-19 20:09:23 UTC
(In reply to r2 from comment #0)
> This bug was filed from the crash reporting server and is
> br-2520800e-6840-4482-8a82-b0944a0ad962.

Hm, I should had checked the report first.
The problem is not with GPU. Indeed, it's a CPU illegal instruction, caused by some wrong instruction set used for building Skia.

FTR: the disassembly of SkRect::round in 7.1.6.2 on Windows x64 official build is:

> 00007FFC5673B9C0  vmovups     xmm0,xmmword ptr [rcx]  
> 00007FFC5673B9C4  vaddps      xmm0,xmm0,xmmword ptr [__xmm@3f0000003f0000003f0000003f000000 (07FFC567F05D0h)]  
> 00007FFC5673B9CC  mov         rax,rdx  
> 00007FFC5673B9CF  vroundps    xmm0,xmm0,9  
> 00007FFC5673B9D5  vminps      xmm0,xmm0,xmmword ptr [__xmm@4effffff4effffff4effffff4effffff (07FFC567EC910h)]  
> 00007FFC5673B9DD  vmaxps      xmm0,xmm0,xmmword ptr [__xmm@ceffffffceffffffceffffffceffffff (07FFC567EC920h)]  
> 00007FFC5673B9E5  vcvttps2dq  xmm0,xmm0  
> 00007FFC5673B9E9  vmovups     xmmword ptr [rdx],xmm0  
> 00007FFC5673B9ED  ret

Note e.g. vroundps, which is specific for AVX, while our baseline for WindowsX64 is SSE2.

This shows that my idea about the need to disable Skia/Raster is wrong, and simply the bug around building Skia should be fixed. Or maybe it is already fixed in a recent 7.2 version?

At least this is how it looks in 7.2.1.2 x64 official build:

> 00007FFC56583110  push        rsi  
> 00007FFC56583111  sub         rsp,60h  
> 00007FFC56583115  movaps      xmmword ptr [rsp+50h],xmm9  
> 00007FFC5658311B  movaps      xmmword ptr [rsp+40h],xmm8  
> 00007FFC56583121  movaps      xmmword ptr [rsp+30h],xmm7  
> 00007FFC56583126  movaps      xmmword ptr [rsp+20h],xmm6  
> 00007FFC5658312B  mov         rsi,rdx  
> 00007FFC5658312E  movups      xmm6,xmmword ptr [rcx]  
> 00007FFC56583131  addps       xmm6,xmmword ptr [__xmm@3f0000003f0000003f0000003f000000 (07FFC56B5F530h)]  
> 00007FFC56583138  movaps      xmm0,xmm6  
> 00007FFC5658313B  shufps      xmm0,xmm6,0E7h  
> 00007FFC5658313F  call        floorf (07FFC56B58F07h)  
> 00007FFC56583144  movaps      xmm8,xmm0  
> 00007FFC56583148  movaps      xmm0,xmm6  
> 00007FFC5658314B  unpckhpd    xmm0,xmm6  
> 00007FFC5658314F  call        floorf (07FFC56B58F07h)  
> 00007FFC56583154  movaps      xmm9,xmm0  
> 00007FFC56583158  unpcklps    xmm9,xmm8  
> 00007FFC5658315C  movaps      xmm0,xmm6  
> 00007FFC5658315F  call        floorf (07FFC56B58F07h)  
> 00007FFC56583164  movaps      xmm7,xmm0  
> 00007FFC56583167  shufps      xmm6,xmm6,0E5h  
> 00007FFC5658316B  movaps      xmm0,xmm6  
> 00007FFC5658316E  call        floorf (07FFC56B58F07h)  
> 00007FFC56583173  unpcklps    xmm7,xmm0  
> 00007FFC56583176  movlhps     xmm7,xmm9  
> 00007FFC5658317A  minps       xmm7,xmmword ptr [__xmm@4effffff4effffff4effffff4effffff (07FFC56B5B850h)]  
> 00007FFC56583181  maxps       xmm7,xmmword ptr [__xmm@ceffffffceffffffceffffffceffffff (07FFC56B5B860h)]  
> 00007FFC56583188  cvttps2dq   xmm0,xmm7  
> 00007FFC5658318C  movups      xmmword ptr [rsi],xmm0  
> 00007FFC5658318F  mov         rax,rsi  
> 00007FFC56583192  movaps      xmm6,xmmword ptr [rsp+20h]  
> 00007FFC56583197  movaps      xmm7,xmmword ptr [rsp+30h]  
> 00007FFC5658319C  movaps      xmm8,xmmword ptr [rsp+40h]  
> 00007FFC565831A2  movaps      xmm9,xmmword ptr [rsp+50h]  
> 00007FFC565831A8  add         rsp,60h  
> 00007FFC565831AC  pop         rsi  
> 00007FFC565831AD  ret
Comment 16 Luboš Luňák 2021-09-19 21:26:40 UTC
(In reply to Julien Nabet from comment #5)
> should we use "Disable hardware acceleration" to set "UseSkia" at False ?

 No. Skia/Raster has nothing to do with hardware acceleration.

> Heiko/Xisco/Luboš: Should we create a new entry like 'Disable Skia' or

 I don't quite see the point, we generally do not provide options to disable things that are expected to work. It makes sense to disable HW acceleration, because that depends on HW drivers, which are outside of our control. But 'Disable Skia' would be like 'Disable Gtk' or 'Disable Fontconfig', we don't provide options to disable fontconfig just because there may be a bug in it.

> Luboš: I read in vcl/skia/README that "raster" was "CPU-based drawing (here
> primarily used for debugging)"

 That's misleading, Raster is also used as a fallback when Vulkan is not available.

(In reply to Mike Kaganski from comment #15)
> Note e.g. vroundps, which is specific for AVX, while our baseline for
> WindowsX64 is SSE2.
> 
> This shows that my idea about the need to disable Skia/Raster is wrong, and
> simply the bug around building Skia should be fixed. Or maybe it is already
> fixed in a recent 7.2 version?

 I'm not aware of any difference here between 7.1 and 7.2, they both build SkRect using normal Clang settings which should target SSE2. But SkRect::round() is inline in the class, so it's possible there's one copy emitted built with AVX and then we get unlucky when the linker selects that one copy when merging. Since SkOpts_avx.cpp indirectly pulls in SkRect.h, that's most likely what's happening. I'll ask on the Skia mailing list.
Comment 17 Ming Hua 2021-09-20 07:18:05 UTC
(In reply to Mike Kaganski from comment #15)
> Note e.g. vroundps, which is specific for AVX, while our baseline for
> WindowsX64 is SSE2.
It's great that we have a crash report here that developers can look at and diagnose.

I'd just like to point out that this is likely the same issue that got a lot of head-scratching "newer version can't start with Skia enabled" reports like bug 144434, bug 144174, and bug 143152.  Since Luboš said there should be no difference around this area between 7.1 and 7.2 (and there *definitely* shouldn't be any difference between 7.1.5 and 7.1.6), it looks likely that there is some non-deterministic factor in our build system that use the wrong instructions for some builds.
Comment 18 Mike Kaganski 2021-09-20 08:02:05 UTC
FTR: I believe it's https://github.com/google/skia/commit/bb29f4353b138faccc35960c983fbc360486c0c8 that requires inclusion of compilation units built with different instruction set support, having alternative "versions" of functions, as Luboš explained in comment 16, that results in that non-deterministic factor.
Comment 19 Mike Kaganski 2021-09-20 08:23:53 UTC
*** Bug 144434 has been marked as a duplicate of this bug. ***
Comment 20 Mike Kaganski 2021-09-20 08:24:07 UTC
*** Bug 144617 has been marked as a duplicate of this bug. ***
Comment 21 Luboš Luňák 2021-09-20 11:01:28 UTC
*** Bug 144174 has been marked as a duplicate of this bug. ***
Comment 22 Luboš Luňák 2021-09-20 11:05:00 UTC
I expect https://gerrit.libreoffice.org/c/core/+/122335 should take care of it.
Comment 23 Commit Notification 2021-09-20 20:51:27 UTC
Luboš Luňák committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/36f76223193fb96df7b8cbc1a1ff30f739857189

use clang-cl's -Zc:dllexportInlines- for Skia (tdf#144598)

It will be available in 7.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 24 Luboš Luňák 2021-09-21 08:00:25 UTC
Fix pushed to master, backports to 7.1 and 7.2 are waiting in gerrit, closing.

I don't know if there's something more needed to be done for 7.x branches, but I'll leave that to QA to decide.
Comment 25 Commit Notification 2021-09-21 08:25:39 UTC
Luboš Luňák committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/afc221abb6e1895aa62abeca96c8b5043ddae63e

use clang-cl's -Zc:dllexportInlines- for Skia (tdf#144598)

It will be available in 7.2.2.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 26 Commit Notification 2021-09-21 08:25:55 UTC
Luboš Luňák committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

https://git.libreoffice.org/core/commit/11f3ed0ca9f87da6ef5b24f6cb4f58335b78ff7d

use clang-cl's -Zc:dllexportInlines- for Skia (tdf#144598)

It will be available in 7.1.7.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 27 Julien Nabet 2021-09-23 19:38:48 UTC
*** Bug 144679 has been marked as a duplicate of this bug. ***
Comment 28 Julien Nabet 2021-09-23 19:47:02 UTC Comment hidden (obsolete)
Comment 29 Julien Nabet 2021-09-24 19:21:44 UTC Comment hidden (obsolete)
Comment 30 Julien Nabet 2021-09-25 13:13:54 UTC
*** Bug 144630 has been marked as a duplicate of this bug. ***
Comment 31 Julien Nabet 2021-09-26 20:08:15 UTC
*** Bug 144736 has been marked as a duplicate of this bug. ***
Comment 32 V Stuart Foote 2021-09-27 23:06:45 UTC Comment hidden (obsolete)
Comment 33 Mike Kaganski 2021-10-02 09:31:41 UTC
*** Bug 144870 has been marked as a duplicate of this bug. ***
Comment 34 Mike Kaganski 2021-10-02 20:03:23 UTC
*** Bug 144881 has been marked as a duplicate of this bug. ***
Comment 35 Jan-Marek Glogowski 2021-10-03 09:30:10 UTC
*** Bug 144887 has been marked as a duplicate of this bug. ***
Comment 36 Xisco Faulí 2021-10-04 10:22:57 UTC
*** Bug 144900 has been marked as a duplicate of this bug. ***
Comment 37 Mike Kaganski 2021-10-04 10:23:43 UTC
*** Bug 144909 has been marked as a duplicate of this bug. ***
Comment 38 Ming Hua 2021-10-05 12:37:43 UTC
*** Bug 144944 has been marked as a duplicate of this bug. ***
Comment 39 V Stuart Foote 2021-10-05 17:57:35 UTC
*** Bug 144753 has been marked as a duplicate of this bug. ***
Comment 40 Mike Kaganski 2021-10-09 06:21:02 UTC
*** Bug 145011 has been marked as a duplicate of this bug. ***
Comment 41 Julien Nabet 2021-10-10 06:35:50 UTC
*** Bug 145026 has been marked as a duplicate of this bug. ***
Comment 42 Mike Kaganski 2021-10-12 15:43:27 UTC
*** Bug 145097 has been marked as a duplicate of this bug. ***
Comment 43 Xisco Faulí 2021-10-14 08:49:53 UTC
*** Bug 145120 has been marked as a duplicate of this bug. ***
Comment 44 Mike Kaganski 2021-10-18 17:32:28 UTC
*** Bug 145209 has been marked as a duplicate of this bug. ***
Comment 45 Ming Hua 2021-10-19 02:39:48 UTC
*** Bug 145214 has been marked as a duplicate of this bug. ***
Comment 46 Mike Kaganski 2021-10-19 12:16:45 UTC
*** Bug 145222 has been marked as a duplicate of this bug. ***
Comment 47 Xisco Faulí 2021-10-19 13:29:00 UTC
*** Bug 145224 has been marked as a duplicate of this bug. ***
Comment 48 Mike Kaganski 2021-10-19 20:35:32 UTC
*** Bug 145227 has been marked as a duplicate of this bug. ***
Comment 49 Xisco Faulí 2021-10-22 09:53:40 UTC
*** Bug 145273 has been marked as a duplicate of this bug. ***
Comment 50 Jan-Marek Glogowski 2021-10-22 16:55:19 UTC
*** Bug 145278 has been marked as a duplicate of this bug. ***
Comment 51 Mike Kaganski 2021-10-24 12:03:33 UTC
*** Bug 145294 has been marked as a duplicate of this bug. ***
Comment 52 Ming Hua 2021-10-25 03:04:14 UTC
*** Bug 145304 has been marked as a duplicate of this bug. ***