Issue 118851 - [From Symphony] Calc crashed if paste unsupport formula from MS Excel
[From Symphony] Calc crashed if paste unsupport formula from MS Excel
Status: CLOSED FIXED
Product: Calc
Classification: Application
Component: ui
OOo 3.4 Beta
PC Windows 7
: P2 critical (vote)
: 4.0.0
Assigned To: zhang jianfang
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-30 04:44 UTC by Yan Ji
Modified: 2012-10-09 09:17 UTC (History)
5 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation on: ---
Developer Difficulty: ---


Attachments
sample file (25.00 KB, application/vnd.ms-excel)
2012-01-30 04:46 UTC, Yan Ji
no flags Details
a patch for bug 118851 (973 bytes, patch)
2012-05-25 03:43 UTC, ChaoHuang
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Yan Ji 2012-01-30 04:44:37 UTC
1. Open attached sample file
2. Copy cell F4 and F5 which contain formula "PMT" and "Table.
3. New a spreadsheet and paste

Defect: Calc crashed.
Comment 1 Yan Ji 2012-01-30 04:46:41 UTC
Created attachment 77176 [details]
sample file
Comment 2 Yan Ji 2012-02-07 02:14:17 UTC
The problem can be reproduced.
Comment 3 mayongl 2012-02-13 01:04:09 UTC
This is caused by uninitialized call of pCode pointer in SCDocument::InsertTableOp()
Comment 4 mayongl 2012-02-13 01:07:08 UTC
Reopened this for fix.
Comment 5 ChaoHuang 2012-05-24 06:20:42 UTC
(In reply to comment #3)
> This is caused by uninitialized call of pCode pointer in
> SCDocument::InsertTableOp()

In file "main\sc\source\core\data\cell.cxx", there is a function "ScFormulaCell::Compile"

void ScFormulaCell::Compile( const String& rFormula, sal_Bool bNoListening,
                            const FormulaGrammar::Grammar eGrammar )
{
	if ( pDocument->IsClipOrUndo() ) return;
	sal_Bool bWasInFormulaTree = pDocument->IsInFormulaTree( this );
	if ( bWasInFormulaTree )
		pDocument->RemoveFromFormulaTree( this );
	// pCode darf fuer Abfragen noch nicht geloescht, muss aber leer sein
	if ( pCode )
		pCode->Clear();
	ScTokenArray* pCodeOld = pCode;
	ScCompiler aComp( pDocument, aPos);
        aComp.SetGrammar(eGrammar);
	pCode = aComp.CompileString( rFormula );
	if ( pCodeOld )
		delete pCodeOld;
	if( !pCode->GetCodeError() )
	{
		if ( !pCode->GetLen() && aResult.GetHybridFormula().Len() && rFormula == aResult.GetHybridFormula() )
		{	// #65994# nicht rekursiv CompileTokenArray/Compile/CompileTokenArray
			if ( rFormula.GetChar(0) == '=' )
				pCode->AddBad( rFormula.GetBuffer() + 1 );
			else
				pCode->AddBad( rFormula.GetBuffer() );
		}
		bCompile = sal_True;
		CompileTokenArray( bNoListening );
	}
	else
	{
		bChanged = sal_True;
		SetTextWidth( TEXTWIDTH_DIRTY );
		SetScriptType( SC_SCRIPTTYPE_UNKNOWN );
	}
	if ( bWasInFormulaTree )
		pDocument->PutInFormulaTree( this );
}

The construction of "pCode" will be ignored by condition "if ( pDocument->IsClipOrUndo() )" at the first line in function "ScFormulaCell::Compile". In other places, there is no judgement for "pCode" before using it. 

Can we remove the returning condition "if ( pDocument->IsClipOrUndo() )" ?
Thanks!


(In reply to comment #3)
> This is caused by uninitialized call of pCode pointer in
> SCDocument::InsertTableOp()
Comment 6 ChaoHuang 2012-05-25 02:42:32 UTC
////// Key step to reproduce the defect
1. Open the sample file in MS excel on Windows
2. Copy cell F5 on the sheet
3. Open Aoo3.4, select a cell, paste from clipboard

Aoo3.4 will crash after execute these 3 steps.
Comment 7 ChaoHuang 2012-05-25 03:20:13 UTC
////// Root cause
There is a data table in cell F5 on the first sheet. A ScFormulaCell will be created when do pasting from MS excel to Aoo3.4 spreatsheet.
Please refer to call stacks below

 	sc.dll!ScFormulaCell::ScFormulaCell(ScDocument * pDoc=0x0960fb98, const ScAddress & rPos={...}, const String & rFormula={...}, formula::FormulaGrammar::Grammar eGrammar=GRAM_NATIVE, unsigned char cMatInd=0)  Line 678	C++
>	sc.dll!ScDocument::InsertTableOp(const ScTabOpParam & rParam={...}, short nCol1=5, long nRow1=4, short nCol2=5, long nRow2=8, const ScMarkData & rMark={...})  Line 288	C++
 	scfilt.dll!ImportExcel::TableOp()  Line 1107	C++
 	scfilt.dll!ImportExcel8::Read()  Line 1145 + 0xb bytes	C++
 	scfilt.dll!ScFormatFilterPluginImpl::ScImportExcel(SfxMedium & rMedium={...}, ScDocument * pDocument=0x0960fb98, EXCIMPFORMAT eFormat=EIF_AUTO)  Line 208 + 0x28 bytes	C++
 	sc.dll!ScViewFunc::PasteDataFormat(unsigned long nFormatId=121, const com::sun::star::uno::Reference<com::sun::star::datatransfer::XTransferable> & rxTransferable={...}, short nPosX=4, long nPosY=3, Point * pLogicPos=0x00000000, unsigned char bLink=0, unsigned char bAllowDialogs='')  Line 543 + 0x2e bytes	C++
 	sc.dll!ScViewFunc::PasteFromSystem(unsigned long nFormatId=121, unsigned char bApi=0)  Line 811 + 0x50 bytes	C++
 	sc.dll!ScViewFunc::PasteFromSystem()  Line 651 + 0x11 bytes	C++
 	sc.dll!ScCellShell::PasteFromClipboard(ScViewData * pViewData=0x053563c8, ScTabViewShell * pTabViewShell=0x05356378, bool bShowDialog=true)  Line 2188	C++
 	sc.dll!ScCellShell::ExecuteEdit(SfxRequest & rReq={...})  Line 1179 + 0x21 bytes	C++
 	
The construction for "ScFormulaCell::ScFormulaCell" on line 651 in file "main/sc/source/core/data/cell.cxx" will call function "ScFormulaCell::Compile( rFormula, sal_True, eGrammar )".
Please refer to code snippet below.

void ScFormulaCell::Compile( const String& rFormula, sal_Bool bNoListening,
                            const FormulaGrammar::Grammar eGrammar )
{
	if ( pDocument->IsClipOrUndo() ) return;
	sal_Bool bWasInFormulaTree = pDocument->IsInFormulaTree( this );
	if ( bWasInFormulaTree )
		pDocument->RemoveFromFormulaTree( this );
	// pCode darf fuer Abfragen noch nicht geloescht, muss aber leer sein
	if ( pCode )
		pCode->Clear();
	ScTokenArray* pCodeOld = pCode;
	ScCompiler aComp( pDocument, aPos);
    aComp.SetGrammar(eGrammar);
	pCode = aComp.CompileString( rFormula );
	if ( pCodeOld )
		delete pCodeOld;
	if( !pCode->GetCodeError() )
	{
		if ( !pCode->GetLen() && aResult.GetHybridFormula().Len() && rFormula == aResult.GetHybridFormula() )
		{	// #65994# nicht rekursiv CompileTokenArray/Compile/CompileTokenArray
			if ( rFormula.GetChar(0) == '=' )
				pCode->AddBad( rFormula.GetBuffer() + 1 );
			else
				pCode->AddBad( rFormula.GetBuffer() );
		}
		bCompile = sal_True;
		CompileTokenArray( bNoListening );
	}
	else
	{
		bChanged = sal_True;
		SetTextWidth( TEXTWIDTH_DIRTY );
		SetScriptType( SC_SCRIPTTYPE_UNKNOWN );
	}
	if ( bWasInFormulaTree )
		pDocument->PutInFormulaTree( this );
}

The initialization for pCode will be ignored by condition " if ( pDocument->IsClipOrUndo() ) ". After that, any operation on pCode will cause to "bad memory access", which is usually a crash on Windows.


////// Solution
The initialization for pCode should not be ignored by condition " if ( pDocument->IsClipOrUndo() ) " in function " ScFormulaCell::Compile ". The condition can be changed to " if ( pDocument->IsClipOrUndo() && (pCode != NULL) ) ".

The clone for pCode in function "ScFormulaCell::ScFormulaCell" on line 750 in file "main/sc/source/core/data/cell.cxx" also should be improved
from
	pCode = rCell.pCode->Clone();
to
  pCode = (rCell.pCode) ? (rCell.pCode->Clone()) : NULL;
Comment 8 ChaoHuang 2012-05-25 03:43:14 UTC
Created attachment 77591 [details]
a patch for bug 118851
Comment 9 zhang jianfang 2012-06-03 05:48:28 UTC
Take over the bug to commit it's patch.
Comment 10 zhang jianfang 2012-06-03 05:52:39 UTC
committed to trunk in revision 1345624.
Comment 11 Terry Yang 2012-06-18 09:52:03 UTC
verified fixed on trunk 1350879
Comment 12 Shenfeng Liu 2012-10-09 09:16:53 UTC
set Target Milestone to AOO 3.5.0 for PM purpose.