[I7-1999] [Mantis 2035] Inform crashes with a stack overflow when appending to indexed text variables.

[curiousdannii-testing]

Reported by : climbingstars

Description :

Inform crashes with a stack overflow when any second command is typed.

Steps to reproduce :

"Test"

The data string is an indexed text that varies.

After reading a command:
if the data string is "" begin;
now the data string is unpunctuated word number 2 in the description of the player;
otherwise;
now the data string is "[data string] [unpunctuated word number 2 in the description of the player]";
end if;
say the data string.

The Testing Room is A Room.

Additional information :

It looks like the issue is with the data string appending line.

imported from: [Mantis 2035] Inform crashes with a stack overflow when appending to indexed text variables.

• status: Closed
• resolution: Resolved
• resolved: 2022-04-10T04:40:34+10:00
• imported: 2022/01/10

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by zarf :
I haven't tested this code, but you're creating a substitution which includes itself. This will crash the VM because it expands infinitely.

I don't remember if there's a recommended way to append strings. You can probably make it work by creating a temporary value which is "the substituted form of ..." and then copying it back to data string.

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by zarf :
I did some debugging to verify that, yes, the VM gets into an infinite loop of expanding the [data string] substitution.

Why doesn't your second example fail? I don't know. There may be some internal detail about how indexed text is handled which makes that case fail to fail in the way you expect. It's still best to avoid it by using a "substituted form" phrase.

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by mattweiner :
From WwI §20.7:

"What's going on here is this: Inform substitutes text immediately if it contains references to a temporary value such as "T", and otherwise only if it needs to access the contents."

In the first example climbingstars posted above, the line

now the data string is "[data string] [unpunctuated word number counter in the description of the player]";

contains a reference to the temporary variable "counter," so I think this means that the text gets substituted immediately, so you don't get the infinite loop.

In the thing you posted just above, you didn't make "temporary" a substituted form, so you've just created a temporary text variable that calls data string as a function. Then you set data string to that function and data string is still calling itself.

Anyhow the way to do this appending is to use the substituted form right away, and you don't need a temporary variable. So you can put this in the original text and it will work:

now the data string is the substituted form of "[data string] [unpunctuated word number 2 in the description of the player]";

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by climbingstars :
But this works perfectly!

"Test"

The data string is an indexed text that varies.

After reading a command:
repeat with counter running from 1 to the number of words in the description of the player begin;
if the data string is "" begin;
now the data string is unpunctuated word number counter in the description of the player;
otherwise;
now the data string is "[data string] [unpunctuated word number counter in the description of the player]";
end if;
end repeat;
say the data string.

The Testing Room is A Room.

Surely this should fail too if that were the case.

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by climbingstars :
On checking the auto.inf file after compiling, I noticed some differences.

The first example generates this.

BlkValueCopy((Global_Vars-->10), (TEXT_TY_ExpandIfPerishable((I7SFRAME+WORDSIZE*2),TX_S_135)));

While the second generates this for the same line where "tmp_0" is the counter variable.

BlkValueCopy((Global_Vars->10), ((LocalParking->0=tmp_0),TEXT_TY_ExpandIfPerishable((I7SFRAME+WORDSIZE*2),TX_S_135)));

Maybe if Inform were to use this second case for the first example, it would work fine. Either way if this is not the best way to append strings, it would be nicer for inform to reject the code rather than crash at run time.

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by climbingstars :
It still fails even with the temporary variable!

"Test"

The data string is an indexed text that varies.

After reading a command:
if the data string is "" begin;
now the data string is unpunctuated word number 2 in the description of the player;
otherwise;
let temporary be "[data string] [unpunctuated word number 2 in the description of the player]";
now the data string is temporary;
end if;
say the data string.

The Testing Room is A Room.

[curiousdannii-testing]

61eedb62875fc10070240916:

Fixed via this commit: https://github.com/ganelson/inform/commit/23670bf671a281ad9b7c33f83766d062fbdc491b

Comment by Graham Nelson:
After some thought, I decided that it was better to leave the compiler as it stands, but to add to the documentation and explain the wisdom of using "substituted form of" in cases like this. In the end, a compiler can never fully defend against a program which explicitly asks for infinite regress.