From 5726ec9862816078693652a3b6b63262f70605ea Mon Sep 17 00:00:00 2001 From: Martin Kedmenec Date: Thu, 24 Apr 2025 14:34:52 +0200 Subject: [PATCH 1/2] Fix #163. Improve VM stack push/pop logic --- src/vm.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/vm.c b/src/vm.c index db61d29..22d9c60 100644 --- a/src/vm.c +++ b/src/vm.c @@ -27,14 +27,23 @@ static constexpr int kFunctionPoolSize = 16; #endif /// Pushes a value onto the stack. -/// Is defined as a macro since cc65 doesn't support passing structs to -/// functions by value for regular functions. -#define Push(value) (stack[stack_index++] = (value)) +/// Defined as a macro since cc65 doesn't support passing structs to functions +/// by value for regular functions. +#define Push(value) \ + do { \ + if (kStackSize <= stack_index) { \ + puts("Error: Stack overflow."); \ + } else { \ + stack[stack_index++] = (value); \ + } \ + } while (0) /// Pops a value from the stack. -/// Is defined as a macro since cc65 doesn't support passing structs to -/// functions by value for regular functions. -#define Pop() (stack[--stack_index]) +/// Defined as a macro since cc65 doesn't support passing structs to functions +/// by value for regular functions. +#define Pop() \ + (0 == stack_index ? (puts("Error: Stack underflow."), kEmptyStackValue) \ + : stack[--stack_index]) typedef struct Constants { const void* pointer[kConstantsSize]; @@ -49,14 +58,21 @@ typedef struct Function { } Function; typedef struct StackValue { - VariableType type; union as { int number; char* string; Function* function; } as; + VariableType type; +#ifdef __CC65__ + unsigned char padding; ///< Used to add 1 byte padding to the struct, so that + ///< the whole struct has a size of exactly 4 bytes. + ///< See https://cc65.github.io/doc/cc65.html#s4 +#endif } StackValue; +static const StackValue kEmptyStackValue = {}; + typedef struct CallFrame { size_t return_address[kCallFrameTableSize]; size_t stack_offset[kCallFrameTableSize]; From c0eb3dafe2f323fd02284dc6c310ac36cf9fdfeb Mon Sep 17 00:00:00 2001 From: Martin Kedmenec Date: Fri, 25 Apr 2025 09:51:51 +0200 Subject: [PATCH 2/2] Fix #167. Fix bytecode corruption in nested conditionals --- src/parser.c | 22 +++++++++------ tests/lexer_test.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++ tests/lexer_test.h | 1 + tests/main.c | 1 + 4 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/parser.c b/src/parser.c index ef0bcd9..07d914d 100644 --- a/src/parser.c +++ b/src/parser.c @@ -422,10 +422,13 @@ static void ParsePrintStatement() { EmitByte(kOpPrint); } +// clang-format off +#pragma static-locals(push, off) +// clang-format on // NOLINTNEXTLINE(misc-no-recursion) static void ParseIfStatement() { - size_t if_jump_address = 0; - size_t else_jump_address = 0; + size_t condition_patch_slot = 0; + size_t exit_patch_slot = 0; if (!ExpectToken(1, kTokenLeftParenthesis)) { return; @@ -439,7 +442,7 @@ static void ParseIfStatement() { EmitByte(kOpJumpIfFalse); - if_jump_address = instruction_address; + condition_patch_slot = instruction_address; EmitByte(0); @@ -449,7 +452,7 @@ static void ParseIfStatement() { } if (kTokenElse != token.type) { - instructions[if_jump_address] = instruction_address; + instructions[condition_patch_slot] = instruction_address; ExpectToken(1, kTokenEndif); @@ -458,11 +461,11 @@ static void ParseIfStatement() { EmitByte(kOpJump); - else_jump_address = instruction_address; + exit_patch_slot = instruction_address; EmitByte(0); - instructions[if_jump_address] = instruction_address; + instructions[condition_patch_slot] = instruction_address; ConsumeNextToken(); @@ -470,10 +473,13 @@ static void ParseIfStatement() { ParseStatement(); } - instructions[else_jump_address] = instruction_address; + instructions[exit_patch_slot] = instruction_address; ExpectToken(1, kTokenEndif); } +// clang-format off +#pragma static-locals(pop) +// clang-format on static void DefineVariable(const char* const identifier_name, const VariableType type, const bool is_local) { @@ -727,7 +733,7 @@ static void ParseIdentifierStatement(const char* const identifier_name) { // NOLINTNEXTLINE(misc-no-recursion) static void ParseStatement() { - static char identifier_name[kIdentifierNameLength]; + char identifier_name[kIdentifierNameLength]; if (AcceptToken(1, kTokenPrint)) { ParsePrintStatement(); diff --git a/tests/lexer_test.c b/tests/lexer_test.c index 7ab097d..5cc7e23 100644 --- a/tests/lexer_test.c +++ b/tests/lexer_test.c @@ -189,6 +189,76 @@ void TestIf() { TEST_ASSERT_EQUAL_INT(kTokenEndif, token.type); } +void TestNestedIf() { + FillProgramBuffer( + "if(true)\nif(false)\nprint(\"foo\")\nelse\nprint(\"bar\")" + "\nendif\nendif"); + + ConsumeNextToken(); // if + TEST_ASSERT_EQUAL_INT(kTokenIf, token.type); + + ConsumeNextToken(); // ( + TEST_ASSERT_EQUAL_INT(kTokenLeftParenthesis, token.type); + + ConsumeNextToken(); // true + TEST_ASSERT_EQUAL_INT(kTokenBoolean, token.type); + TEST_ASSERT_EQUAL_INT(1, token.value.number); + + ConsumeNextToken(); // ) + TEST_ASSERT_EQUAL_INT(kTokenRightParenthesis, token.type); + + ConsumeNextToken(); // if + TEST_ASSERT_EQUAL_INT(kTokenIf, token.type); + + ConsumeNextToken(); // ( + TEST_ASSERT_EQUAL_INT(kTokenLeftParenthesis, token.type); + + ConsumeNextToken(); // false + TEST_ASSERT_EQUAL_INT(kTokenBoolean, token.type); + TEST_ASSERT_EQUAL_INT(0, token.value.number); + + ConsumeNextToken(); // ) + TEST_ASSERT_EQUAL_INT(kTokenRightParenthesis, token.type); + + ConsumeNextToken(); // print + TEST_ASSERT_EQUAL_INT(kTokenPrint, token.type); + + ConsumeNextToken(); // ( + TEST_ASSERT_EQUAL_INT(kTokenLeftParenthesis, token.type); + + ConsumeNextToken(); // foo + TEST_ASSERT_EQUAL_INT(kTokenString, token.type); + TEST_ASSERT_EQUAL_STRING("foo", token.value.text); + + ConsumeNextToken(); // ) + TEST_ASSERT_EQUAL_INT(kTokenRightParenthesis, token.type); + + ConsumeNextToken(); // else + TEST_ASSERT_EQUAL_INT(kTokenElse, token.type); + + ConsumeNextToken(); // print + TEST_ASSERT_EQUAL_INT(kTokenPrint, token.type); + + ConsumeNextToken(); // ( + TEST_ASSERT_EQUAL_INT(kTokenLeftParenthesis, token.type); + + ConsumeNextToken(); // bar + TEST_ASSERT_EQUAL_INT(kTokenString, token.type); + TEST_ASSERT_EQUAL_STRING("bar", token.value.text); + + ConsumeNextToken(); // ) + TEST_ASSERT_EQUAL_INT(kTokenRightParenthesis, token.type); + + ConsumeNextToken(); // endif + TEST_ASSERT_EQUAL_INT(kTokenEndif, token.type); + + ConsumeNextToken(); // endif + TEST_ASSERT_EQUAL_INT(kTokenEndif, token.type); + + ConsumeNextToken(); // EOF + TEST_ASSERT_EQUAL_INT(kTokenEof, token.type); +} + void TestFor() { FillProgramBuffer("for(true)print(\"Hello, world!\")endfor"); diff --git a/tests/lexer_test.h b/tests/lexer_test.h index b8d181c..b39fe4f 100644 --- a/tests/lexer_test.h +++ b/tests/lexer_test.h @@ -16,6 +16,7 @@ void TestGreaterThanOrEqualTo(); void TestLessThanOrEqualTo(); void TestBooleanLiteral(); void TestIf(); +void TestNestedIf(); void TestFor(); void TestNot(); void TestIntVariableDeclaration(); diff --git a/tests/main.c b/tests/main.c index f9cc577..cdedc67 100644 --- a/tests/main.c +++ b/tests/main.c @@ -30,6 +30,7 @@ int main() { RUN_TEST(TestLessThanOrEqualTo); RUN_TEST(TestBooleanLiteral); RUN_TEST(TestIf); + RUN_TEST(TestNestedIf); RUN_TEST(TestFor); RUN_TEST(TestNot); RUN_TEST(TestIntVariableDeclaration);