From 37089ef940a25e04f263233d5436b2d8b7ef3f70 Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Thu, 29 Aug 2019 17:03:50 +0200 Subject: [PATCH] Improve error stack The old error stack was fairly obtuse and hard to use for debugging. This improves it notably by ensuring all line numbers are relative to the file, and not, say, the macro definition. This is a breaking change if you were parsing the old stack, but the change should be painless, and the new stack only brings more info. The syntax is unchanged for files, macros see their name prefixed with the file they're defined in and a pair of colors, REPT blocks simply append a '::REPT~n' to the context they're in, where 'n' is the number of iterations the REPT has done. This is especially helpful in macro-heavy code such as rgbds-structs. --- include/asm/fstack.h | 4 +- include/asm/symbol.h | 2 +- src/asm/asmy.y | 6 ++- src/asm/fstack.c | 61 ++++++++++++++++++++++++++-- src/asm/symbol.c | 7 +++- test/asm/label-macro-arg.out | 2 +- test/asm/label-macro-arg.out.pipe | 2 +- test/asm/label-redefinition.out | 2 +- test/asm/label-redefinition.out.pipe | 2 +- test/asm/macro-recursion.out | 2 +- test/asm/macro-recursion.out.pipe | 2 +- test/asm/multiple-charmaps.out | 6 +-- test/asm/multiple-charmaps.out.pipe | 6 +-- test/asm/strsub.out | 16 ++++---- test/asm/strsub.out.pipe | 16 ++++---- 15 files changed, 100 insertions(+), 36 deletions(-) diff --git a/include/asm/fstack.h b/include/asm/fstack.h index 627c8325..98bcbfc7 100644 --- a/include/asm/fstack.h +++ b/include/asm/fstack.h @@ -33,6 +33,8 @@ struct sContext { char *pREPTBlock; uint32_t nREPTBlockCount; uint32_t nREPTBlockSize; + int32_t nREPTBodyFirstLine; + int32_t nREPTBodyLastLine; }; extern unsigned int nMaxRecursionDepth; @@ -43,7 +45,7 @@ void fstk_Init(char *s); void fstk_Dump(void); void fstk_AddIncludePath(char *s); uint32_t fstk_RunMacro(char *s); -void fstk_RunRept(uint32_t count); +void fstk_RunRept(uint32_t count, int32_t nReptLineNo); FILE *fstk_FindFile(char *fname, char **incPathUsed); int32_t fstk_GetLine(void); diff --git a/include/asm/symbol.h b/include/asm/symbol.h index cc10ab8b..cf8e6d4c 100644 --- a/include/asm/symbol.h +++ b/include/asm/symbol.h @@ -74,7 +74,7 @@ char *sym_GetStringValue(char *tzSym); void sym_UseCurrentMacroArgs(void); void sym_SetMacroArgID(uint32_t nMacroCount); uint32_t sym_isString(char *tzSym); -void sym_AddMacro(char *tzSym); +void sym_AddMacro(char *tzSym, int32_t nDefLineNo); void sym_Ref(char *tzSym); void sym_ShiftCurrentMacroArgs(void); void sym_AddString(char *tzSym, char *tzValue); diff --git a/src/asm/asmy.y b/src/asm/asmy.y index bf7c0d74..35dec3c4 100644 --- a/src/asm/asmy.y +++ b/src/asm/asmy.y @@ -816,15 +816,17 @@ shift : T_POP_SHIFT { sym_ShiftCurrentMacroArgs(); } rept : T_POP_REPT uconst { + uint32_t nDefinitionLineNo = nLineNo; copyrept(); - fstk_RunRept($2); + fstk_RunRept($2, nDefinitionLineNo); } ; macrodef : T_LABEL ':' T_POP_MACRO { + int32_t nDefinitionLineNo = nLineNo; copymacro(); - sym_AddMacro($1); + sym_AddMacro($1, nDefinitionLineNo); } ; diff --git a/src/asm/fstack.c b/src/asm/fstack.c index 8a067c4f..a4977ef8 100644 --- a/src/asm/fstack.c +++ b/src/asm/fstack.c @@ -42,6 +42,8 @@ static uint32_t nMacroCount; static char *pCurrentREPTBlock; static uint32_t nCurrentREPTBlockSize; static uint32_t nCurrentREPTBlockCount; +static int32_t nCurrentREPTBodyFirstLine; +static int32_t nCurrentREPTBodyLastLine; uint32_t ulMacroReturnValue; @@ -93,6 +95,8 @@ static void pushcontext(void) (*ppFileStack)->pREPTBlock = pCurrentREPTBlock; (*ppFileStack)->nREPTBlockSize = nCurrentREPTBlockSize; (*ppFileStack)->nREPTBlockCount = nCurrentREPTBlockCount; + (*ppFileStack)->nREPTBodyFirstLine = nCurrentREPTBodyFirstLine; + (*ppFileStack)->nREPTBodyLastLine = nCurrentREPTBodyLastLine; break; default: fatalerror("%s: Internal error.", __func__); @@ -107,6 +111,11 @@ static int32_t popcontext(void) if (nCurrentStatus == STAT_isREPTBlock) { if (--nCurrentREPTBlockCount) { + char *pREPTIterationWritePtr; + unsigned long nREPTIterationNo; + int nNbCharsWritten; + int nNbCharsLeft; + yy_delete_buffer(CurrentFlexHandle); CurrentFlexHandle = yy_scan_bytes(pCurrentREPTBlock, @@ -115,6 +124,29 @@ static int32_t popcontext(void) sym_UseCurrentMacroArgs(); sym_SetMacroArgID(nMacroCount++); sym_UseNewMacroArgs(); + + /* Increment REPT count in file path */ + pREPTIterationWritePtr = + strrchr(tzCurrentFileName, '~') + 1; + nREPTIterationNo = + strtoul(pREPTIterationWritePtr, NULL, 10); + nNbCharsLeft = sizeof(tzCurrentFileName) + - (pREPTIterationWritePtr - tzCurrentFileName); + nNbCharsWritten = snprintf(pREPTIterationWritePtr, + nNbCharsLeft, "%lu", + nREPTIterationNo + 1); + if (nNbCharsWritten >= nNbCharsLeft) { + /* + * The string is probably corrupted somehow, + * revert the change to avoid a bad error + * output. + */ + sprintf(pREPTIterationWritePtr, "%lu", + nREPTIterationNo); + fatalerror("Cannot write REPT count to file path"); + } + + nLineNo = nCurrentREPTBodyFirstLine; return 0; } } @@ -156,6 +188,9 @@ static int32_t popcontext(void) pCurrentREPTBlock = pLastFile->pREPTBlock; nCurrentREPTBlockSize = pLastFile->nREPTBlockSize; nCurrentREPTBlockCount = pLastFile->nREPTBlockCount; + nCurrentREPTBodyFirstLine = pLastFile->nREPTBodyFirstLine; + /* + 1 to account for the `ENDR` line */ + nLineNo = pLastFile->nREPTBodyLastLine + 1; break; default: fatalerror("%s: Internal error.", __func__); @@ -322,16 +357,23 @@ void fstk_RunInclude(char *tzFileName) uint32_t fstk_RunMacro(char *s) { struct sSymbol *sym = sym_FindMacro(s); + int nPrintedChars; if (sym == NULL || sym->pMacro == NULL) return 0; pushcontext(); sym_SetMacroArgID(nMacroCount++); - nLineNo = -1; + /* Minus 1 because there is a newline at the beginning of the buffer */ + nLineNo = sym->nFileLine - 1; sym_UseNewMacroArgs(); nCurrentStatus = STAT_isMacro; - strcpy(tzCurrentFileName, s); + nPrintedChars = snprintf(tzCurrentFileName, _MAX_PATH + 1, + "%s::%s", sym->tzFileName, s); + if (nPrintedChars > _MAX_PATH) { + popcontext(); + fatalerror("File name + macro name is too large to fit into buffer"); + } pCurrentMacro = sym; CurrentFlexHandle = yy_scan_bytes(pCurrentMacro->pMacro, @@ -387,9 +429,14 @@ void fstk_RunString(char *s) /* * Set up a repeat block for parsing */ -void fstk_RunRept(uint32_t count) +void fstk_RunRept(uint32_t count, int32_t nReptLineNo) { if (count) { + static const char *tzReptStr = "::REPT~1"; + + /* For error printing to make sense, fake nLineNo */ + nCurrentREPTBodyLastLine = nLineNo; + nLineNo = nReptLineNo; sym_UseCurrentMacroArgs(); pushcontext(); sym_SetMacroArgID(nMacroCount++); @@ -398,6 +445,14 @@ void fstk_RunRept(uint32_t count) nCurrentStatus = STAT_isREPTBlock; nCurrentREPTBlockSize = ulNewMacroSize; pCurrentREPTBlock = tzNewMacro; + nCurrentREPTBodyFirstLine = nReptLineNo + 1; + nLineNo = nReptLineNo; + + if (strlen(tzCurrentFileName) + strlen(tzReptStr) > _MAX_PATH) + fatalerror("Cannot append \"%s\" to file path", + tzReptStr); + strcat(tzCurrentFileName, tzReptStr); + CurrentFlexHandle = yy_scan_bytes(pCurrentREPTBlock, nCurrentREPTBlockSize); yy_switch_to_buffer(CurrentFlexHandle); diff --git a/src/asm/symbol.c b/src/asm/symbol.c index f73fb409..b62349ef 100644 --- a/src/asm/symbol.c +++ b/src/asm/symbol.c @@ -705,7 +705,7 @@ void sym_Export(char *tzSym) /* * Add a macro definition */ -void sym_AddMacro(char *tzSym) +void sym_AddMacro(char *tzSym, int32_t nDefLineNo) { struct sSymbol *nsym = createNonrelocSymbol(tzSym); @@ -715,6 +715,11 @@ void sym_AddMacro(char *tzSym) nsym->ulMacroSize = ulNewMacroSize; nsym->pMacro = tzNewMacro; updateSymbolFilename(nsym); + /* + * The symbol is created at the line after the `endm`, + * override this with the actual definition line + */ + nsym->nFileLine = nDefLineNo; } } diff --git a/test/asm/label-macro-arg.out b/test/asm/label-macro-arg.out index 1da45de9..6a76e93f 100644 --- a/test/asm/label-macro-arg.out +++ b/test/asm/label-macro-arg.out @@ -1,4 +1,4 @@ -ERROR: label-macro-arg.asm(45) -> test_char(2): +ERROR: label-macro-arg.asm(45) -> label-macro-arg.asm::test_char(31): Macro 'something' not defined $5 $6 diff --git a/test/asm/label-macro-arg.out.pipe b/test/asm/label-macro-arg.out.pipe index 383e5c89..7968df1c 100644 --- a/test/asm/label-macro-arg.out.pipe +++ b/test/asm/label-macro-arg.out.pipe @@ -1,4 +1,4 @@ -ERROR: -(45) -> test_char(2): +ERROR: -(45) -> -::test_char(31): Macro 'something' not defined $5 $6 diff --git a/test/asm/label-redefinition.out b/test/asm/label-redefinition.out index 8781cf81..09eb79b3 100644 --- a/test/asm/label-redefinition.out +++ b/test/asm/label-redefinition.out @@ -1,3 +1,3 @@ ERROR: label-redefinition.asm(7): - 'Sym' already defined in m(6) + 'Sym' already defined in label-redefinition.asm::m(6) error: Assembly aborted (1 errors)! diff --git a/test/asm/label-redefinition.out.pipe b/test/asm/label-redefinition.out.pipe index ef695055..39b21dc2 100644 --- a/test/asm/label-redefinition.out.pipe +++ b/test/asm/label-redefinition.out.pipe @@ -1,3 +1,3 @@ ERROR: -(7): - 'Sym' already defined in m(6) + 'Sym' already defined in -::m(6) error: Assembly aborted (1 errors)! diff --git a/test/asm/macro-recursion.out b/test/asm/macro-recursion.out index beb954a1..e69c4c5f 100644 --- a/test/asm/macro-recursion.out +++ b/test/asm/macro-recursion.out @@ -1,2 +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): +ERROR: macro-recursion.asm(4) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2): Recursion limit (64) exceeded diff --git a/test/asm/macro-recursion.out.pipe b/test/asm/macro-recursion.out.pipe index ae11d64f..4f4f7954 100644 --- a/test/asm/macro-recursion.out.pipe +++ b/test/asm/macro-recursion.out.pipe @@ -1,2 +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): +ERROR: -(4) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2): Recursion limit (64) exceeded diff --git a/test/asm/multiple-charmaps.out b/test/asm/multiple-charmaps.out index 5cf76894..16418b15 100644 --- a/test/asm/multiple-charmaps.out +++ b/test/asm/multiple-charmaps.out @@ -1,10 +1,10 @@ warning: multiple-charmaps.asm(75): Using 'charmap' within a section when the current charmap is 'main' is deprecated -ERROR: multiple-charmaps.asm(100) -> new_(6): +ERROR: multiple-charmaps.asm(100) -> multiple-charmaps.asm::new_(7): Charmap 'map1' already exists -ERROR: multiple-charmaps.asm(102) -> set_(2): +ERROR: multiple-charmaps.asm(102) -> multiple-charmaps.asm::set_(13): Charmap 'map5' doesn't exist -ERROR: multiple-charmaps.asm(104) -> pop_(2): +ERROR: multiple-charmaps.asm(104) -> multiple-charmaps.asm::pop_(23): No entries in the charmap stack main charmap $0 diff --git a/test/asm/multiple-charmaps.out.pipe b/test/asm/multiple-charmaps.out.pipe index 78278aa2..95bdb3a3 100644 --- a/test/asm/multiple-charmaps.out.pipe +++ b/test/asm/multiple-charmaps.out.pipe @@ -1,10 +1,10 @@ warning: -(75): Using 'charmap' within a section when the current charmap is 'main' is deprecated -ERROR: -(100) -> new_(6): +ERROR: -(100) -> -::new_(7): Charmap 'map1' already exists -ERROR: -(102) -> set_(2): +ERROR: -(102) -> -::set_(13): Charmap 'map5' doesn't exist -ERROR: -(104) -> pop_(2): +ERROR: -(104) -> -::pop_(23): No entries in the charmap stack main charmap $0 diff --git a/test/asm/strsub.out b/test/asm/strsub.out index 79eea030..a19fdc5e 100644 --- a/test/asm/strsub.out +++ b/test/asm/strsub.out @@ -1,18 +1,18 @@ -warning: strsub.asm(13) -> xstrsub(1): +warning: strsub.asm(13) -> strsub.asm::xstrsub(4): STRSUB: Length too big: 32 -warning: strsub.asm(14) -> xstrsub(1): +warning: strsub.asm(14) -> strsub.asm::xstrsub(4): STRSUB: Length too big: 300 -warning: strsub.asm(15) -> xstrsub(1): +warning: strsub.asm(15) -> strsub.asm::xstrsub(4): STRSUB: Position starts at 1 -warning: strsub.asm(15) -> xstrsub(1): +warning: strsub.asm(15) -> strsub.asm::xstrsub(4): STRSUB: Length too big: 300 -warning: strsub.asm(16) -> xstrsub(1): +warning: strsub.asm(16) -> strsub.asm::xstrsub(4): STRSUB: Position 4 is past the end of the string -warning: strsub.asm(17) -> xstrsub(1): +warning: strsub.asm(17) -> strsub.asm::xstrsub(4): STRSUB: Position 4 is past the end of the string -warning: strsub.asm(17) -> xstrsub(1): +warning: strsub.asm(17) -> strsub.asm::xstrsub(4): STRSUB: Length too big: 1 -warning: strsub.asm(20) -> xstrsub(1): +warning: strsub.asm(20) -> strsub.asm::xstrsub(4): STRSUB: Length too big: 10 A B diff --git a/test/asm/strsub.out.pipe b/test/asm/strsub.out.pipe index 15e474cf..0ff2b1c7 100644 --- a/test/asm/strsub.out.pipe +++ b/test/asm/strsub.out.pipe @@ -1,18 +1,18 @@ -warning: -(13) -> xstrsub(1): +warning: -(13) -> -::xstrsub(4): STRSUB: Length too big: 32 -warning: -(14) -> xstrsub(1): +warning: -(14) -> -::xstrsub(4): STRSUB: Length too big: 300 -warning: -(15) -> xstrsub(1): +warning: -(15) -> -::xstrsub(4): STRSUB: Position starts at 1 -warning: -(15) -> xstrsub(1): +warning: -(15) -> -::xstrsub(4): STRSUB: Length too big: 300 -warning: -(16) -> xstrsub(1): +warning: -(16) -> -::xstrsub(4): STRSUB: Position 4 is past the end of the string -warning: -(17) -> xstrsub(1): +warning: -(17) -> -::xstrsub(4): STRSUB: Position 4 is past the end of the string -warning: -(17) -> xstrsub(1): +warning: -(17) -> -::xstrsub(4): STRSUB: Length too big: 1 -warning: -(20) -> xstrsub(1): +warning: -(20) -> -::xstrsub(4): STRSUB: Length too big: 10 A B