Bug 128334 - Assert using FormulaR1C1
Summary: Assert using FormulaR1C1
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.4.0.0.alpha1+
Hardware: All All
: medium normal
Assignee: Eike Rathke
URL:
Whiteboard: target:7.2.0 target:7.1.3
Keywords: haveBacktrace
Depends on:
Blocks: Macro-VBA
  Show dependency treegraph
 
Reported: 2019-10-23 10:01 UTC by Mike Kaganski
Modified: 2021-04-07 17:07 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:
Regression By:


Attachments
A spreadsheet with VBA macro using FormulaR1C1 (17.44 KB, application/vnd.oasis.opendocument.spreadsheet)
2019-10-23 10:01 UTC, Mike Kaganski
Details
bt with debug symbols (12.59 KB, text/plain)
2021-04-05 16:04 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2019-10-23 10:01:43 UTC
Created attachment 155250 [details]
A spreadsheet with VBA macro using FormulaR1C1

Running the macro "Макрос5" from VBAProject/Module2 in the attached file in debug master build breaks an assert and crashes:

---------------------------
Microsoft Visual C++ Runtime Library
---------------------------
Debug Error!

Program: C:\lo\src\core\instdir\program\soffice.bin

abort() has been called

(Press Retry to debug the application)

---------------------------
Abort   Retry   Ignore   
---------------------------

Possibly relevant is that I have tested it with a locale which uses semicolon as formula argument separator.

Version: 6.4.0.0.alpha1+ (x64)
Build ID: f1379133177b2c157019913d2fcdb075859adeb6
CPU threads: 12; OS: Windows 10.0 Build 18362; UI render: GL; VCL: win; 
Locale: ru-RU (ru_RU); UI-Language: en-US
Calc: CL
Comment 1 Xisco Faulí 2019-10-23 13:38:57 UTC
Not reproducible in

Version: 6.4.0.0.alpha1+
Build ID: 437dc68285dab0f08a1ded2193d86d64f560cd9b
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded
Comment 2 Oliver Brinzing 2019-10-23 17:04:51 UTC
i can reproduce a crash with:

Version: 6.4.0.0.alpha1+ (x64)
Build ID: 2e81c006b85214d28927ae4e3e18eda5f3be944a
CPU threads: 4; OS: Windows 10.0 Build 18362; UI render: default; VCL: win; 
Locale: de-DE (de_DE); UI-Language: en-US
Calc: threaded

no crash with:

Version: 6.3.2.2 (x64)
Build-ID: 98b30e735bda24bc04ab42594c85f7fd8be07b9c
CPU-Threads: 4; BS: Windows 10.0; UI-Render: Standard; VCL: win; 
Gebietsschema: de-DE (de_DE); UI-Sprache: de-DE
Calc:
Comment 3 Xisco Faulí 2020-09-30 12:52:13 UTC
Not reproducible in

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

@Mike Kaganski, do you still reproduce this issue in a master build ?
Comment 4 Xisco Faulí 2020-11-04 14:48:52 UTC
Not reproduced in

Version: 7.0.3.1 (x86)
Build ID: d7547858d014d4cf69878db179d326fc3483e082
CPU threads: 2; OS: Windows 6.1 Service Pack 1 Build 7601; UI render: Skia/Raster; VCL: win
Locale: es-ES (es_ES); Interfaz: es-ES
Calc: threaded
Comment 5 Mike Kaganski 2020-11-04 14:56:27 UTC Comment hidden (obsolete)
Comment 6 Mike Kaganski 2020-11-04 16:07:44 UTC
Oh sorry! It's still reproducible with Version: 7.1.0.0.alpha1+ (x64)
Build ID: d4892e5452bff155da6c34063ffe8c331284d8e9
CPU threads: 12; OS: Windows 10.0 Build 19042; UI render: Skia/Raster; VCL: win
Locale: ru-RU (ru_RU); UI: ru-RU
Calc: CL

I forgot that the problem is with an assertion - so only shown in debug builds. Testing in comment 5 was using a release build.
Comment 7 Julien Nabet 2021-04-05 16:04:30 UTC
Created attachment 170966 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today + enable-dbgutil I could reproduce this.
Comment 8 Eike Rathke 2021-04-06 14:52:36 UTC
Taking.
Comment 9 Eike Rathke 2021-04-06 18:18:35 UTC
From the backtrace I can see why it fails and can fix that.

But I can't reproduce. I tried also in a ru_RU.UTF-8 locale (not ru-RU UI but en-US UI like in the original submit). As UI function parameter separator I used default ';' semicolon (as ru-RU has ',' comma decimal separator). From the code in ScCompiler::IsNamedRange() I deduce that if rUpperName="," and mnCurrentSheetEndPos=18 (beginIndex=18) it somehow must be following the 'ЗАПАСНЫЕ ЧАСТИ'!RC[-19] but not being recognized as function parameter separator, otherwise we'd not end up in IsNamedRange(). Or as single standalone Russian decimal separator, which actually could be the case here, but not for me. Note that all happens in ScVbaRange::getFormulaR1C1() which obtains the old existing value of Z1

  =IF(ISERROR(SEARCH("ПРОМО",$'запасные части'.g10.1)),Y10)

Note that the VBA wrapper attempts to convert R1C1 to API grammar first and vice versa, which leaves some mess here and I think the Russian comment in the document is about, as when via macro setting

  =IF(ISERROR(SEARCH("ПРОМО",'ЗАПАСНЫЕ ЧАСТИ'!RC[-19],1)),RC[-1],0)

is converted to

  IF(ISERROR(SEARCH("ПРОМО";$'ЗАПАСНЫЕ ЧАСТИ'.G10.1));Y10)

Note the G10.1 that should be G1;1 and Y10 be Y1;0 instead and both smell like a ',' decimal separator converted to '.' (probably due to ru-RU locale vs en-US API locale) as in
RC[-19],1 => RC[-19]0.1 => G10.1
RC[-1],0 => RC[-1]0.0 => RC[-1]0 => Y10

Later $'ЗАПАСНЫЕ ЧАСТИ'. is 18 characters long and if there was a ',' following somewhere (which I can not reproduce) that could explain if resetting mnCurrentSheetEndPos is missing in the flow.

Btw, converting the then already erroneous formula back to R1C1 results in

  =IF(ISERROR(SEARCH("ПРОМО";$'запасные части'.g10.1));R[9]C[-1])

A complete mess in the VBA wrapper regarding separators that should be addressed later.

Mike, how is that VBA FormulaR1C1 supposed to work, using English function names and separators, or localized function names and separators? A mix thereof? If localized separators then the macro would be wrong anyway. And would work only in specific locales. Which I totally would find Excel capable of such nonsense. And which is implemented in the VBA wrapper with FormulaGrammar::GRAM_NATIVE_XL_R1C1
Comment 10 Mike Kaganski 2021-04-06 19:11:12 UTC
(In reply to Eike Rathke from comment #9)
> Mike, how is that VBA FormulaR1C1 supposed to work, using English function
> names and separators, or localized function names and separators? A mix
> thereof? If localized separators then the macro would be wrong anyway. And
> would work only in specific locales. Which I totally would find Excel
> capable of such nonsense. And which is implemented in the VBA wrapper with
> FormulaGrammar::GRAM_NATIVE_XL_R1C1

:-D I am afraid I have no idea how that is expected to work - that is not my code, and I remember I caught the failing assertion when trying to help someone without looking too deep ... so I'm not surprised the code could be "suboptimal" :-D
Comment 11 Julien Nabet 2021-04-06 19:12:59 UTC
Eike: just for information, I use French (France) locale + English (USA) for user interface. Decimal separator put as "Same as locale setting" (In France, decimal separator is ",".)
(enable-dbgutil to have this assertion)

If you need someone to test a patch (to fix or even just to add some logs), don't hesitate! :-)
Comment 12 Eike Rathke 2021-04-06 22:26:38 UTC
(In reply to Mike Kaganski from comment #10)
> :-D I am afraid I have no idea how that is expected to work - that is not my
> code
ActiveCell.FormulaR1C1 is a VBA property to get/set the formula on the currently selected cell, which in the sample is Общий!Z1, and we emulate that.

It works if the macro uses the formula string

  "=IF(ISERROR(SEARCH(""ПРОМО"";'ЗАПАСНЫЕ ЧАСТИ'!RC[-19];1));RC[-1];0)"

i.e. replacing the ',' comma separators with ';' semicolon. But that still won't work in a Russian UI with translated function names.

Digging a bit revealed
https://stackoverflow.com/questions/35724156/different-languages-issue-when-inserting-formula-from-vba
and there are 4 methods/properties

Range.Formula
Range.FormulaR1C1
Range.FormulaLocal
Range.FormulaR1C1Local

of which we have the first two implemented but wrongly as the local last two. Glorious.
Comment 13 Commit Notification 2021-04-06 23:01:43 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

Resolves: tdf#128334 Reset mnCurrentSheetEndPos, mnCurrentSheetTab in all cases

It will be available in 7.2.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 14 Eike Rathke 2021-04-06 23:04:06 UTC
Pending review https://gerrit.libreoffice.org/c/core/+/113647 for 7-1
Comment 15 Commit Notification 2021-04-07 00:23:37 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

Related: tdf#128334 Make VBA Range getFormula(R1C1) work not only by accident

It will be available in 7.2.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 16 Commit Notification 2021-04-07 10:31:48 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

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

Resolves: tdf#128334 Reset mnCurrentSheetEndPos, mnCurrentSheetTab in all cases

It will be available in 7.1.3.

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 17 Eike Rathke 2021-04-07 16:48:12 UTC
I created bug 141543 for the Formula/FormulaR1C1 vs FormulaLocal/FormulaR1C1Local thing.
Comment 18 Julien Nabet 2021-04-07 17:07:39 UTC
With master sources updated today, I don't reproduce the assertion anymore, thank you Eike!