adv3Lite parser stack overflow

Authoring TADS
1

A beta tester on my WIP discovered a stack overflow bug in adv3Lite. My WIP uses adv3Lite 1.5 (I know, I know, I’ll upgrade in my next project), but I’ve reproduced this on the latest GitHub master of adv3Lite (which reports as version 2.1).

Using this code as a test harness:

versionInfo: GameID
;

gameMain: GameMainDef
  initialPlayerChar = me
;

startRoom: Room 'A room'
;

me: Thing
  location = startRoom

  isFixed = true
  person = 2
  contType = Carrier
;

and this as the test code:

Thing 'a penny' @startRoom
;

The transcript looks like this:

>l
A room

You can see a penny here.

>version
version

adv3Lite Library version 2.1
T3 VM (mjr-T3) version 3.1.3

>get penny. g.
Taken.

You’re already holding the penny.

You’re already holding the penny.

// ... repeated many times ...

You’re already holding the penny.

Runtime error: stack overflow
-->../adv3Lite/parser.t, line 3542
   ../adv3Lite/parser.t, line 3422
   ../adv3Lite/parser.t, line 4232
   ../adv3Lite/parser.t, line 3542
   ../adv3Lite/parser.t, line 3422

// ... stack loop repeated many times ...

   ../adv3Lite/parser.t, line 603
   ../adv3Lite/actions.t, line 3119
   ../adv3Lite/doer.t, line 414
   ../adv3Lite/doer.t, line 383
   ../adv3Lite/command.t, line 509
   ../adv3Lite/command.t, line 464
   ../adv3Lite/command.t, line 239
   ../adv3Lite/parser.t, line 603
   ../adv3Lite/actions.t, line 3119
   ../adv3Lite/doer.t, line 414
   ../adv3Lite/doer.t, line 383
   ../adv3Lite/command.t, line 509
   ../adv3Lite/command.t, line 464
   ../adv3Lite/command.t, line 239
   ../adv3Lite/parser.t, line 603
   ../adv3Lite/main.t, line 177
   ../adv3Lite/main.t, line 118
   ../adv3Lite/misc.t, line 124
   ../adv3Lite/main.t, line 70
   ../adv3Lite/main.t, line 24
   /opt/homebrew/Cellar/frobtads/2.0/share/frobtads/tads3/lib/_main.t, line 217
   /opt/homebrew/Cellar/frobtads/2.0/share/frobtads/tads3/lib/_main.t, line 122
   /opt/homebrew/Cellar/frobtads/2.0/share/frobtads/tads3/lib/_main.t, line 31

I can provide a full stack trace, if needed, but it’s quite easy to reproduce. The key is to issue a command on the object, followed by a period, and then g (i.e., AGAIN) followed by a period. It doesn’t work for all commands (for example, MOVE PENNY. G. doesn’t do it), but only some (DROP PENNY. G. will reproduce, even if you’re not holding the penny to start with).

I did a little investigating down the stack. I believe the infinite regress only occurs when the action has againRepeatsParse = true set, causing the last command to be parsed again. (Perhaps instead of re-parsing GET PENNY it’s re-parsing GET PENNY. G.?)

@Eric_Eve I would normally take this on to try and come up with a suitable patch, but these are in the guts of the Mercury parser, where I’m less familiar. I’ll keep looking, but I suspect you’ll come up with a proper fix faster than me.

2 Likes
2

That would be my best guess on first reading, but I’ll need to investigate it in more depth.

1 Like
3

@jnelson Further investigation reveals that this is indeed the case; the entire command line was being reparsed so one would get a potentially infinite series of GET PENNY. G. You’d get the same error with a semicolon GET PENNY; G

I’ve fixed this by changing the modified Command.buildCommandString() in english.t so that it strips G or AGAIN out of the command line if the command line contains a full stop or semicolon (note that this method is only used by the AGAIN command to construct the command line that is to be reparsed). Any translations of adv3Lite would need to make an equivalent change in their language-specific code.

The fix has been uploaded to GitHub and will appear in the next release of adv3Lite.

3 Likes
4

For what matter, I confirm the bug. (is useful having a test item available in the PC’s inventory; this allow me to point that the infinite multi-loop involves the last pair of command: with DROP DIE.TAKE DIE.G. I got this infinite loop:

Dropped.

Taken.

You’re already holding the testing die.

You’re already holding the testing die.

You’re already holding the testing die.

More seriously, the loop strike also when dealing with soft boundaries, and can strike during map traversal (N and G are nearby enough to be involuntary typed by mistake) and the overflow happens also when in a certain place in Railei one types:

w.w.w.g.w.w

and, much more serious, the direction-g bug strike also when bonking on a wall, (the most probable outcome in mistyping a traversal sequence…), with an overflow stemming from infinite “you can’t go that way…” loop. Thanks to the Divine that this really serious bug surfaces in Summer 2024 and not in Summer 2026…

Best regards from Italy,
dott. Piergiorgio.

5

I haven’t tested this in adv3lite proper because I have been working on a variation for my use and am on a Mac, but it looks like setting againRepeatsParse = nil on the Again object solves these problems. Needs to be tested.

1 Like
6

Great stuff, Eric. I pulled latest mainline, reran my test, and verified it’s working.

However—and I should have had this in my original post—my beta tester’s original bug report had him using a variant of this command:

>get penny. g. drop penny. g

This would cause the stack overflow as well, but I found a more minimal case, so that’s what I reported.

With your fix applied, the above now leads to this:

>get penny. g. drop penny. g
Taken. 

You’re already holding the penny. 

I don’t understand that command.Dropped. 

You aren’t holding the penny.

(Note the lack of white space between “…understand that command.” and “You…”)

This is infinitely better than the stack overflow, but it does look like there’s an intermediate bug here, between the first g. (AGAIN) action and the drop penny that’s leading to a NotUnderstoodError.

3 Likes
7

I’ve made another attempt at a fix which I’ve now uploaded to GitHub. It fixes the cases you mention but can get a bit confused if you string too many commands together on the same line, e,g., get penny. g. drop penny. g. get penny. g. drop penny. g (in this case you get output from 14 commands instead of the expected 8). But at least you don’t get the Command Not Understood problem and the game ends up in the right state. (I suspect what’s happening here relates to the scope of each G in the line).

At this point I’m not sure how to improve on it, but hopefully not too many players will attempt to string together large numbers of commands including AGAIN on the same command line and this partial fix will be more or less okay. It is at least an improvement on what was there before.

2 Likes
8

Thanks!

It came about due to a somewhat unusual puzzle, where a lever needs to be turned the same direction more than once. I don’t think a long string of AGAIN will happen too often.

I realize I didn’t mention this before: It was the inestimable playtesting skills of @rovarsson that found this bug. He deserves credit for the find.

2 Likes