Bug Hunting Session
Bug 60420 - Copy chart leads to crash
Summary: Copy chart leads to crash
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Chart (show other bugs)
Version:
(earliest affected)
4.0.0.3 release
Hardware: Other All
: medium critical
Assignee: Julien Nabet
URL:
Whiteboard: target:4.1.0 target:4.0.1
Keywords:
: 60960 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-02-07 13:03 UTC by Ruslan Kabatsayev
Modified: 2013-02-16 18:45 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Test document (28.91 KB, application/vnd.oasis.opendocument.spreadsheet)
2013-02-07 13:03 UTC, Ruslan Kabatsayev
Details
bt + console logs on master (9.14 KB, text/plain)
2013-02-07 20:22 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ruslan Kabatsayev 2013-02-07 13:03:03 UTC
Created attachment 74340 [details]
Test document

How to reproduce:
1. Open test document
2. Select the chart
3. Ctrl+C
4. Get crash

Here's the backtrace: http://pastebin.com/DF58wAgV
Comment 1 Jorendc 2013-02-07 13:37:35 UTC
Thanks for reporting!

I can reproduce this behavior using Linux Mint 14 x64 with LibreOffice 4.0.0.3 rc3. Because this is currently the oldest version we can confirm this behavior, I set the bug version to this LibreOffice version.

Following [1] I mark this bug as 'Critical Medium'. Critical: because of the crash. Medium: I did some test using other charts, and can't reproduce this crash.

[1] https://wiki.documentfoundation.org/images/0/06/Prioritizing_Bugs_Flowchart.jpg

Kind regards,
Joren
Comment 2 Jorendc 2013-02-07 13:39:19 UTC
@Markus, Kohei, Eike: I know there where some issues with copy/past charts on the same sheet, and when the sheet wasn't named as 'sheet1' (as default). Is this problem maybe related?

Thanks for your time,
Joren
Comment 3 Julien Nabet 2013-02-07 20:22:40 UTC
Created attachment 74379 [details]
bt + console logs on master

On pc Debian x86-64 with master sources updated today, I reproduced the crash.

I attached console logs + bt.
Comment 4 Julien Nabet 2013-02-07 20:25:24 UTC
Sometimes "it" = maItems.end()
so aCopied.reserve(std::distance(it, itEnd)); crashes

With this patch, I don't reproduce the problem:
diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
index ad156ef..59c470b 100644
--- a/sc/source/core/data/column.cxx
+++ b/sc/source/core/data/column.cxx
@@ -1247,9 +1247,12 @@ void ScColumn::CopyStaticToDocument(SCROW nRow1, SCROW nRow2, ScColumn& rDestCol
     it = std::find_if(maItems.begin(), maItems.end(), FindInRows(nRow1, nRow2));
     if (it != maItems.end())
         itEnd = std::find_if(it, maItems.end(), FindAboveRow(nRow2));
+    else
+        return;

But perhaps it's not so simple

If ok, I can push this patch on master.
Comment 5 Kohei Yoshida 2013-02-07 20:36:09 UTC
I'll look at this later.
Comment 6 Ruslan Kabatsayev 2013-02-07 20:43:19 UTC
(In reply to comment #4)
I confirm this patch prevents crash.
Comment 7 Kohei Yoshida 2013-02-07 20:44:44 UTC
(In reply to comment #4)
> Sometimes "it" = maItems.end()
> so aCopied.reserve(std::distance(it, itEnd)); crashes
> 
> With this patch, I don't reproduce the problem:
> diff --git a/sc/source/core/data/column.cxx b/sc/source/core/data/column.cxx
> index ad156ef..59c470b 100644
> --- a/sc/source/core/data/column.cxx
> +++ b/sc/source/core/data/column.cxx
> @@ -1247,9 +1247,12 @@ void ScColumn::CopyStaticToDocument(SCROW nRow1,
> SCROW nRow2, ScColumn& rDestCol
>      it = std::find_if(maItems.begin(), maItems.end(), FindInRows(nRow1,
> nRow2));
>      if (it != maItems.end())
>          itEnd = std::find_if(it, maItems.end(), FindAboveRow(nRow2));
> +    else
> +        return;
> 
> But perhaps it's not so simple
> 
> If ok, I can push this patch on master.

Actually, Julien, that fix is just fine & makes sense.  But can you change one thing for me?

Instead of the above, let's check for it == maItems.end() and if that's true, return right then.  So, the code should look like

if (it == maItems.end())
    // Nothing to copy.
    return;

itEnd = std::find_if(it, maItems.end(), FindAboveRow(nRow2));

(and continue on)

That should be equivalent of your fix, but looks a bit better.

Once you push it to master, let me know.  I'll cherry-pick it to the 4.0 branch.

Thanks a lot!
Comment 8 Not Assigned 2013-02-07 21:29:51 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7e2f34a3172a756a0fd70cae1294250206945a6e

Resolves: fdo#60420 Copy chart leads to crash



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 9 Not Assigned 2013-02-07 21:35:51 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-4-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=9fe8668203eae2f66c837b266847d639a5dac42c&h=libreoffice-4-0

Resolves: fdo#60420 Copy chart leads to crash


It will be available in LibreOffice 4.0.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 10 Kohei Yoshida 2013-02-07 21:40:27 UTC
I'll mark this fixed.
Comment 11 Rainer Bielefeld Retired 2013-02-16 18:45:57 UTC
*** Bug 60960 has been marked as a duplicate of this bug. ***