segfaulted (need some advice)

[Wizard]

I was on call a couple days ago when one of our programs crashed. I'm pretty sure I know why it crashed, I'd just like some confirmation, and some help with the right words before I fill in the official crash report. Here's the relevant parts:

Code: Select all

class Book {
public:
  void setTime(const char * tt1) { memcpy(_tt1, tt1, 32); }
private:
  char _tt1[32];
}
// and a bunch of stuff...
Book book;
RWCstring time = order.getTime(); // order.getTime returns a const char* of the time of this order.
book.setTime(time);

When used in a char* context, an RWCString returns the internal char* that it uses to store the string, similar to a std::string (I have no idea why this was written with RogueWave, I just go with the flow)
It crashes in the memcpy. The size of the RWCString is 12 bytes.
When I look up the data in gdb, the first 27 bytes of time and _tt1 are the same. The 28th byte is different. The 28th byte of time is at memory location 0x44500000. I don't believe that to be a coincidence.
So I'm pretty sure it faulted because it tried to look up memory that didn't exist in the TLB, or something. Tried to look up a different page? See, this is my problem, I understand physically why it crashed, but can't think of the right words to explain it.
And the fix is obvious: either ensure the RWCString has a capacity of 32 (which can be done in its constructor) or change memcpy to a strncpy.

[Alvaro]

Yes, reading memory from a buffer past the end of the buffer is undefined behavior. Don't do it.

[Wizard]

Aye, I know that, you know that, but when I explain to the upper ups this Tuesday, they'll want to know exactly why. I need techno babble and I'm not sure I'm using the right words. Some of them know what they're talking about and might understand the problem, and I'd look quite the fool if I used the wrong phrase.

[Alvaro]

The way it's implemented right now, setTime copies a buffer of length 32 into _tt, which is another buffer of length 32. Providing a shorter buffer, even if it is for reading, is undefined behavior. In practice, if you don't mind reading garbage, it will work most of the time. But then you may hit the end of a page and then the processor will generate an exception (probably why you saw a pointer with a very round number).

Anyway, the exact reason of why it crashes is irrelevant. You either make sure that the origin buffer is at least 32 bytes long (and document this in setTime), or you change the implementation to not read past the first '\0', if that's what the intended behavior was.

[Wizard]

"Reading memory outside the page" was accepted.

[Alvaro]

I am surprised "invoking undefined behavior according to the standard" wasn't enough.

[Wizard]

It's only undefined because you don't know what will happen. Once the monkeys have flown out my butt, I can say "this set of circumstances has undefined behavior. This particular instance caused monkeys to fly out my butt".
It was because it had worked for months without crashing that they wanted to know what specifically about this set of circumstances caused it to crash.

[ventsyv]

buffer-over-read maybe? Or "reading passed array boundary" ?