Issue server/2342: change assert in serialization to not fire for pc …#54
Issue server/2342: change assert in serialization to not fire for pc …#54HaroldCindy merged 2 commits intomainfrom
Conversation
…if script in crashed state. Set run bit on two buld scripts.
HaroldCindy
left a comment
HaroldCindy
left a comment
There was a problem hiding this comment.
Thanks! Just one note
VM/src/ares.cpp
Outdated
| // the PC had better be in bounds. | ||
| eris_assert(pc_offset >= 0 && pc_offset < lcl->l.p->sizecode); | ||
| // If we have a thread that hasn't been hard-killed, the PC had better be in bounds. | ||
| eris_assert(thread->status != LUA_ERRKILL && pc_offset >= 0 && pc_offset < lcl->l.p->sizecode); |
There was a problem hiding this comment.
Would be better keep the assert as-is, but push it into the
if (yield_point == -1) {
switch(thread->status) {
case LUA_OK:
case LUA_YIELD:
case LUA_BREAK:
// expected to be valid, but is not
case below. We expect that for any out-of-bounds pc_offset it would eventually end up in that branch anyways, and doing the yieldpoints check with an OOB value will just always result in -1
There was a problem hiding this comment.
That path ends up calling erris_error, which terminates and reports an error. The assert would be redundant in that situation and could just be removed in favor of the error report couldn't it?
…st invalid bytecode.
There was a problem hiding this comment.
The condition guarded against by this assert is handled below on line 2288.
| for (i=0; i<p->sizeyieldpoints; ++i) | ||
| { | ||
| p->yieldpoints[i] = READ_VALUE(int32_t); | ||
| if (p->yieldpoints[i] < 0 || p->yieldpoints[i] >= p->sizecode) { |
There was a problem hiding this comment.
Yield points being initialized from incoming (unknown) bytecode could be out of range. Raise the error here.
2 participants
fixes https://github.com/secondlife/server/issues/2342: Only assert on an out of range PC if the script is not in a crashed state.