catseye / fbbi Goto Github PK

According to the specification:

The } "End Block" instruction pops a cell off the stack that it calls n, then pops a vector off the SOSS which it assigns to the storage offset, then transfers n elements (as a block) from the TOSS to the SOSS, then pops the top stack off the stack stack. If n is negative, |n| cells are popped off of the (original) SOSS.

But looking at the code for fi_end, we have:

1. n is popped off the stack (line 237)
2. if n is negative, |n| cells are popped from the SOSS (lines 238-249)
3. the storage offset is restored from the SOSS (lines 251-252)
4. if n is positive, n elements are transfered from the TOSS to the SOSS (line 253)

So when n is negative, the SOSS is cleared too early, and the storage offset gets nuked before it has a chance to be restored. Basically step 3 needs to happen before step 2.

Btw, this is the reason for the Mycology error:

BAD: u with a positive argument gives strange storage offset: expected (0,0)

It's confusing because the real failure happens at the time of the negative argument test, but it's not picked up because that is just checking whether the right number of cells were removed from the stack. The right number were removed, but they were removed in the wrong order, so the storage offset got initialised with junk, and that was the cause of the u failure several tests later.

Been meaning to file this one for a while now. Basically any of the operations that take a string parameter will cause a segfault when operating on an empty stack. A simple test case is:

=@

This should attempt to "execute" an empty string, which on most platforms does nothing, but on FBBI this will generate a segfault because of a null pointer reference.

The fault occurs in the stack_pop_string function. On an empty stack, the initial stack_pop call on line 119 returns null, and the while loop is never executed, but since s->sp == -1 is true on an empty stack, the null that was assigned to x is dereferenced in the final condition on line 126, and thus we get a segfault.

I think the fault can be prevented with the following replacement code:

void stack_pop_string(stack * s, char * str)
{
  int i = 0;
  void * x = NULL;
  SDEBUG("Before Pop String", s);
  x = stack_pop(s);
  while ((x != NULL) && (*(char *)x != 0))
  {
    str[i++] = *(char *)x;
    x = stack_pop(s);
  }
  str[i] = (char)'\0';
}

And here's a more thorough test case that validates a couple of edge cases:

"llun ticilpmI ohce"=$ 0"llun ticilpxE ohce"=$ = a"kcats ytpmE"bk, @

In the program above, we're calling the execute command three times - first using a string with an implicit null terminatorr, then with an explicit null terminator, and finally using a completely empty stack. If all the calls are successful, the program should output:

Implicit null
Explicit null
Empty Stack

If we want to get more adversarial, though, there is still another potential segfault. The stack_pop_string function returns the popped string by copying it into a fixed length buffer (always 256 chars). So we can easily cause a segfault just by using a string longer than 256 characters. A simple test case:

'd3*k1 " ohce"= @

If you want to avoid this fault, you'd need to add a check in the stack_pop_string function to prevent it overflowing the buffer. Something like this should do the trick:

void stack_pop_string(stack * s, char * str)
{
  int i = 0;
  void * x = NULL;
  SDEBUG("Before Pop String", s);
  x = stack_pop(s);
  while ((x != NULL) && (*(char *)x != 0))
  {
    if (i+1 < 256) str[i++] = *(char *)x;
    x = stack_pop(s);
  }
  str[i] = (char)'\0';
}

Although ideally you shouldn't be hardcoding the size like that - it would be safer to pass that in as a parameter from the various calling locations.

For example, the code below:

"] ; ; ; ; ; [">:#,_@

should be expected to produce the following output:

[ ; ; ; ; ; ]

However, on FBBI the output looks like this:

[;;;;; ]

What's happening is the later spaces are mistakenly detected as an uninterrupted series, and a series of spaces is expected to be collapsed into one in Befunge-98. The source of the problem is the ip_move function, which has an early exit when a ; is encountered in stringmode. This prevents the spacemode (pm) variable being reset, which would usually occur at the end of the of the function.

A simple two character program of the form  @ (a space followed by @) will cause FBBI to go into an infinite loop.

The problem is that the first column being empty means the left boundary of the program is considered to be column 1. This means that the program counter initially starts off out of bounds, and when that happens, FBBI thinks it has wrapped past the right boundary, so it reverses direction and starts backtracking. However, in this case it's outside the left boundary so backtracking is of no help whatsoever - it's just going to keep backtracking forever (or at least until some kind of integer overflow).

The same sort of issue can be triggered with a jump instruction, by jumping outside the program boundary with the program counter moving towards the boundary that you've just escaped. For example, this program should also be expected to terminate, but doesn't:

>v
@>9-j

In the Get SysInfo command, the documentation for the current date just says it should return:

((year - 1900) * 256 * 256) + (month * 256) + (day of month)

It doesn't explicitly declare the range of the month and day of month fields, but I think most people would assume a month in the range 1 to 12, and a day of month in the range 1 to 31, unless specified otherwise.

However, it appears that FBBI is returning the month in the range 0 to 11. Looking at the code, my impression is it's just using what the C stdlib tm_mon field returns, and wasn't actually a conscious decision. So I may be wrong, but I'm assuming this is a bug.

It's also worth mentioning that every other Befunge-98 interpreter I've tested* has gone with a month range of 1 to 12, so FBBI certainly appears to be the odd one out in this regard.

[*] I've tried CCBI, Fungi, PyFunge, cfunge, and Rc/Funge.

Given the argument 6, the y (Get SysInfo) command is supposed to return the operating system's path separator. On Windows I would have expected this to be a backslash, but it's not.

The reason for this behaviour is that FBBI is determining which separator to return by testing the __MSDOS__ define. A more accurate result might be accomplished by testing both __MSDOS__ and _WIN32. So maybe something like this:

#if defined(__MSDOS__) || defined(_WIN32) 
  ip_push(i, (long)0|'\\');  /* MS-DOS or Windows paradigm */
#else
  ip_push(i, (long)0|'/');   /* probably.  Mac users will have 2 suffer 4 now :-> */
#endif

The iterate command k seems to treat 0 specially. In particular, I'm observing the following behaviour:

• If the argument is 0, the next instruction is skipped.
• If the argument is positive (n), the next instruction is executed n times with IP at the current position. Then the IP advances and the next command is executed again at the position of that command. That means if I do something like 3k., I'll actually print 4 numbers. In particular, it's not possible to execute a command only once using k, unless it's some sort of direction changing or source code modifying command.

I'm not sure whether this is the way it was intended, but the specification (looking at this document: https://github.com/catseye/Funge-98/blob/master/doc/funge98.markdown) doesn't mention that 0 is treated differently from other inputs. It mentions that zero would skip the next instruction entirely, but I had assumed that this means argument 1 would result in it being executed exactly once.

Quoting from the section on Strings in the Funge-98 documentation:

In Funge-98 stringmode, spaces are treated "SGML-style"; that is, when any contiguous series of spaces is processed, it only takes one tick and pushes one space onto the stack. This introduces a small backward-incompatibility with Befunge-93; programs that have multiple spaces and/or wrap while in stringmode will have to be changed to work the same under Funge-98.

This suggests to me that strings that wrap around the playfield should push one space onto the stack when that wrapping occurs. If my understanding is correct, I would then expect the program below to output 32:

<@."

At least that's how most Befunge-98 implementations seem to interpret it. But in FBBI that code ends up outputting 60 (the ASCII value for <), since no space is pushed.

My first thought was that this was just a different interpretation of the spec, but the behaviour isn't consistent. If you add another line of content to the source that is wider the first line, then the behaviour suddenly changes.

<@."
xxxxx

Now FBBI will output 32 rather than 60, even though that second line should clearly have no effect on the result. This makes me think the original behaviour might well have been a bug.

As for fixing it, my first thought would be to remove the final ip_march call from the ip_backtrack function. That way you're guaranteed to execute at least one out-of-bounds step after a backtrack, and thus you're guaranteed a space will always be pushed. You shouldn't have to worry about too many spaces, since the '98 spacemode check already handles that. There may be other problems that arise from such a change, though.

According to the specification:

In the case of an end-of-file or other file error condition, the & and ~ both act like r.

However, when it comes to the & (Input Decimal) command, FBBI goes into an infinite loop in the case of an end-of-file.

The problem is the loop at the start of the fi_idec function which tries to skip over any non-digits in the input stream. On end-of-file, a digit is never encountered so the loop never ends.

That loop is also responsible for FBBI failing to read negative numbers correctly, because the sign of the number is dropped before it gets to the scanf call.