Bug 109863 - Crash in: XPropertyList::Count()
Summary: Crash in: XPropertyList::Count()
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
5.3.0.0.alpha0+
Hardware: All All
: highest critical
Assignee: Julien Nabet
QA Contact:
URL:
Whiteboard: target:6.0.0 target:5.4.1 target:5.3....
Keywords: bibisected, bisected, regression
: 111741 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-07-28 20:38 UTC by Telesto
Modified: 2017-08-22 19:40 UTC (History)
6 users (show)

See Also:
Crash report or crash signature: ["XPropertyList::Count()"]


Attachments
bt with debug symbols (11.41 KB, text/plain)
2017-08-02 20:50 UTC, Julien Nabet
Details
bt with debug symbols + gdb session (17.19 KB, text/plain)
2017-08-13 21:57 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Telesto 2017-07-28 20:38:59 UTC
This bug was filed from the crash reporting server and is br-bd5f5f90-c950-4ab2-85c2-2b24159e116c.
=========================================

1. Open Impress
2. Cancel Template selection or choose a template
3. Styles and Formatting Sidebar (F11)
4. "Presentation Styles" button at the top of the Deck
5. Right Click "Background" -> Modify
6. In the Area Tab -> Pattern Button -> Crash
Comment 1 Xisco Faulí 2017-07-31 22:23:06 UTC
Confirmed in

Versión: 5.3.5.1
Id. de compilación: 020db1aa8142e57290f8a21e4df31185392d0e38
Subproc. CPU: 1; SO: Windows 6.1; Repres. IU: predet.; Motor de trazado: HarfBuzz; 
Configuración regional: es-ES (es_ES); Calc: group
Comment 2 Xisco Faulí 2017-07-31 22:34:59 UTC
Regression introduced by:

author	Rishabh Kumar <kris.kr296@gmail.com>	2016-08-22 11:18:05 (GMT)
committer	Katarina Behrens <Katarina.Behrens@cib.de>	2016-10-12 08:36:13 (GMT)
commit	686349476e03f951f4a9ff9755b9f71951b64ea5 (patch)
tree	7cb559cca9cf88dc0a2e2a957244701ab0b4fe60
parent	da01e9ec5dfb7787b4a3669486b3940590933850 (diff)
[GSoC] Move all fill style tabs inside area tab

Bisected with bibisect-linux-64-5.3

Adding Cc: to Rishabh Kumar
Comment 3 Julien Nabet 2017-08-02 20:50:36 UTC
Created attachment 135090 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today + enable-dbgutil + gtk3, I had an assert.
I don't know if it's related or may help.
Comment 4 Telesto 2017-08-03 12:06:50 UTC
Also reproducible with Writer:
1. Insert Header
2. Format -> Page -> Header tab
3. Press the More button
4. Area Tab -> Click Pattern button -> Crash

http://crashreport.libreoffice.org/stats/crash_details/c274eb13-fd30-4cc6-b3eb-152e11e2f79e
Comment 5 Julien Nabet 2017-08-05 23:25:18 UTC
Since I also reproduced the crash from comment 4, I gave it a try.

I submitted a patch to review here:
https://gerrit.libreoffice.org/#/c/40798/
Comment 6 Commit Notification 2017-08-06 13:03:11 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

tdf#109863: use SID_PATTERN_LIST after bitmap tab converting

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 7 Julien Nabet 2017-08-06 13:14:54 UTC
backports in review:
- https://gerrit.libreoffice.org/#/c/40805/ for 5.4 branch
- https://gerrit.libreoffice.org/#/c/40806/ for 5.3 branch
Comment 8 Commit Notification 2017-08-08 10:29:20 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

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

tdf#109863: use SID_PATTERN_LIST after bitmap tab converting

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 9 Commit Notification 2017-08-08 10:30:37 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-5-3":

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

tdf#109863: use SID_PATTERN_LIST after bitmap tab converting

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 10 Xisco Faulí 2017-08-09 07:40:52 UTC
Steps from comment 4 do not crash in

Version: 6.0.0.0.alpha0+
Build ID: 0342c5e8086c8200ecadbe9d52dd4ef6a093effb
CPU threads: 4; OS: Linux 4.10; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group

but the original steps from the description are still crashing.
Moving to NEW...
Comment 11 Xisco Faulí 2017-08-12 21:03:20 UTC
*** Bug 111741 has been marked as a duplicate of this bug. ***
Comment 12 Tamás Zolnai 2017-08-13 14:40:11 UTC
Hi guys,

I still experience the crash on master, updated today:
f70e978f4bf223d7edbb4c3a981f46938a3b3ed1

I used the reproducer steps from the first comment (slide styles).
Comment 13 Julien Nabet 2017-08-13 21:57:18 UTC
Created attachment 135525 [details]
bt with debug symbols + gdb session

On pc Debian x86-64 with master sources updated today, I still reproduce the assert from tdf#111414.
I had to comment 3 parts to retrieve the crash attached.
--- a/svl/source/items/itempool.cxx
+++ b/svl/source/items/itempool.cxx
@@ -813,10 +813,10 @@ const SfxPoolItem& SfxItemPool::GetDefaultItem( sal_uInt16 nWhich ) const
     {
         if ( pImpl->mpSecondary )
             return pImpl->mpSecondary->GetDefaultItem( nWhich );
-        assert(!"unknown which - don't ask me for defaults");
+        //assert(!"unknown which - don't ask me for defaults");
     }
 
-    DBG_ASSERT( pImpl->mpStaticDefaults, "no defaults known - don't ask me for defaults" );
+    //DBG_ASSERT( pImpl->mpStaticDefaults, "no defaults known - don't ask me for defaults" );
     sal_uInt16 nPos = GetIndex_Impl(nWhich);
     SfxPoolItem* pDefault = pImpl->maPoolDefaults[nPos];
     if ( pDefault )
--- a/svl/source/items/poolio.cxx
+++ b/svl/source/items/poolio.cxx
@@ -724,7 +724,7 @@ sal_uInt16 SfxItemPool::GetIndex_Impl(sal_uInt16 nWhich) const
 {
     if (nWhich < pImpl->mnStart || nWhich > pImpl->mnEnd)
     {
-        assert(false && "missing bounds check before use");
+        //assert(false && "missing bounds check before use");
         return 0;
     }
Comment 14 Commit Notification 2017-08-14 05:38:00 UTC
Tamás Zolnai committed a patch related to this issue.
It has been pushed to "master":

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

tdf#109863: Crash while trying to set pattern fill in Impress

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 15 Commit Notification 2017-08-14 11:43:54 UTC
Tamás Zolnai committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

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

tdf#109863: Crash while trying to set pattern fill in Impress

It will be available in 5.4.2.

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 Commit Notification 2017-08-14 11:47:36 UTC
Tamás Zolnai committed a patch related to this issue.
It has been pushed to "libreoffice-5-3":

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

tdf#109863: Crash while trying to set pattern fill in Impress

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 17 Julien Nabet 2017-08-14 16:48:13 UTC
With master sources updated today, even when removing assert and DBG_ASSERT (see https://bugs.documentfoundation.org/show_bug.cgi?id=109863#c13) I still reproduce this.

To not reproduce this, I must comment this line:
472          nSlot = SID_ATTR_BRUSH_CHAR;
(see https://opengrok.libreoffice.org/xref/core/cui/source/tabpages/backgrnd.cxx#472), I don't know why.

Perhaps something specific in my env?
Comment 18 Tamás Zolnai 2017-08-14 18:04:38 UTC
(In reply to Julien Nabet from comment #17)
> With master sources updated today, even when removing assert and DBG_ASSERT
> (see https://bugs.documentfoundation.org/show_bug.cgi?id=109863#c13) I still
> reproduce this.
> 
> To not reproduce this, I must comment this line:
> 472          nSlot = SID_ATTR_BRUSH_CHAR;
> (see
> https://opengrok.libreoffice.org/xref/core/cui/source/tabpages/backgrnd.
> cxx#472), I don't know why.
> 
> Perhaps something specific in my env?

Hi Julien,

I see now. You are speaking about a different bug. An assert while step 5. What I fixed and this issue is about, the crash after step 6 (see comment 1), when trying to activate pattern tab. Actually I was used a bit different reproducer steps, not clicking on "Background" style, but "Title" style where this assertion did not come up. You already created a different bug for that assertion (tdf#111414), so let's keep this one closed and check why the other one is there.
Comment 19 Tamás Zolnai 2017-08-14 18:11:37 UTC
(In reply to Tamás Zolnai from comment #18)
> (In reply to Julien Nabet from comment #17)
> > With master sources updated today, even when removing assert and DBG_ASSERT
> > (see https://bugs.documentfoundation.org/show_bug.cgi?id=109863#c13) I still
> > reproduce this.
> > 
> > To not reproduce this, I must comment this line:
> > 472          nSlot = SID_ATTR_BRUSH_CHAR;
> > (see
> > https://opengrok.libreoffice.org/xref/core/cui/source/tabpages/backgrnd.
> > cxx#472), I don't know why.
> > 
> > Perhaps something specific in my env?
> 
> Hi Julien,
> 
> I see now. You are speaking about a different bug. An assert while step 5.
> What I fixed and this issue is about, the crash after step 6 (see comment
> 1), when trying to activate pattern tab. Actually I was used a bit different
> reproducer steps, not clicking on "Background" style, but "Title" style
> where this assertion did not come up. You already created a different bug
> for that assertion (tdf#111414), so let's keep this one closed and check why
> the other one is there.

I checked that whether the assertion is there in libreoffice-5-3 or libreoffice-5-4 branches and can't reproduce it, so it's a master branch only bug.
Comment 20 Julien Nabet 2017-08-14 18:20:17 UTC
(In reply to Tamás Zolnai from comment #18)
> ...
> I see now. You are speaking about a different bug. An assert while step 5.
> What I fixed and this issue is about, the crash after step 6 (see comment
> 1), when trying to activate pattern tab. Actually I was used a bit different
> reproducer steps, not clicking on "Background" style, but "Title" style
> where this assertion did not come up. You already created a different bug
> for that assertion (tdf#111414), so let's keep this one closed and check why
> the other one is there.

As I said, for the test, I disabled some lines (there are 3 locations) to avoid the assert and I still reproduce the crash with step by step from Telesto in initial description.
Comment 21 Tamás Zolnai 2017-08-14 18:25:30 UTC
(In reply to Julien Nabet from comment #20)
> (In reply to Tamás Zolnai from comment #18)
> > ...
> > I see now. You are speaking about a different bug. An assert while step 5.
> > What I fixed and this issue is about, the crash after step 6 (see comment
> > 1), when trying to activate pattern tab. Actually I was used a bit different
> > reproducer steps, not clicking on "Background" style, but "Title" style
> > where this assertion did not come up. You already created a different bug
> > for that assertion (tdf#111414), so let's keep this one closed and check why
> > the other one is there.
> 
> As I said, for the test, I disabled some lines (there are 3 locations) to
> avoid the assert and I still reproduce the crash with step by step from
> Telesto in initial description.

Ok, then. Actually I have no problem if you reopen this bug and work on the issue. I just said that an assert and a crash is a different thing (assert does not stop the program in a release build). Also I was able to reproduce the crash while clicking on pattern tab also on libreoffice-5-3 and libreoffice-5-4 branches, but the assertion comes up only on master for me.
Comment 22 Tamás Zolnai 2017-08-14 18:29:19 UTC
(In reply to Tamás Zolnai from comment #21)
> (In reply to Julien Nabet from comment #20)
> > (In reply to Tamás Zolnai from comment #18)
> > > ...
> > > I see now. You are speaking about a different bug. An assert while step 5.
> > > What I fixed and this issue is about, the crash after step 6 (see comment
> > > 1), when trying to activate pattern tab. Actually I was used a bit different
> > > reproducer steps, not clicking on "Background" style, but "Title" style
> > > where this assertion did not come up. You already created a different bug
> > > for that assertion (tdf#111414), so let's keep this one closed and check why
> > > the other one is there.
> > 
> > As I said, for the test, I disabled some lines (there are 3 locations) to
> > avoid the assert and I still reproduce the crash with step by step from
> > Telesto in initial description.
> 
> Ok, then. Actually I have no problem if you reopen this bug and work on the
> issue. I just said that an assert and a crash is a different thing (assert
> does not stop the program in a release build). Also I was able to reproduce
> the crash while clicking on pattern tab also on libreoffice-5-3 and
> libreoffice-5-4 branches, but the assertion comes up only on master for me.

Ah, sorry. I misunderstood your words. OK, see now, but it still a master branch only thing. On libreoffice-5-3 and libreoffice-5-4 the assert is not there and also the program does not crash.
Comment 23 Tamás Zolnai 2017-08-14 18:31:30 UTC
The issue is still on master with an assertion.
Comment 24 Tamás Zolnai 2017-08-14 18:42:25 UTC
The issue on master fixed by reverting this commit:

Rohan Kumar <rohankanojia420@gmail.com>	2017-04-08 22:13:38 (GMT)
Katarina Behrens <Katarina.Behrens@cib.de>	2017-05-30 17:08:00 (GMT)
commit	4c5ce12608526e76d90a400fa3f499ab83528e90 (patch)
tree	d4fd9f993e3f37732872cfeda3a389a834a3641b
parent	bffb5437b48327a0572d905c792bbcc5f1f25f9a (diff)
Fix highlighting in sd::FuTemplate
Comment 25 Commit Notification 2017-08-14 21:50:08 UTC
Tamás Zolnai committed a patch related to this issue.
It has been pushed to "master":

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

Related to tdf#109863: Crash when trying to modify background slide style

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 26 Julien Nabet 2017-08-14 21:56:59 UTC
Tamás:
I confirm I don't reproduce tdf#109863 + tdf#111414 !
Now I must recognize I don't understand the logic of all this.

1) why adding all the pages to remove a  bunch of them when bBackground
Couldn't we add this bunch only if not bBackground instead of adding them then removing them?

2) What's the role of RID_SVXPAGE_BACKGROUND ?

Katarina: thought you might be interested in this one.
Comment 27 Tamás Zolnai 2017-08-14 22:08:03 UTC
(In reply to Julien Nabet from comment #26)
> Tamás:
> I confirm I don't reproduce tdf#109863 + tdf#111414 !
> Now I must recognize I don't understand the logic of all this.
> 
> 1) why adding all the pages to remove a  bunch of them when bBackground
> Couldn't we add this bunch only if not bBackground instead of adding them
> then removing them?
> 
> 2) What's the role of RID_SVXPAGE_BACKGROUND ?
> 
> Katarina: thought you might be interested in this one.

1) -> Yes, I also have this in my mind, but I did not bother with removing this stuff, but using the existing logic. I already saw the same logic at other places too. I don't know, but seems a good idea to change it.

2) -> RID_SVXPAGE_BACKGROUND means character background tab page. It was added on libreoffice-5-3 (tdf#96681), to allow to specify character background for styles. Since character background (also called as highlighting) is a character property and slide background has no text content, it useless to have that tabpage (I guess other tab pages are removed for the same reason). I suspect that the crash was because of that the corresponding itemset did not contains character property items.
Comment 28 Julien Nabet 2017-08-15 11:31:44 UTC
(In reply to Tamás Zolnai from comment #27)
> ...
> 1) -> Yes, I also have this in my mind, but I did not bother with removing
> this stuff, but using the existing logic. I already saw the same logic at
> other places too. I don't know, but seems a good idea to change it.
I understand, I would have done the same.

> 
> 2) -> RID_SVXPAGE_BACKGROUND means character background tab page.
> ...
Thank you for the explanation and again for this for the fix! :-)
Comment 29 Commit Notification 2017-08-22 19:40:56 UTC
Tamás Zolnai committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

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

Related to tdf#109863: Crash when close slide style dialog with OK

It will be available in 5.4.2.

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.