Bug 111173 - FILEOPEN Calc crashes when opening XLSX with chart.
Summary: FILEOPEN Calc crashes when opening XLSX with chart.
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
5.0 all versions
Hardware: All All
: highest critical
Assignee: Not Assigned
URL:
Whiteboard: target:6.0.0 target:5.4.1 target:5.3.6
Keywords: bibisected, bisected, haveBacktrace, regression
Depends on:
Blocks:
 
Reported: 2017-08-02 18:54 UTC by wolf.duttlinger
Modified: 2017-09-20 12:18 UTC (History)
4 users (show)

See Also:
Crash report or crash signature: ["chart::PieChart::performLabelBestFitInnerPlacement(chart::PieChart::ShapeParam &,chart::PieChart::PieLabelInfo &)"]

Attachments
Excel file causing crash - three tabs with charts mimiking speedometer (38.54 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2017-08-02 18:54 UTC, wolf.duttlinger 		Details
gdb backtrace (51.41 KB, text/plain)
2017-08-02 21:20 UTC, Xisco Faulí 		Details
quick patch (1.13 KB, patch)
2017-08-03 21:14 UTC, Julien Nabet 		Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description wolf.duttlinger 2017-08-02 18:54:07 UTC 
Created attachment 135081 [details]
Excel file causing crash - three tabs with charts mimiking speedometer

Attached file created in Excel. Contains three tabs. On every tab at least one chart. On two tabs each a spin button linked to a cell. Charts trying to mimik a speedometer - combined ring & circle diagram with segments of value 0 (to mimik the hand).

Opening the file in Calc repeatedly crashes - simply popping up the document recovery dialog.

If you want me to supply any furhter data please specify. Attached the file...
Comment 1 Xisco Faulí 2017-08-02 19:34:42 UTC 
Thanks for repoting this bug.
Regression introduced by:

author	Marco Cecchetti <marco.cecchetti@collabora.com>	2015-04-23 07:56:49 (GMT)
committer	Andras Timar <andras.timar@collabora.com>	2015-05-04 09:06:14 (GMT)
commit ea374ab51e229bb1a959a271c1405ef72ad71316 (patch)
tree 8451ab92dcb380fd59ba430a7741d8f9296bab26
parent a21a0b6dceaf965673ae601318e77991919c8f6a (diff)
tdf#90839 - added support for inside placement for the best fit case

bisected with bibisect-50max

Adding Cc: to Marco Cecchetti
Comment 2 Xisco Faulí 2017-08-02 21:20:07 UTC 
Created attachment 135091 [details]
gdb backtrace
Comment 3 Xavier Van Wijmeersch 2017-08-03 16:47:18 UTC 
I confirm the crash with

Version: 6.0.0.0.alpha0+
Build ID: 8a24df0bed643bd06be113284c4439eac9ee62a2
CPU threads: 8; OS: Linux 4.9; UI render: default; VCL: gtk3; 
Locale: nl-BE (en_US.UTF-8); Calc: group

openjdk 8u_144_b01

and also no link for sending the crash report
Comment 4 Julien Nabet 2017-08-03 20:51:59 UTC 
On pc Debian x86-64 with master sources updated today, I could reproduce this.
Here are some extra info:
(gdb) frame 5
#5  0x00007fffbfece0ef in chart::PieChart::performLabelBestFitInnerPlacement (this=0x5555581c9370, rShapeParam=..., rPieLabelInfo=...)
    at /home/julien/lo/libreoffice/chart2/source/view/charttypes/PieChart.cxx:1558
1558	    awt::Point aOldPos( rPieLabelInfo.xLabelGroupShape->getPosition() );
(gdb) p rPieLabelInfo.xLabelGroupShape
$1 = empty uno::Reference
(gdb) p rPieLabelInfo
$2 = (const chart::PieChart::PieLabelInfo &) @0x7ffffffecca0: {xTextShape = empty uno::Reference, xLabelGroupShape = empty uno::Reference, 
  aFirstPosition = {<basegfx::B2ITuple> = {mnX = 5086, mnY = 3323}, <No data fields>}, aOrigin = {<basegfx::B2ITuple> = {mnX = 6276, mnY = 4339}, <No data fields>}, 
  fValue = 45, bMovementAllowed = true, bMoved = false, xTextTarget = uno::Reference to (SvxShapeGroup *) 0x555558309460, pPrevious = 0x0, pNext = 0x0, aPreviousPosition = {
    X = 0, Y = 0}}
Comment 5 Julien Nabet 2017-08-03 21:14:56 UTC 
Created attachment 135107 [details]
quick patch

The only location I found where xLabelGroupShape is initialized is here:

    356     uno::Reference< container::XChild > xChild( aPieLabelInfo.xTextShape, uno::UNO_QUERY );
    357     if( xChild.is() )
    358         aPieLabelInfo.xLabelGroupShape.set( xChild->getParent(), uno::UNO_QUERY );
(see https://opengrok.libreoffice.org/xref/core/chart2/source/view/charttypes/PieChart.cxx#358)

aPieLabelInfo.xTextShape is initialized some lines before:
    350     aPieLabelInfo.xTextShape = createDataLabel(
    351         xTextTarget, rSeries, nPointIndex, nVal, rParam.mfLogicYSum,
    352         aScreenPosition2D, eAlignment, 0, nTextMaximumFrameWidth);

But "createDataLabel" may return an empty ref (see https://opengrok.libreoffice.org/xref/core/chart2/source/view/charttypes/VSeriesPlotter.cxx#406)

I attached a quick (but perhaps dirty or wrong) patch.
With it I could open the file.
Comment 6 Marco Cecchetti 2017-08-08 13:57:32 UTC 
Thanks Julien for your patch and Xisco for providing a debug trace.

I provided a patch here:
https://gerrit.libreoffice.org/#/c/40877

It is not so much different from Julien's one; I just thought safer to skip to append the pie label info into the list.
Comment 7 Commit Notification 2017-08-08 17:02:46 UTC 
Marco Cecchetti committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=05c3791710397346aae0b2a9cca1792567941d62

bug fix: tdf#111173

It will be available in 6.0.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 8 Julien Nabet 2017-08-08 17:04:33 UTC 
Marco: I pushed the patch on master sources.
May I let you cherry-pick it for LO 5.4 and 5.3 branches?
Comment 9 Commit Notification 2017-08-08 17:34:11 UTC 
Marco Cecchetti committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3b66be90d4fffed9cbe61ba99b4ad94daa04f68c

unit test for tdf#111173

It will be available in 6.0.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 10 Marco Cecchetti 2017-08-08 17:38:30 UTC 
(In reply to Julien Nabet from comment #8)
> Marco: I pushed the patch on master sources.
> May I let you cherry-pick it for LO 5.4 and 5.3 branches?

I did it! 

The patch above is the unit test
Comment 11 Commit Notification 2017-08-10 14:10:15 UTC 
Marco Cecchetti committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f44e218ac77e1a6125e645a44c801808df99c7f0&h=libreoffice-5-4

bug fix: tdf#111173

It will be available in 5.4.1.

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

Affected users are encouraged to test the fix and report feedback.
Comment 12 Julien Nabet 2017-08-13 19:45:56 UTC 
Let's put this one to FIXED since :
- it's fixed on master
- it's fixed on 5.4 branch
- there's the patch for 5.3 branch waiting for review.
Comment 13 Xisco Faulí 2017-08-14 08:54:31 UTC 
Verified as per comment 12
Comment 14 Commit Notification 2017-08-22 17:55:43 UTC 
Marco Cecchetti committed a patch related to this issue.
It has been pushed to "libreoffice-5-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=5d3d8958898bae177dfa38a31ce28497e5dd7a95&h=libreoffice-5-3

bug fix: tdf#111173

It will be available in 5.3.6.

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

Affected users are encouraged to test the fix and report feedback.
Comment 15 Commit Notification 2017-09-20 11:45:01 UTC 
Marco Cecchetti committed a patch related to this issue.
It has been pushed to "libreoffice-5-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=136f8cda3bb076c12e5ff3f9bc72d627cdf687f5&h=libreoffice-5-3

unit test for tdf#111173

It will be available in 5.3.7.

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

Affected users are encouraged to test the fix and report feedback.
Comment 16 Julien Nabet 2017-09-20 12:18:16 UTC 
Simplify targets a bit.