Bug 134576 - A VBA macro crashes LO
Summary: A VBA macro crashes LO
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.1.0 target:7.0.0.2
Keywords:
Depends on:
Blocks:
 
Reported: 2020-07-06 17:17 UTC by Mike Kaganski
Modified: 2023-02-19 09:41 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
The VBA macro in the ODT crashes LO (9.19 KB, application/vnd.oasis.opendocument.text)
2020-07-06 17:17 UTC, Mike Kaganski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2020-07-06 17:17:50 UTC
Created attachment 162725 [details]
The VBA macro in the ODT crashes LO

Opening the ODT and running the URLPictureInsert macro in it crashes LO.

Ref: https://ask.libreoffice.org/en/question/253778/

The macro is for Calc, and I put it into a Writer document on purpose: for me, it doesn't crash when put into am ODS' library, but crashes if put into common library, or into ODT.

There must be an error, but *no crash*.

Tested with Version: 7.0.0.1 (x64)
Build ID: 04ba7e3f1e51af6c5d653e543a620e36719083fd
CPU threads: 12; OS: Windows 10.0 Build 18363; UI render: Skia/Raster; VCL: win
Locale: ru-RU (ru_RU); UI: en-US
Calc: CL
Comment 1 Xisco Faulí 2020-07-09 19:05:21 UTC
Reproduced in

Version: 7.1.0.0.alpha0+
Build ID: 0695b5f5b328a1bc9cfdc8ba020dbdd674185e04
CPU threads: 4; OS: Linux 4.19; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

and

Version: 5.2.0.0.alpha0+
Build ID: 3ca42d8d51174010d5e8a32b96e9b4c0b3730a53
Threads 4; Ver: 4.19; Render: default; 

Locale: en-US (en_US.UTF-8)
Comment 2 Xisco Faulí 2020-07-09 19:15:10 UTC
Regression introduced by:

https://cgit.freedesktop.org/libreoffice/core/commit/?id=3848819cac3b951e60b42332ad87cd465b20d926

author	Caolán McNamara <caolanm@redhat.com>	2014-01-27 13:13:56 +0000
committer	Caolán McNamara <caolanm@redhat.com>	2014-01-27 14:38:38 +0000
commit 3848819cac3b951e60b42332ad87cd465b20d926 (patch)
tree f10b98855bdab4d4790e7b0f1c49f58f96fe62d4
parent fd7dbe5a15c3393ea9ad7c26267056743099c506 (diff)
coverity#707652 Uninitialized scalar field

Bisected with: bibisect-43max

Adding Cc: to Caolán McNamara
Comment 3 Caolán McNamara 2020-07-10 09:31:34 UTC
for me, its the line

Range("A2").Select

which causes the crash. I can't make sense of the bisect though.
Comment 4 Mike Kaganski 2020-07-10 09:54:58 UTC
(In reply to Caolán McNamara from comment #3)
> I can't make sense of the bisect though.

Well, it makes perfect sense :-) after the patch, the dereference of the initialized nullptr crashes the program immediately; previously, the program that dereferenced some garbage crashed only on close ;-P
Comment 5 Mike Kaganski 2020-07-10 09:59:44 UTC
The problem (in a different form, caused by the change mentioned in comment 2, which is a good change) is reproducible even in OOo 2.2. Running the macro might not crash the program immediately (or might, depends on the exact garbage that happens there in uninitialized memory), but it will crash on close.

Tested with OpenOffice.org 2.2.0 on Win10 x64.
Comment 6 Caolán McNamara 2020-07-10 10:16:36 UTC
seems that the
On Error Resume Next
is relevant in the sense there isn't an ActiveSheet so the macro fails earlier and reports that and stops running, with "On Error Resume Next" it keeps going on detecting an error in the script
Comment 7 Mike Kaganski 2020-07-10 10:31:16 UTC
I had been able to simplify the crasher to this:

Sub crash
On Error Resume Next
For Each a In b
c("d").e
Next
End Sub

And it doesn't depend on VBA support...
Comment 8 Mike Kaganski 2020-07-10 10:33:01 UTC
Or even

Sub crash
On Error Resume Next
For Each a In b
c.d
Next
End Sub
Comment 9 Caolán McNamara 2020-07-10 10:41:07 UTC
well, we could try
https://gerrit.libreoffice.org/c/core/+/98491
which at least doesn't crash for the initial case, maybe that's "good enough". I don't have any in depth knowledge of basic to know if there's a more systemic solution that should be followed
Comment 10 Commit Notification 2020-07-10 13:01:11 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

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

tdf#134576 at least don't crash in this edge-case

It will be available in 7.1.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 11 Caolán McNamara 2020-07-10 13:17:16 UTC
that at least makes this example not crash, I haven't backported it but not against that if its considered to make things better
Comment 12 Commit Notification 2020-07-10 18:06:37 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5760c94b8847164f9a7a181f031c7c86643944af

tdf#134576: proper handling of For/For Each with On Error Resume Next

It will be available in 7.1.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 13 Commit Notification 2020-07-14 19:55:38 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

https://git.libreoffice.org/core/commit/1ecda3adc91328bc616b96367d90907350f2500f

tdf#134576 at least don't crash in this edge-case

It will be available in 7.0.0.2.

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 14 Commit Notification 2020-07-14 19:55:48 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

https://git.libreoffice.org/core/commit/4ef431e0c9306072b011c782b15eed87b7bdfcb5

tdf#134576: proper handling of For/For Each with On Error Resume Next

It will be available in 7.0.0.2.

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 15 Xisco Faulí 2020-07-20 10:35:06 UTC
Verified in

Version: 7.1.0.0.alpha0+
Build ID: abea0d6647c7f1f7e76c73c26cb80e6a67dc5111
CPU threads: 4; OS: Linux 4.19; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

@Caolán, Mike K, thanks for fixing this issue!!
Comment 16 Commit Notification 2020-07-21 07:41:06 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/23bfaf601d78ab2a80ec5493ef7eab1410975769

tdf#134576: basic_macros: Add unittest

It will be available in 7.1.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.