Bug 146742

Summary:	The return value of Basic function is not initialized when called from Calc formula
Product:	LibreOffice	Reporter:	Mike Kaganski <mikekaganski>
Component:	BASIC	Assignee:	Andreas Heinisch <andreas.heinisch>
Status:	RESOLVED FIXED
Severity:	normal	CC:	andreas.heinisch, sokol
Priority:	medium
Version:	Inherited From OOo
Hardware:	All
OS:	All
See Also:	https://bugs.documentfoundation.org/show_bug.cgi?id=143582 https://bugs.documentfoundation.org/show_bug.cgi?id=149622 https://bugs.documentfoundation.org/show_bug.cgi?id=145069
Whiteboard:	target:7.4.0 target:7.3.1
Crash report or crash signature:		Regression By:
Attachments:	A spreadsheet with a macro used as spreadsheet function

Description Mike Kaganski 2022-01-13 15:16:16 UTC

Created attachment 177527 [details]
A spreadsheet with a macro used as spreadsheet function

The attachment contains this macro:

Function Foo(n) As Boolean
  If n = 3 Then Foo = True
End Function

This function is used in cells B1-B9, like "=FOO(A1)"; the cells A1-A9 contain numbers from 1 to 9. The intended behavior is that only B3 should show TRUE, and all other cells in B should be FALSE (because when a function is not explicitly assigned a return value, it should use default value of its return type, which for Boolean is False). However, when applying the function, it only shows FALSE in A1 and A2, and if you later re-calculate those cells, they will become TRUE - as if the first calculated result gets stuck - very similar to bug 143582.

Tested with Version: 7.4.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 840b4eb2f3443ff883016e6a8a8ae49e9cbd9e4e
CPU threads: 12; OS: Windows 10.0 Build 19044; UI render: Skia/Raster; VCL: win
Locale: ru-RU (ru_RU); UI: en-US
Calc: CL

Comment 1 Mike Kaganski 2022-01-13 15:30:32 UTC

Apso repro using OOo 3.2.0

Comment 2 Mike Kaganski 2022-01-13 15:38:22 UTC

Code pointer: BasicManager::ExecuteMacro. But the question arises: isn't it better to move the initialization code to SbMethod::Call maybe, to avoid duplicated code, which is error-prone? Or is there a case where we should not do this? Andreas, what do you think?

Comment 3 Andreas Heinisch 2022-01-13 19:57:48 UTC

Confirmed in:

Version: 7.4.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 423773c0054ac8c70c47606113dc0d92af2ac8e2
CPU threads: 6; OS: Windows 10.0 Build 19042; UI render: Skia/Raster; VCL: win
Locale: de-DE (de_DE); UI: en-US
Calc: CL

Imho, we should move the initialization code to SbMethod::Call. I could not think about a scenario where this could lead to an error. I tested some calls including recursions or remaining values in the function calls.

Comment 4 Commit Notification 2022-02-03 08:44:36 UTC

Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6a0f00d3b9d1a74637c92ec6eff1ba5fedc82f3d

tdf#146742 - Move the initialization code of a method to SbMethod::Call

It will be available in 7.4.0.

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

Affected users are encouraged to test the fix and report feedback.

Comment 5 Commit Notification 2022-02-03 11:22:38 UTC

Andreas Heinisch committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/b8387f64cc722ef501be32e6cb30397aae8af83e

tdf#146742 - Move the initialization code of a method to SbMethod::Call

It will be available in 7.3.1.

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

Affected users are encouraged to test the fix and report feedback.

Comment 6 Vladimir Sokolinskiy 2022-02-03 12:06:49 UTC

Colleagues, thank you very much!