From 14ebd5d58be3fcb5d2208f890498dd8c57f4d165 Mon Sep 17 00:00:00 2001 From: Robin Haberkorn Date: Tue, 17 Mar 2015 20:18:18 +0100 Subject: fixed invalid memory accesses in the expression stack and reworked expression stack this was probably a regression from d94b18819ad4ee3237c46ad43a962d0121f0c3fe and should not be in v0.5. The return value of Expressions::find_op() must always be checked since it might not find the operator, returning 0 (it used to be 0). A zero index pointed to uninitialized memory - in the worst case it pointed to invalid memory resulting in segfaults. Too large indices were also not handled. This was probably responsible for recent PPA build issues. Valgrind/memcheck reports this error but I misread it as a bogus warning. I took the opportunity to clean up the ValueStack implementation and made it more robust by adding a few assertions. ValueStacks now grow from large to small addresses (like stack data structures usually do). This means, there is no need to work with negative indices into the stack pointer. To reduce the potential for invalid stack accesses, stack indices are now unsigned and have origin 0. Previously, all indices < 1 were faulty but weren't checked. Also, I added some minor optimizations. --- src/parser.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'src/parser.cpp') diff --git a/src/parser.cpp b/src/parser.cpp index 6ca6652..b16b792 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -898,7 +898,7 @@ StateStart::custom(gchar chr) BEGIN_EXEC(this); v = QRegisters::globals["_"]->get_integer(); - rc = expressions.pop_num_calc(1, v); + rc = expressions.pop_num_calc(0, v); if (eval_colon()) rc = ~rc; @@ -1086,7 +1086,7 @@ StateStart::custom(gchar chr) */ case 'J': BEGIN_EXEC(this); - v = expressions.pop_num_calc(1, 0); + v = expressions.pop_num_calc(0, 0); if (Validate::pos(v)) { if (current_doc_must_undo()) interface.undo_ssm(SCI_GOTOPOS, @@ -1888,7 +1888,7 @@ StateECommand::custom(gchar chr) expressions.push(Flags::ed); } else { tecoInt on = expressions.pop_num_calc(); - tecoInt off = expressions.pop_num_calc(1, ~(tecoInt)0); + tecoInt off = expressions.pop_num_calc(0, ~(tecoInt)0); undo.push_var(Flags::ed); Flags::ed = (Flags::ed & ~off) | on; @@ -2227,10 +2227,10 @@ cleanup: if (!expressions.args()) throw Error(" command requires at least a message code"); - scintilla_message.iMessage = expressions.pop_num_calc(1, 0); + scintilla_message.iMessage = expressions.pop_num_calc(0, 0); } if (!scintilla_message.wParam) - scintilla_message.wParam = expressions.pop_num_calc(1, 0); + scintilla_message.wParam = expressions.pop_num_calc(0, 0); return &States::scintilla_lparam; } @@ -2242,7 +2242,7 @@ StateScintilla_lParam::done(const gchar *str) if (!scintilla_message.lParam) scintilla_message.lParam = *str ? (sptr_t)str - : expressions.pop_num_calc(1, 0); + : expressions.pop_num_calc(0, 0); expressions.push(interface.ssm(scintilla_message.iMessage, scintilla_message.wParam, @@ -2288,7 +2288,7 @@ StateScintilla_lParam::done(const gchar *str) void StateInsert::initial(void) { - int args; + guint args; expressions.eval(); args = expressions.args(); @@ -2297,7 +2297,7 @@ StateInsert::initial(void) interface.ssm(SCI_BEGINUNDOACTION); for (int i = args; i > 0; i--) { - gchar chr = (gchar)expressions.peek_num(i); + gchar chr = (gchar)expressions.peek_num(i-1); interface.ssm(SCI_ADDTEXT, 1, (sptr_t)&chr); } for (int i = args; i > 0; i--) -- cgit v1.2.3