Bug in DirectEntry

Compiling or Editing the Panda Source
1

Hi,

I have a crash in DirectEntry that only occurs when Panda was built with Visual Studio 2005.
However, I looked into it some further and I believe that this is not a compiler bug, but rather a real programming bug in the source.

Here’s my code to reproduce the crash:

import direct.directbase.DirectStart
from direct.gui.DirectGui import *
from pandac.PandaModules import *

#add button
b = DirectEntry(text = "" ,scale=.05,focus=1)
run()
  • run this program with a version of Panda3d that has been compiled with Visual Studio 2005
  • press space (to enter a space as the first character in the DirectEntry field)
  • watch it crash

I ran the program in the debugger and found out the following:

  • Each keystroke eventually ends up in the function
void PGEntry::keystroke(const MouseWatcherParameter &param, bool background)
  • In this function, spaces are handled as a special case to avoid too long lines with all space at the end:
if (!too_long && keycode == ' ') {
      // Even if we haven't filled up all of the available
      // lines, we should reject a space that's typed at the
      // end of the current line if it would make that line
      // exceed the maximum width, just so we don't allow an
      // infinite number of spaces to accumulate.
      int r, c;
      _text.calc_r_c(r, c, _cursor_position);
  • when space is the first character you type, _cursor_position is 0 when _text.calc_r_c is invoked
  • the error occurs in the calc_r_c function:
bool TextAssembler::
calc_r_c(int &r, int &c, int n) const {
  nassertr(n >= 0 && n <= (int)_text_string.size(), false);

  if (n == (int)_text_string.size()) {
    // A special case for one past the last character.
    if (_text_string.empty()) {
      r = 0;
      c = 0;
    } else {
      r = _text_block.size() - 1;
      c = _text_block[r]._string.size();
    }
    return true;
  }

  r = 0;
  while (r + 1 < (int)_text_block.size() &&
         _text_block[r + 1]._row_start < n) {
    r += 1;
  }

  const TextRow &row = _text_block[r];
  bool is_real_char = true;

  if (row._got_soft_hyphens) {
    // If there are any soft hyphen or soft break keys in the source
    // text, we have to scan past them to get c precisely.
    c = 0;
    int i = row._row_start;
    while (i < n - 1) {
      if (_text_string[i]._character != text_soft_hyphen_key && 
          _text_string[i]._character != text_soft_break_key) {
        ++c;
      }
      ++i;
    }
    if (_text_string[n - 1]._character != text_soft_hyphen_key && 
        _text_string[n - 1]._character != text_soft_break_key) {
      ++c;
      if (_text_string[n - 1]._character == '\n') {
        is_real_char = false;
      }
    } else {
      is_real_char = false;
    }

  } else {
    // If there are no soft characters, then the string maps
    // one-to-one.
    c = min(n - row._row_start, (int)row._string.size());
    if (_text_string[n - 1]._character == '\n') {
      is_real_char = false;
    }
  }

  return is_real_char;
}

right at the bottom, in the last else-block, n still is 0 (coming from the passed in _cursor_position) and the indexing of [n -1] causes the error.

Since VS 2005 is all about security, it checks all index accesses and it thus halts the program.
When I use a Panda version built with VS 2003, no crash occurs, but I’m pretty sure that some random piece of memory gets overwritten.
So even if you don’t have VS 2005, you should be able to verify the behavior in about any other development environment.

I thought about fixing it myself, but the whole “space-at-the-end-of-the-line” handling mechanics seem a bit obscure to me, and I don’t want to break something else.

So could one of the Panda3D-gurus step in for the rescue? :wink:

Thanks,

Erik

2

You’re absolutely right, it’s a bug, though on non-VS2005 compilers it just creates a read of random memory, not a write, so it is largely harmless. Bugs of this nature crop of from time to time, because I don’t use VS2005 for Panda development, and none of the compilers I do use will generate code that raises an exception here.

My apologies. I’ve just checked in the appropriate fix to textAssembler.cxx.

David

3

Thanks, I’ll checkout the fix right away.

ashamed, yes of course you’re right, it READS from some random address. I missed the second = and thought it would write somewhere.

And I was already wondering a whole day why it doesn’t crash on older builds… :slight_smile:

Erik

4

By the way, thanks so much for your very detailed analysis of the problem. It made it a snap to go in and fix it, and your effort there is very much appreciated.

David