Bug Hunting Session
Bug 93600 - Condition Formatting: Entering "=" in comparison value crashes program
Summary: Condition Formatting: Entering "=" in comparison value crashes program
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
5.0.0.5 release
Hardware: All All
: highest major
Assignee: Katarina Behrens (CIB)
URL:
Whiteboard: target:5.1.0 target:5.0.2
Keywords: bibisected, bisected, haveBacktrace, regression
Depends on:
Blocks:
 
Reported: 2015-08-23 16:08 UTC by Mike Bell
Modified: 2016-10-25 19:19 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
bt with debug symbols (10.20 KB, text/plain)
2015-08-29 12:50 UTC, Julien Nabet
Details
Example of conditional formatting with "=" (9.17 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2015-08-31 01:26 UTC, Luke
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Bell 2015-08-23 16:08:38 UTC
Steps to reproduce:
1. Select a cell with a formula or value in.
2. Select menu Format / Conditional Formatting / Condition...
3. Under Condition 1, "Cell value is", "equal to" type "=" into the empty text field.
4. Program crashes immediately and initiates document recovery.

(Was expecting to be able to enter an expression at this point)
Comment 1 Joel Madero 2015-08-23 19:38:47 UTC
Confirmed

Ubuntu 15.04 x64
LibreOffice 5.0.0.5
LibreOffice 4.4 (works fine)

Marking as:
NEW:
MAJOR: Crash
Highest (regression)
Comment 2 Katarina Behrens (CIB) 2015-08-24 16:49:48 UTC
Interesting ...


#0  0x00007fffbe7e96c4 in formula::FormulaToken::GetType() const () from /build/libo-work/instdir/program/../program/libsclo.so
#1  0x00007fffbee91b33 in ScConditionFrmtEntry::OnEdChanged(Edit*) () from /build/libo-work/instdir/program/../program/libsclo.so
#2  0x00007fffbee91945 in ScConditionFrmtEntry::LinkStubOnEdChanged(void*, void*) () from /build/libo-work/instdir/program/../program/libsclo.so
#3  0x00007ffff18fd39d in Link<void*, long>::Call (this=0x195ce68, data=0x195cb80) at /build/libo-work/include/tools/link.hxx:127
#4  0x00007ffff1aa786d in Control::ImplCallEventListenersAndHandler (this=0x195cb80, nEvent=1110, rHandler=..., pCaller=0x195cb80) at /build/libo-work/vcl/source/control/ctrl.cxx:331
#5  0x00007ffff1ab40d3 in Edit::Modify (this=0x195cb80) at /build/libo-work/vcl/source/control/edit.cxx:2414
#6  0x00007fffc419bc70 in formula::RefEdit::Modify() () from /build/libo-work/instdir/program/../program/libforuilo.so
#7  0x00007ffff1aaa941 in Edit::ImplModified (this=0x195cb80) at /build/libo-work/vcl/source/control/edit.cxx:406
#8  0x00007ffff1ab0e48 in Edit::ImplHandleKeyEvent (this=0x195cb80, rKEvt=...) at /build/libo-work/vcl/source/control/edit.cxx:1768
#9  0x00007ffff1ab0fcb in Edit::KeyInput (this=0x195cb80, rKEvt=...) at /build/libo-work/vcl/source/control/edit.cxx:1778
#10 0x00007fffc419bd52 in formula::RefEdit::KeyInput(KeyEvent const&) () from /build/libo-work/instdir/program/../program/libforuilo.so
#11 0x00007ffff1a6cda5 in ImplHandleKey (pWindow=0x1978020, nSVEvent=MouseNotifyEvent::KEYINPUT, nKeyCode=1295, nCharCode=61, nRepeat=0, bForward=true)
    at /build/libo-work/vcl/source/window/winproc.cxx:1027
#12 0x00007ffff1a715cd in ImplWindowFrameProc (_pWindow=0x1978020, nEvent=5, pEvent=0x7fffffffcc20) at /build/libo-work/vcl/source/window/winproc.cxx:2458
#13 0x00007fffd9572c7b in SalFrame::CallCallback (this=0x1977a50, nEvent=5, pEvent=0x7fffffffcc20) at /build/libo-work/vcl/inc/salframe.hxx:246
#14 0x00007fffd95f203a in X11SalFrame::HandleKeyEvent (this=0x1977a50, pEvent=0x7fffffffcf70) at /build/libo-work/vcl/unx/generic/window/salframe.cxx:3427
Comment 3 Michael Weghorn 2015-08-24 20:11:38 UTC
(bi)bisect result (using the bibisect-50max repository):

d3abdc0c75a13c4478cc7243d52bda960e4b8cf8 is the first bad commit
commit d3abdc0c75a13c4478cc7243d52bda960e4b8cf8
Author: Matthew Francis <mjay.francis@gmail.com>
Date:   Wed May 27 20:46:24 2015 +0800

    source-hash-fd28dea50930797652afbdce6992bea08c56caa0
    
    Bibisect: This commit covers the following irrelevant source commit(s)
    78ad5ecd988270f3308fe98cc128ddf832c0c00b
    
    commit fd28dea50930797652afbdce6992bea08c56caa0
    Author:     Laszlo Kis-Adam <dfighter1985@gmail.com>
    AuthorDate: Fri Mar 20 02:14:38 2015 +0100
    Commit:     Markus Mohrhard <markus.mohrhard@googlemail.com>
    CommitDate: Sat Mar 28 19:55:30 2015 +0100
    
        tdf#42897 Warn the user if string without quote is entered as condition value.
    
        Change-Id: I5b30b608c0192b434ff237513ed7fbbf5af43f11

:040000 040000 33fb626471211d271e23f2a6547b2f7fef877204 fa7e949b617999a98a0e9dc4f549e76e1de5f619 M	opt


$ git bisect log
# bad: [dda106fd616b7c0b8dc2370f6f1184501b01a49e] source-hash-0db96caf0fcce09b87621c11b584a6d81cc7df86
# good: [5b9dd620df316345477f0b6e6c9ed8ada7b6c091] source-hash-2851ce5afd0f37764cbbc2c2a9a63c7adc844311
git bisect start 'latest' 'oldest'
# good: [0c30a2c797b249d0cd804cb71554946e2276b557] source-hash-45aaec8206182c16025cbcb20651ddbdf558b95d
git bisect good 0c30a2c797b249d0cd804cb71554946e2276b557
# bad: [2ce02b2ce56f12b9fcb9efbd380596975a3a5686] source-hash-17d714eef491bda2512ba8012e5b3067ca19a5be
git bisect bad 2ce02b2ce56f12b9fcb9efbd380596975a3a5686
# good: [e4deb8a42948865b7b23d447c1547033cb54535b] source-hash-ce46c98dbeb3364684843daa5b269c74fce2af64
git bisect good e4deb8a42948865b7b23d447c1547033cb54535b
# bad: [30a39c6a9e3c59d493447b25aaeb1f70f194bbd7] source-hash-be44ec8c28ce2af9644fcc58317dc1c9b20e2a21
git bisect bad 30a39c6a9e3c59d493447b25aaeb1f70f194bbd7
# bad: [283b2ccc2ba2f6257fa67d1dd9b035e7dd9d6d1d] source-hash-3f97e6e03ac5180bfdbdaa768480bc6bc3ee5664
git bisect bad 283b2ccc2ba2f6257fa67d1dd9b035e7dd9d6d1d
# good: [1d8e9728fd50ef6d1c6e9f3af5fb9cbe95ca5ff4] source-hash-c6282e9bc42dcd1f85005db94416fcaf4caa50c1
git bisect good 1d8e9728fd50ef6d1c6e9f3af5fb9cbe95ca5ff4
# good: [3d2b8ca6cd93e2236660efac8112f34835231e20] source-hash-9b88d532375a6d89c241053c40581d9f963fa1b0
git bisect good 3d2b8ca6cd93e2236660efac8112f34835231e20
# bad: [15621dc13992c6b2abcd4c72ce793dc7bcce42db] source-hash-fed82904e2af94fd264d4f54c9144e8b2ad3f234
git bisect bad 15621dc13992c6b2abcd4c72ce793dc7bcce42db
# bad: [d78143f97abb400e5afd115b854b64c8b8139d96] source-hash-c79ea206b8b2a78488aa20e8b440806078c37537
git bisect bad d78143f97abb400e5afd115b854b64c8b8139d96
# bad: [d3abdc0c75a13c4478cc7243d52bda960e4b8cf8] source-hash-fd28dea50930797652afbdce6992bea08c56caa0
git bisect bad d3abdc0c75a13c4478cc7243d52bda960e4b8cf8
# good: [b2287c7ee44e7a580254f8168888c72e9640731c] source-hash-44b9b27b1ad161191d254f5497e7eb0766854966
git bisect good b2287c7ee44e7a580254f8168888c72e9640731c
# good: [a4088b97559bd44f135c8ae44e46d4de0bd08cfa] source-hash-51a705c439ef2074120f276cce641d8e21768e8f
git bisect good a4088b97559bd44f135c8ae44e46d4de0bd08cfa
# good: [412ff3acd5b7503d00d0398d8b87c162f6989cba] source-hash-c1434d6826aec313854072c9614a2d281762f5e1
git bisect good 412ff3acd5b7503d00d0398d8b87c162f6989cba
# first bad commit: [d3abdc0c75a13c4478cc7243d52bda960e4b8cf8] source-hash-fd28dea50930797652afbdce6992bea08c56caa0
Comment 4 Michael Weghorn 2015-08-24 20:12:37 UTC
@Laszlo: Could you possibly have a look at this?
Comment 5 Julien Nabet 2015-08-29 12:50:39 UTC
Created attachment 118256 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today, I could reproduce this.
Comment 6 Julien Nabet 2015-08-29 13:00:55 UTC
Some gdb session:
Breakpoint 1, ScConditionFrmtEntry::OnEdChanged (this=0x736faa0, pEdit=0x73994a0)
    at /home/julien/compile-libreoffice/libreoffice/sc/source/ui/condformat/condformatdlgentry.cxx:311
311	    boost::scoped_ptr<ScTokenArray> ta(aComp.CompileString(aFormula));
(gdb) n
314	    if( ta->GetCodeError() )
(gdb) p ta
$1 = 
  boost::scoped_ptr {<formula::FormulaTokenArray> = {_vptr.FormulaTokenArray = 0x2aaacfe2b7d0 <vtable for ScTokenArray+16>, pCode = 0x0, pRPN = 0x0, nLen = 0, nRPN = 0, nIndex = 0, nError = 0, nMode = NORMAL, bHyperLink = false, mbFromRangeName = false}, mnHashValue = 1, meVectorState = FormulaVectorEnabled}
(gdb) p aFormula
$2 = "="
Comment 7 Julien Nabet 2015-08-29 13:14:37 UTC
Laszlo: noticing http://cgit.freedesktop.org/libreoffice/core/commit/?id=fd28dea50930797652afbdce6992bea08c56caa0, I thought you might be interested in this one. 

With this naive patch, I got no crash:
diff --git a/sc/source/ui/condformat/condformatdlgentry.cxx b/sc/source/ui/condformat/condformatdlgentry.cxx
index 7785c0d..148f076 100644
--- a/sc/source/ui/condformat/condformatdlgentry.cxx
+++ b/sc/source/ui/condformat/condformatdlgentry.cxx
@@ -320,6 +320,10 @@ IMPL_LINK(ScConditionFrmtEntry, OnEdChanged, Edit*, pEdit)
 
     // Recognized col/row name or string token, warn the user
     formula::FormulaToken* token = ta->First();
+
+    if (!token)
+        return 0;
+
     formula::StackVar t = token->GetType();
     OpCode op = token->GetOpCode();
     if( ( op == ocColRowName ) ||

Is it ok or am I just hiding the problem?
Comment 8 Katarina Behrens (CIB) 2015-08-29 15:25:14 UTC
Slightly different approach: https://gerrit.libreoffice.org/18126 but maybe also totally off ... depends on whether '=' can be matched as cell value or it's essentially and error

Julien: maybe combine our two patches, two wrongs can cancel each other :D and make it right

OP: To match formula cells, 'formula is' (instead of 'cell value is') item is much better choice
Comment 9 Julien Nabet 2015-08-29 16:21:23 UTC
(In reply to Katarina Behrens (CIB) from comment #8)
> Slightly different approach: https://gerrit.libreoffice.org/18126 but maybe
> also totally off ... depends on whether '=' can be matched as cell value or
> it's essentially and error
> 
> Julien: maybe combine our two patches, two wrongs can cancel each other :D
> and make it right
> 
> OP: To match formula cells, 'formula is' (instead of 'cell value is') item
> is much better choice
Sorry I hadn't seen you had submitted a patch to review. I don't know what should be done here. Perhaps we should ask advice to Calc experts?
Comment 10 Commit Notification 2015-08-30 20:49:52 UTC
Katarina Behrens committed a patch related to this issue.
It has been pushed to "master":

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

tdf#93600: Avoid crash on entering '=' as a cell value

It will be available in 5.1.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 11 Luke 2015-08-31 01:26:29 UTC
Created attachment 118282 [details]
Example of conditional formatting with "="

The logical result should be to match the character "=". This is how other spread sheets handle it. In this example, "na" is black and "=" is blue. This is how Google Sheets, Excel, and WPS sheets all handle it. Until this bug is fixed, you can't actually create the attached file in Calc.
Comment 12 Katarina Behrens (CIB) 2015-08-31 08:23:48 UTC
> The logical result should be to match the character "=". This is how other
> spread sheets handle it. In this example, "na" is black and "=" is blue.
> This is how Google Sheets, Excel, and WPS sheets all handle it. Until this
> bug is fixed, you can't actually create the attached file in Calc

Calc does match "=" character if you quote it (just like any other character or string, otherwise they're considered to be range/sheet names)

Anyway, that's orthogonal to this issue, can you please file a different bug for that?
Comment 13 Commit Notification 2015-09-01 22:13:43 UTC
Katarina Behrens committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

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

tdf#93600: Avoid crash on entering '=' as a cell value

It will be available in 5.0.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 14 Julien Nabet 2015-09-04 22:27:51 UTC
Fixed since it's ok on 5.1.0 and 5.0.2 (+ 5.0.2 is the very next released version :-))
Comment 15 Robinson Tryon (qubit) 2015-12-17 10:31:32 UTC Comment hidden (obsolete)