From 6068b565f5f487ad725cacc1ff79ed4451cd89b7 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Fri, 30 Aug 2019 04:42:56 +0200 Subject: [PATCH 1/6] Add recursion limit for INCLUDE and macros (And REPT.) Not exactly a *recursion* limit, more like a *stack depth* limit, but calling it "recursion" conveys its purpose better. The default of 64 is super overkill: even in a a project with what I believe to be above-average levels of nesting, the level only peaked at 6. Keeping in mind the purpose of this is to catch infinite recursion, which is still caught quickly (in usual cases, anyways), this default seems sensible. And it passes tests. What more do you need? --- include/asm/fstack.h | 2 ++ src/asm/fstack.c | 10 ++++++++++ src/asm/main.c | 10 +++++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/include/asm/fstack.h b/include/asm/fstack.h index 0c0c74c2..f3e8a878 100644 --- a/include/asm/fstack.h +++ b/include/asm/fstack.h @@ -35,6 +35,8 @@ struct sContext { uint32_t nREPTBlockSize; }; +extern unsigned int nMaxFileStackDepth; + void fstk_RunInclude(char *tzFileName); void fstk_RunMacroArg(int32_t s); void fstk_Init(char *s); diff --git a/src/asm/fstack.c b/src/asm/fstack.c index 3a84877e..b471cdd6 100644 --- a/src/asm/fstack.c +++ b/src/asm/fstack.c @@ -28,6 +28,8 @@ #include "types.h" static struct sContext *pFileStack; +static unsigned int nFileStackDepth; +unsigned int nMaxFileStackDepth; static struct sSymbol *pCurrentMacro; static YY_BUFFER_STATE CurrentFlexHandle; static FILE *pCurrentFile; @@ -51,6 +53,8 @@ uint32_t ulMacroReturnValue; #define STAT_isMacroArg 2 #define STAT_isREPTBlock 3 +/* Max context stack size */ + /* * Context push and pop */ @@ -58,6 +62,9 @@ static void pushcontext(void) { struct sContext **ppFileStack; + if (++nFileStackDepth > nMaxFileStackDepth) + fatalerror("Recursion limit (%d) exceeded", nMaxFileStackDepth); + ppFileStack = &pFileStack; while (*ppFileStack) ppFileStack = &((*ppFileStack)->pNext); @@ -158,6 +165,8 @@ static int32_t popcontext(void) fatalerror("%s: Internal error.", __func__); } + nFileStackDepth--; + free(*ppLastFile); *ppLastFile = NULL; yy_switch_to_buffer(CurrentFlexHandle); @@ -417,6 +426,7 @@ void fstk_Init(char *pFileName) if (pCurrentFile == NULL) err(1, "Unable to open file '%s'", pFileName); } + nFileStackDepth = 0; nMacroCount = 0; nCurrentStatus = STAT_isInclude; diff --git a/src/asm/main.c b/src/asm/main.c index c2c79e3e..0d928e92 100644 --- a/src/asm/main.c +++ b/src/asm/main.c @@ -315,6 +315,8 @@ int main(int argc, char *argv[]) /* yydebug=1; */ + nMaxFileStackDepth = 64; + DefaultOptions.gbgfx[0] = '0'; DefaultOptions.gbgfx[1] = '1'; DefaultOptions.gbgfx[2] = '2'; @@ -332,7 +334,7 @@ int main(int argc, char *argv[]) newopt = CurrentOptions; - while ((ch = getopt(argc, argv, "b:D:Eg:hi:LM:o:p:Vvw")) != -1) { + while ((ch = getopt(argc, argv, "b:D:Eg:hi:LM:o:p:r:Vvw")) != -1) { switch (ch) { case 'b': if (strlen(optarg) == 2) { @@ -386,6 +388,12 @@ int main(int argc, char *argv[]) errx(1, "Argument for option 'p' must be between 0 and 0xFF"); break; + case 'r': + nMaxFileStackDepth = strtoul(optarg, &ep, 0); + + if (optarg[0] == '\0' || *ep != '\0') + errx(1, "Invalid argument for option 'r'"); + break; case 'V': printf("rgbasm %s\n", get_package_version_string()); exit(0); From dc2c97fe0cf129941b54f8213e536ab2bbfd8463 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Fri, 30 Aug 2019 20:47:04 +0200 Subject: [PATCH 2/6] Comment and improve ParseSymbol and AppendMacroArg --- include/asm/lexer.h | 2 +- src/asm/globlex.c | 44 +++++++++++++++++++++++++------------------- src/asm/lexer.c | 11 ++++++----- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/include/asm/lexer.h b/include/asm/lexer.h index 94c2affa..3a86a773 100644 --- a/include/asm/lexer.h +++ b/include/asm/lexer.h @@ -65,7 +65,7 @@ void lex_SetBuffer(char *buffer, uint32_t len); int yywrap(void); int yylex(void); void yyunput(char c); -void yyunputstr(char *s); +void yyunputstr(const char *s); void yyskipbytes(uint32_t count); void yyunputbytes(uint32_t count); diff --git a/src/asm/globlex.c b/src/asm/globlex.c index 7884b8c6..98786a34 100644 --- a/src/asm/globlex.c +++ b/src/asm/globlex.c @@ -188,11 +188,11 @@ uint32_t ParseNumber(char *s, uint32_t size) } /* - * If the symbol name ends before the end of the macro arg, return true - * and point "rest" to the rest of the macro arg. - * Otherwise, return false. + * If the symbol name ends before the end of the macro arg, + * return a pointer to the rest of the macro arg. + * Otherwise, return NULL. */ -bool AppendMacroArg(char whichArg, char *dest, size_t *destIndex, char **rest) +char *AppendMacroArg(char whichArg, char *dest, size_t *destIndex) { char *marg; @@ -222,14 +222,13 @@ bool AppendMacroArg(char whichArg, char *dest, size_t *destIndex, char **rest) dest[*destIndex] = ch; (*destIndex)++; } else { - *rest = marg; - return true; + return marg; } marg++; } - return false; + return NULL; } uint32_t ParseSymbol(char *src, uint32_t size) @@ -251,7 +250,9 @@ uint32_t ParseSymbol(char *src, uint32_t size) */ ch = src[srcIndex++]; - if (AppendMacroArg(ch, dest, &destIndex, &rest)) + rest = AppendMacroArg(ch, dest, &destIndex); + /* If the symbol's end was in the middle of the token */ + if (rest) break; } else { if (destIndex >= MAXSYMLEN) @@ -262,28 +263,33 @@ uint32_t ParseSymbol(char *src, uint32_t size) dest[destIndex] = 0; + /* Tell the lexer we read all bytes that we did */ + yyskipbytes(srcIndex); + + /* + * If an escape's expansion left some chars after the symbol's end, + * such as the `::` in a `Backup\1` expanded to `BackupCamX::`, + * put those into the buffer. + * Note that this NEEDS to be done after the `yyskipbytes` above. + */ + if (rest) + yyunputstr(rest); + + /* If the symbol is an EQUS, expand it */ if (!oDontExpandStrings && sym_isString(dest)) { char *s; - yyskipbytes(srcIndex); - - if (rest) - yyunputstr(rest); - + /* Feed the symbol's contents into the buffer */ yyunputstr(s = sym_GetStringValue(dest)); + /* Lines inserted this way shall not increase nLineNo */ while (*s) { if (*s++ == '\n') - nLineNo -= 1; + nLineNo--; } return 0; } - yyskipbytes(srcIndex); - - if (rest) - yyunputstr(rest); - strcpy(yylval.tzSym, dest); return 1; } diff --git a/src/asm/lexer.c b/src/asm/lexer.c index 66d0d33d..cdaaaf55 100644 --- a/src/asm/lexer.c +++ b/src/asm/lexer.c @@ -88,17 +88,18 @@ void yyunput(char c) *(--pLexBuffer) = c; } -void yyunputstr(char *s) +void yyunputstr(const char *s) { - int32_t i, len; + int32_t len; len = strlen(s); - if (pLexBuffer - len < pLexBufferRealStart) + pLexBuffer -= len; + + if (pLexBuffer < pLexBufferRealStart) fatalerror("Buffer safety margin exceeded"); - for (i = len - 1; i >= 0; i--) - *(--pLexBuffer) = s[i]; + memcpy(pLexBuffer, s, len); } void yy_switch_to_buffer(YY_BUFFER_STATE buf) From e0e8170fe6bbbb786e407a9a95f5848ab10a1f26 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Sat, 31 Aug 2019 15:36:58 +0200 Subject: [PATCH 3/6] Add recursion limit for string expansions Unlike macros, REPTs and INCLUDEs, this recursion depth is independent. This is intentional, because string expansions work very differently. While it's easy to know when a string expansion begins, checking where it ends is much more complicated, since the expansion's contents are simply injected back into the lex buffer. Therefore, the depth has to be checked after lexing took place. Because of this, the placement of the expansion end check is somewhat haphazard, but I think it's good. While I have no certainty, all tests ended with all expansions properly ended, and I couldn't find any pitfalls. Finally, `pCurrentStringExpansion` has been made global so error printing can use it to tell the user if an error occurred inside of an expansion. --- include/asm/fstack.h | 2 +- include/asm/lexer.h | 9 ++++++++ src/asm/fstack.c | 6 ++--- src/asm/globlex.c | 2 ++ src/asm/lexer.c | 53 ++++++++++++++++++++++++++++++++++++++++++-- src/asm/main.c | 4 ++-- 6 files changed, 68 insertions(+), 8 deletions(-) diff --git a/include/asm/fstack.h b/include/asm/fstack.h index f3e8a878..627c8325 100644 --- a/include/asm/fstack.h +++ b/include/asm/fstack.h @@ -35,7 +35,7 @@ struct sContext { uint32_t nREPTBlockSize; }; -extern unsigned int nMaxFileStackDepth; +extern unsigned int nMaxRecursionDepth; void fstk_RunInclude(char *tzFileName); void fstk_RunMacroArg(int32_t s); diff --git a/include/asm/lexer.h b/include/asm/lexer.h index 3a86a773..c4e845b5 100644 --- a/include/asm/lexer.h +++ b/include/asm/lexer.h @@ -40,6 +40,13 @@ enum eLexerState { LEX_STATE_MACROARGS }; +struct sStringExpansionPos { + char *tzName; + char *pBuffer; + char *pBufferPos; + struct sStringExpansionPos *pParent; +}; + #define INITIAL 0 #define macroarg 3 @@ -62,6 +69,7 @@ void lex_FloatDeleteSecondRange(uint32_t id, uint16_t start, uint16_t end); void lex_Init(void); void lex_AddStrings(const struct sLexInitString *lex); void lex_SetBuffer(char *buffer, uint32_t len); +void lex_BeginStringExpansion(const char *tzName); int yywrap(void); int yylex(void); void yyunput(char c); @@ -70,6 +78,7 @@ void yyskipbytes(uint32_t count); void yyunputbytes(uint32_t count); extern YY_BUFFER_STATE pCurrentBuffer; +extern struct sStringExpansionPos *pCurrentStringExpansion; void upperstring(char *s); void lowerstring(char *s); diff --git a/src/asm/fstack.c b/src/asm/fstack.c index b471cdd6..fbe1459c 100644 --- a/src/asm/fstack.c +++ b/src/asm/fstack.c @@ -29,7 +29,7 @@ static struct sContext *pFileStack; static unsigned int nFileStackDepth; -unsigned int nMaxFileStackDepth; +unsigned int nMaxRecursionDepth; static struct sSymbol *pCurrentMacro; static YY_BUFFER_STATE CurrentFlexHandle; static FILE *pCurrentFile; @@ -62,8 +62,8 @@ static void pushcontext(void) { struct sContext **ppFileStack; - if (++nFileStackDepth > nMaxFileStackDepth) - fatalerror("Recursion limit (%d) exceeded", nMaxFileStackDepth); + if (++nFileStackDepth > nMaxRecursionDepth) + fatalerror("Recursion limit (%d) exceeded", nMaxRecursionDepth); ppFileStack = &pFileStack; while (*ppFileStack) diff --git a/src/asm/globlex.c b/src/asm/globlex.c index 98786a34..066cd9d1 100644 --- a/src/asm/globlex.c +++ b/src/asm/globlex.c @@ -279,6 +279,8 @@ uint32_t ParseSymbol(char *src, uint32_t size) if (!oDontExpandStrings && sym_isString(dest)) { char *s; + lex_BeginStringExpansion(dest); + /* Feed the symbol's contents into the buffer */ yyunputstr(s = sym_GetStringValue(dest)); diff --git a/src/asm/lexer.c b/src/asm/lexer.c index cdaaaf55..212a1f06 100644 --- a/src/asm/lexer.c +++ b/src/asm/lexer.c @@ -51,6 +51,9 @@ uint32_t tFloatingChars[256]; uint32_t nFloating; enum eLexerState lexerstate = LEX_STATE_NORMAL; +struct sStringExpansionPos *pCurrentStringExpansion; +static unsigned int nNbStringExpansions; + /* UTF-8 byte order mark */ static const unsigned char bom[BOM_SIZE] = { 0xEF, 0xBB, 0xBF }; @@ -102,6 +105,31 @@ void yyunputstr(const char *s) memcpy(pLexBuffer, s, len); } +/* + * Marks that a new string expansion with name `tzName` ends here + * Enforces recursion depth + */ +void lex_BeginStringExpansion(const char *tzName) +{ + if (++nNbStringExpansions > nMaxRecursionDepth) + fatalerror("Recursion limit (%d) exceeded", nMaxRecursionDepth); + + struct sStringExpansionPos *pNewStringExpansion = + malloc(sizeof(*pNewStringExpansion)); + char *tzNewExpansionName = strdup(tzName); + + if (!pNewStringExpansion || !tzNewExpansionName) + fatalerror("Could not allocate memory to expand '%s'", + tzName); + + pNewStringExpansion->tzName = tzNewExpansionName; + pNewStringExpansion->pBuffer = pLexBufferRealStart; + pNewStringExpansion->pBufferPos = pLexBuffer; + pNewStringExpansion->pParent = pCurrentStringExpansion; + + pCurrentStringExpansion = pNewStringExpansion; +} + void yy_switch_to_buffer(YY_BUFFER_STATE buf) { pCurrentBuffer = buf; @@ -424,6 +452,9 @@ void lex_Init(void) nLexMaxLength = 0; nFloating = 0; + + pCurrentStringExpansion = NULL; + nNbStringExpansions = 0; } void lex_AddStrings(const struct sLexInitString *lex) @@ -968,12 +999,30 @@ static uint32_t yylex_MACROARGS(void) int yylex(void) { + int returnedChar; switch (lexerstate) { case LEX_STATE_NORMAL: - return yylex_NORMAL(); + returnedChar = yylex_NORMAL(); + break; case LEX_STATE_MACROARGS: - return yylex_MACROARGS(); + returnedChar = yylex_MACROARGS(); + break; default: fatalerror("%s: Internal error.", __func__); } + + /* Check if string expansions were fully read */ + while (pCurrentStringExpansion + && pCurrentStringExpansion->pBuffer == pLexBufferRealStart + && pCurrentStringExpansion->pBufferPos <= pLexBuffer) { + struct sStringExpansionPos *pParent = + pCurrentStringExpansion->pParent; + free(pCurrentStringExpansion->tzName); + free(pCurrentStringExpansion); + + pCurrentStringExpansion = pParent; + nNbStringExpansions--; + } + + return returnedChar; } diff --git a/src/asm/main.c b/src/asm/main.c index 0d928e92..a1829347 100644 --- a/src/asm/main.c +++ b/src/asm/main.c @@ -315,7 +315,7 @@ int main(int argc, char *argv[]) /* yydebug=1; */ - nMaxFileStackDepth = 64; + nMaxRecursionDepth = 64; DefaultOptions.gbgfx[0] = '0'; DefaultOptions.gbgfx[1] = '1'; @@ -389,7 +389,7 @@ int main(int argc, char *argv[]) break; case 'r': - nMaxFileStackDepth = strtoul(optarg, &ep, 0); + nMaxRecursionDepth = strtoul(optarg, &ep, 0); if (optarg[0] == '\0' || *ep != '\0') errx(1, "Invalid argument for option 'r'"); From f9a04696f26debc73adac44642bb129bf2f9c70a Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Sat, 31 Aug 2019 17:22:43 +0200 Subject: [PATCH 4/6] Add recursion overflow tests --- test/asm/equs-recursion.asm | 2 ++ test/asm/equs-recursion.out | 2 ++ test/asm/equs-recursion.out.pipe | 2 ++ test/asm/include-recursion.asm | 1 + test/asm/include-recursion.out | 2 ++ test/asm/include-recursion.out.pipe | 2 ++ test/asm/macro-recursion.asm | 4 ++++ test/asm/macro-recursion.out | 2 ++ test/asm/macro-recursion.out.pipe | 2 ++ 9 files changed, 19 insertions(+) create mode 100644 test/asm/equs-recursion.asm create mode 100644 test/asm/equs-recursion.out create mode 100644 test/asm/equs-recursion.out.pipe create mode 100644 test/asm/include-recursion.asm create mode 100644 test/asm/include-recursion.out create mode 100644 test/asm/include-recursion.out.pipe create mode 100644 test/asm/macro-recursion.asm create mode 100644 test/asm/macro-recursion.out create mode 100644 test/asm/macro-recursion.out.pipe diff --git a/test/asm/equs-recursion.asm b/test/asm/equs-recursion.asm new file mode 100644 index 00000000..794f4960 --- /dev/null +++ b/test/asm/equs-recursion.asm @@ -0,0 +1,2 @@ +recurse EQUS "recurse" +recurse \ No newline at end of file diff --git a/test/asm/equs-recursion.out b/test/asm/equs-recursion.out new file mode 100644 index 00000000..71895a5d --- /dev/null +++ b/test/asm/equs-recursion.out @@ -0,0 +1,2 @@ +ERROR: equs-recursion.asm(2): + Recursion limit (64) exceeded diff --git a/test/asm/equs-recursion.out.pipe b/test/asm/equs-recursion.out.pipe new file mode 100644 index 00000000..c6444b6f --- /dev/null +++ b/test/asm/equs-recursion.out.pipe @@ -0,0 +1,2 @@ +ERROR: -(2): + Recursion limit (64) exceeded diff --git a/test/asm/include-recursion.asm b/test/asm/include-recursion.asm new file mode 100644 index 00000000..9b6a920b --- /dev/null +++ b/test/asm/include-recursion.asm @@ -0,0 +1 @@ +INCLUDE "include-recursion.asm" diff --git a/test/asm/include-recursion.out b/test/asm/include-recursion.out new file mode 100644 index 00000000..8a256e6b --- /dev/null +++ b/test/asm/include-recursion.out @@ -0,0 +1,2 @@ +ERROR: include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1): + Recursion limit (64) exceeded diff --git a/test/asm/include-recursion.out.pipe b/test/asm/include-recursion.out.pipe new file mode 100644 index 00000000..856f8784 --- /dev/null +++ b/test/asm/include-recursion.out.pipe @@ -0,0 +1,2 @@ +ERROR: -(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1) -> include-recursion.asm(1): + Recursion limit (64) exceeded diff --git a/test/asm/macro-recursion.asm b/test/asm/macro-recursion.asm new file mode 100644 index 00000000..25f854e9 --- /dev/null +++ b/test/asm/macro-recursion.asm @@ -0,0 +1,4 @@ +recurse: MACRO + recurse +ENDM + recurse \ No newline at end of file diff --git a/test/asm/macro-recursion.out b/test/asm/macro-recursion.out new file mode 100644 index 00000000..beb954a1 --- /dev/null +++ b/test/asm/macro-recursion.out @@ -0,0 +1,2 @@ +ERROR: macro-recursion.asm(4) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1): + Recursion limit (64) exceeded diff --git a/test/asm/macro-recursion.out.pipe b/test/asm/macro-recursion.out.pipe new file mode 100644 index 00000000..ae11d64f --- /dev/null +++ b/test/asm/macro-recursion.out.pipe @@ -0,0 +1,2 @@ +ERROR: -(4) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1): + Recursion limit (64) exceeded From 3cc67c48cf89af221291761611269b37d9b22641 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Sat, 31 Aug 2019 17:34:54 +0200 Subject: [PATCH 5/6] Add recursion limit info to man and help --- src/asm/main.c | 3 ++- src/asm/rgbasm.1 | 3 +++ src/asm/rgbasm.5 | 3 +-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/asm/main.c b/src/asm/main.c index a1829347..987e15ce 100644 --- a/src/asm/main.c +++ b/src/asm/main.c @@ -288,7 +288,8 @@ static void print_usage(void) { printf( "usage: rgbasm [-EhLVvw] [-b chars] [-Dname[=value]] [-g chars] [-i path]\n" -" [-M dependfile] [-o outfile] [-p pad_value] file.asm\n"); +" [-M dependfile] [-o outfile] [-p pad_value]\n" +" [-r recursion_depth] file.asm\n"); exit(1); } diff --git a/src/asm/rgbasm.1 b/src/asm/rgbasm.1 index c81eae3c..027853b7 100644 --- a/src/asm/rgbasm.1 +++ b/src/asm/rgbasm.1 @@ -21,6 +21,7 @@ .Op Fl M Ar dependfile .Op Fl o Ar outfile .Op Fl p Ar pad_value +.Op Fl r Ar recursion_depth .Ar file .Sh DESCRIPTION The @@ -77,6 +78,8 @@ Write an object file to the given filename. .It Fl p Ar pad_value When padding an image, pad with this value. The default is 0x00. +.It Fl r Ar recursion_depth +Specifies the recursion depth at which RGBASM will assume being in an infinite loop. .It Fl V Print the version of the program and exit. .It Fl v diff --git a/src/asm/rgbasm.5 b/src/asm/rgbasm.5 index babe827c..5917d299 100644 --- a/src/asm/rgbasm.5 +++ b/src/asm/rgbasm.5 @@ -455,8 +455,7 @@ String equates can't be exported or imported. .Sy Important note : An EQUS can be expanded to a string that contains another EQUS and it will be expanded as well. -This means that, if you aren't careful, you may trap the assembler into an -infinite loop if there's a circular dependency in the expansions. +If this creates an infinite loop, RGBASM will error out once a certain depth is reached. See the -r command-line option. Also, a MACRO can have inside an EQUS which references the same MACRO, which has the same problem. .Pp From 476ccc9f6bc0d292f0336132670de0dfacee7aef Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Mon, 2 Sep 2019 02:09:59 +0200 Subject: [PATCH 6/6] Fix undefined behavior in yyunputstr Refer to comment at lexer.c:100 for more info --- src/asm/lexer.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/asm/lexer.c b/src/asm/lexer.c index 212a1f06..ae7781bb 100644 --- a/src/asm/lexer.c +++ b/src/asm/lexer.c @@ -97,11 +97,17 @@ void yyunputstr(const char *s) len = strlen(s); - pLexBuffer -= len; - - if (pLexBuffer < pLexBufferRealStart) + /* + * It would be undefined behavior to subtract `len` from pLexBuffer and + * potentially have it point outside of pLexBufferRealStart's buffer, + * this is why the check is done this way. + * Refer to https://github.com/rednex/rgbds/pull/411#discussion_r319779797 + */ + if (pLexBuffer - pLexBufferRealStart < len) fatalerror("Buffer safety margin exceeded"); + pLexBuffer -= len; + memcpy(pLexBuffer, s, len); }