weird error with Balances

Whenever I type gibberish, this happens.

gfds

[** Programming error: tried to read from -->-1 in the buffer (->) array “parse”, which has entries 2 up to 64 **]

[** Programming error: tried to read from -->-1 in the buffer (->) array “parse”, which has entries 2 up to 64 **]

[** Programming error: tried to read from -->-1 in the buffer (->) array “parse”, which has entries 2 up to 64 **]

[** Programming error: tried to read from -->-1 in the buffer (->) array “parse”, which has entries 2 up to 64 **]

[** Programming error: tried to read from -->-1 in the buffer (->) array “parse”, which has entries 2 up to 64 **]

[** Programming error: tried to read from -->-1 in the buffer (->) array “parse”, which has entries 2 up to 64 **]

[** Programming error: tried to read from -->-1 in the buffer (->) array “parse”, which has entries 2 up to 64 **]

[** Programming error: tried to read from -->-1 in the buffer (->) array “parse”, which has entries 2 up to 64 **]

[** Programming error: tried to read from -->-1 in the buffer (->) array “parse”, which has entries 2 up to 64 **]

[** Programming error: tried to read from -->-1 in the buffer (->) array “parse”, which has entries 2 up to 64 **]

[** Programming error: tried to read from -->-1 in the buffer (->) array “parse”, which has entries 2 up to 64 **]
That’s not a verb I recognise.

Balances by Graham Nelson? It’s very old and nobody has looked at it in a while. Possibly the version you’re playing was compiled with a library version that doesn’t quite match the source code.

Well, this is interesting. When compiled with the latest compiler and library, I get those errors. When using the one compiled with Inform v6.10 Library 6/3, gibberish is treated correctly.

I get the same errors when compiling balances.inf with Inform 6.33N using any library in the range 6/5 - 6/12.

I don’t see these errors when compiling advent.inf with the same compiler and libs.

The problem seems to be with Balance’s ParserError routine, which is part of its spell-casting system.

I’m sure that the ParserError routine is underrunning the parse array no matter how you compile it. If you use an older I6 compiler, the bug escapes notice.

I’ve looked more closely at this.

The underrun is happening during this code in ParserError:

      x=NextWordStopped();
      for (i=player+1:i<=top_object:i++)
          if (i ofclass Spell && Refers(i,x)==1
              && i has known_about) flag=1;

The purpose of this fragment is to determine if the erroneous word is a known spell name and to set flag if so.

Refers(i,x) is a library routine that returns a boolean indicating whether word x can refer to object i, where x is the index of a word in the parse buffer. Note that words in the parse buffer are numbered starting at 1.

The problem is that, when NextWordStopped returns 0, meaning that the next word isn’t in the dictionary, then x is set to 0 and Refers is called for word number 0, an invalid word number.

The underlying problem is that, even if the word were in the dictionary, NextWordStopped would return the word’s dict addr, not the word’s index that’s expected by Refers. In older versions of the library where the error reported by emlon isn’t seen, Refers instead expects the dict addr of a word.

Either way, the library could do more to protect against an invalid word number being passed as an argument.

Here’s an edited snippet of the Refers routine:

[ Refers obj wnum   wd k l m;
    if (obj == 0) rfalse;
    ...
    k = wn; wn = wnum; wd = NextWordStopped(); wn = k;
    ...
];

Note that it validates its obj argument, but not its wnum argument.

Refers sets wn based on wnum and calls NextWordStopped, which in turn calls NextWord.

Here’s the zcode version of NextWord:

[ NextWord i j;
    if (wn > parse->1) { wn++; rfalse; }
    i = wn*2-1; wn++;
    j = parse-->i;
    if (j == ',//') j = comma_word;
    if (j == './/') j = THEN1__WD;
    return j;
];

When wn is 0, the index i is computed as wn*2-1 = -1, so we wind up looking at parse–>-1.

The glulx version of NextWord suffers from the same sort of issue:

[ NextWord i j;
    if (wn > parse-->0) { wn++; rfalse; }
    i = wn*3-2; wn++;
    j = parse-->i;
    if (j == ',//') j=comma_word;
    if (j == './/') j=THEN1__WD;
    return j;
];

Note that both versions of NextWord check for wn being too large, but not for wn being too small.

So, we could fix the library issues by checking for invalid wn in NextWord and Refers. I’ve submitted a PR for this.

Balances, to be compatible with more recent library versions, should be changed to pass wn to Refers instead of calling NextWordStopped.

This bug is now fixed. Thanks for the report emlon! Thanks for the quick analysis Vince!

@emlon: You can grab the latest version of inform6lib with this fix here on github.

The fix gets rid of the bad accesses to the parse buffer and the resulting error messages, but Balances is still broken because of the way Refers changed in lib 6/5. Its ParserError won’t recognize known spell names and will fail to print specialized error messages for them.

Here’s an example interaction that demonstrates this:

(We read the book so that rezrov will become a known spell, meaning that we’ve seen its name.)

Here’s the same interaction with a ParserError that’s been updated for modern libraries:

To apply this fix, replace the existing ParserError with this one:

[ ParserError i flag vb;
  if (etype==VERB_PE or ASKSCOPE_PE or CANTSEE_PE)
  {   if (etype==ASKSCOPE_PE or CANTSEE_PE)
      {   if (verb_word=='cast') vb=1;
          if (verb_word=='learn' or 'memorise' or 'memorize') vb=2;
          if (verb_word=='copy') vb=3;
          if (vb==0) { etype=CANTSEE_PE; rfalse; }
      }
      wn=verb_wordnum; if (vb~=0) wn++;
      for (i=player+1:i<=top_object:i++)
          if (i ofclass Spell && Refers(i,wn)==1
              && i has known_about) flag=1;
      if (flag==1)
      {   if (vb==0 or 1)
             "You haven't got that spell committed to memory.  [Type ~spells~
              to see what you do remember.]";
          if (vb==2)
             "Your training is such that you can only memorise such a spell
              with the aid of a spell book containing it.";
          if (vb==3)
             "You have no text of that spell to copy.";
      }
      if (vb==1)
         "You haven't learned that spell, if indeed it is a spell.";
      if (vb==2 or 3)
         "You haven't access to that spell, if indeed it is a spell.";
  }
  rfalse;
];

I don’t think that the vb == 1 (cast) case worked before, since “cast rezrov” produces CANTSEE_PE even with very old libraries. This is because the cast grammar uses the is_spell attribute as a token rather than using a scope routine that would produce ASKSCOPE_PE. We could fix this by wrapping the attribute test in an IsSpell scope routine, but I decided to add CANTSEE_PE to ParserError instead since it localizes changes to a single routine.

@vlaviano, what do you think would be the oldest compiler and library that balances would now require?

The modified ParserError in my previous post requires lib 6/5 or higher. I don’t think that the changes affect the compiler version required.

However, I’d recommend lib 6/8+ for use with Inform 6.33, regardless of the game. Earlier libs produce an error message during parsing.

I’ve updated balances.inf in my inform6unix repo. Maybe we should comb through the other demo programs too looking for incompatibilities.

After further testing, I’ve learned that I should have gone with the former approach instead(*). The messages in ParserError are intended for parse errors for the direct object (the spell name), not the indirect object (what the spell’s being cast on). The game’s grammar uses scope tokens only for the direct object, so ASKSCOPE_PE will never be produced for the indirect object. However, CANTSEE_PE can be produced for either the direct object or the indirect object. So, my modified ParserError can produce erroneous messages for indirect objects.

Here’s an example:

I’ll fix this and submit a PR to inform6unix.

(*) The constraints imposed by InScope mean that we actually want a LearnedSpell scope routine, not an IsSpell one.

Might be good to insert Balances into the I6lib test suite (github.com/erkyrath/inform6-test) along with a test sequence that checks all of these parser-error cases.

I’ve done these two things.

The updated version of Balances is here (in the demos dir).

Thanks!