Bug 163680 - BASIC: 'Dim s As String * n' doesn't keep the string length on assignments
Summary: BASIC: 'Dim s As String * n' doesn't keep the string length on assignments
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Mike Kaganski
URL:
Whiteboard: target:25.2.0 target:24.8.4
Keywords:
Depends on:
Blocks:
 
Reported: 2024-10-30 07:23 UTC by Mike Kaganski
Modified: 2024-11-01 17:53 UTC (History)
0 users

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2024-10-30 07:23:00 UTC
In VBA, there is a "fixed length" strings type [1], declared as

  Dim s As String * 123

which takes length argument up to 2^16, and makes the string length fixed to that value. Assigning a shorter string to it would pad spaces, and assigning a longer string would truncate.

In LibreOffice Basic, there was always a code to support that feature, not even dependent on the VBA support mode: the SbiSymDef::nLen member, the SbiOpcode::PAD_ (used in SbiParser::Assign), the respective SbiRuntime::StepPAD.

However, it has never worked properly. Before OOo 3.3, the declaration like above would create a 0-length string variable (while it must be 123-character from start, filled with spaces). Starting with OOo 3.3 (and LO), the declaration creates the correct string, but following assignments break that invariant:

  s = "abc"
  MsgBox Len(s) ' emits "3", instead if proper "123"

And using Let to assign the variable:

  Let s = "def"

even used to crash the process before version 4.0 (likely, commit 6702bc37d4bc28ec45c6c25f6a953997f6999270 helped to fix the incorrect cast into the variable internals, but at the same time, it broke the *idea* that the StepPAD function is intended to modify the value on stack, that will be assigned to the variable at the next StepPUT - the current code is basically a no-op, modifying a local variable that is never used).

Additionally, even the original idea was broken, because it (hoped to) directly modify the value, which might belong to another variable. *If* it worked, then a code like this:

  Dim s2 As String
  s2 = "gfh"
  MsgBox Len(s2) ' 3
  Let s = s2
  MsgBox Len(s2) ' would be 123 !!!

would suddenly make s2 equal to s, having the length of 123.

The code generator must emit the SbiOpcode::PAD_ also in SbiParser::Symbol (which handles assignments without Let keyword), and the runtime must take care to replace the variable on stack, when changing the length, instead of modifying the existing variable's value there.

[1] https://learn.microsoft.com/en-us/office/vba/language/reference/user-interface-help/string-data-type
Comment 1 Mike Kaganski 2024-10-31 15:56:38 UTC
https://gerrit.libreoffice.org/c/core/+/175878
Comment 2 Commit Notification 2024-10-31 19:46:39 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

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

tdf#163680: fix fixed-length strings assignment

It will be available in 25.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 3 Commit Notification 2024-11-01 17:53:47 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/8e37f8ab3798fbaad92179ad745962c62bb0fd73

tdf#163680: fix fixed-length strings assignment

It will be available in 24.8.4.

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.