[coreutils] pr -e

Version:

coreutils-6.10 (fixed in 6.11)

How to reproduce?

$ coreutils-6.10/src/pr -e pr-e.bin

pr-e.bin is a file that we created to trigger this bug, which can be found at:

http://opera.ucsd.edu/errlog/pr-e.bin

This file contains many backspaces (‘\b’ or 0x08).

Symptom:

Incorrect results.

In version 6.10, pr keeps outputing backspace(‘\b’ or 0x08) after it already reaches the position 0. The printed content will mess up the header. --- you can use editors that can display backspace (such as ultraedit) to see the effect.

in version 6.11 the pr stop output backspace(‘\b’ or 0x08) after it already reaches the position 0.

Root cause:

The root cause is that the error version didn’t check for negative value of ‘input_position’ (where the position of the cursor is)!

Comments written by us are in blue color.

/* This global: input_position, is key to this failure!

  It indicates the horizontal position within the file.

  It should never become negative! */

/* Horizontal position relative to the current file.

  (output_position depends on where we are on the page;

  input_position depends on where we are in the file.)

  Important for converting tabs to spaces on input. */

static int input_position;

/* processing each char and returns the printing width of

this char. Here, when c = ‘\b’.

* */

char_to_clump(char c) {

 s = clump_buff;

 if (!isprint (c)) {

   … …

   if (c == ‘\b’) {

      /* if ‘\b’, then we should decrease the position index. */

      width = -1;

      chars = 1;

      *s = c;

    }

  }

  /* The patch is to check whether input_position reaches 0. */

+  /* Too many backspaces must put us in position 0 -- never negative.  */

+  if (width < 0 && input_position == 0) {

+          chars = 0;

+          input_position = 0;

+   }

+  else if (width < 0 && input_position <= -width)

+        input_position = 0;

+  else

  /* Without the check, input_position will blindly decrease, and eventually

     become negative. */

        input_position += width;

     return chars; // ‘chars’ is the width of the character.

}

read_line() {

 for (;;) {

   c = getc (p->fp);

   switch (c) {

     case ‘\n’:

       return true;

...

case EOF:

            close_file (p);

           return true;

}

         last_input_position = input_position;

     /* for all the ‘\b’s, each time, the char_to_clump will return 1,

        and this \b is blindly printed by pr from the print_clump

        function below. */

         chars = char_to_clump (c);

     if (truncate_lines && input_position > chars_per_column)

        {

          input_position = last_input_position;

          return false;

        }

         print_clump (p, chars, clump_buff);

        } // for (;;)

} // readline

print_clump (COLUMN *p, int n, char *clump) {

 while (n--)

     /* This is the output point! p->char_func is print_char, which

        will call putchar. Such printing will eventually mess up the

        header. */

     (p->char_func) (*clump++);

}

The for loop in ‘read_line’ is to print one character per iteration. It calls ‘char_to_clump’ to convert a ‘\t’ to 8 spaces. So ‘char_to_clump’ would return how many characters to print in print_clump. However,in buggy version, when there is a lot of ‘\b’, char_to_clump would blindly treat each one as \b, and everytime returns a 1, even when ‘input_position’ becomes negative. This will cause the print_clump to keep printing \b blindly, even if the page header is reached. The correct semantic should be: ‘char_to_clump’ follows ‘\b’ faithfully, UNTIL input_position reaches 0,which means the page top is reached and \b shouldn’t be output anymore.

Information needed to diagnose:

Know ‘input_position’ is < 0 is enough.

Can Errlog print an error message?

No. There is no error check, and this error condition (input_position < 0) is domain specific.

Discussions

We believe this error can be detected via runtime invariant checker. For example, the invariant checker, used in testing, can learn input_position should never by positive, and we can add an error check for this condition.