From 2e3db9d56ac9ebd6daee9d411764c5011dc2242a Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Sat, 3 Oct 2020 21:33:30 +0200 Subject: [PATCH] Clean up label generation Only check for localness when we already know we have a local --- include/asm/symbol.h | 9 +- src/asm/asmy.y | 49 ++++------ src/asm/section.c | 30 +++---- src/asm/symbol.c | 170 +++++++++++++++++++---------------- test/asm/label-macro-arg.err | 2 +- test/asm/sym-scope.asm | 21 +++++ test/asm/sym-scope.err | 9 ++ test/asm/sym-scope.out | 0 8 files changed, 161 insertions(+), 129 deletions(-) create mode 100644 test/asm/sym-scope.asm create mode 100644 test/asm/sym-scope.err create mode 100644 test/asm/sym-scope.out diff --git a/include/asm/symbol.h b/include/asm/symbol.h index 6d2c7e53..52f61a52 100644 --- a/include/asm/symbol.h +++ b/include/asm/symbol.h @@ -34,7 +34,6 @@ struct Symbol { enum SymbolType type; bool isExported; /* Whether the symbol is to be exported */ bool isBuiltin; /* Whether the symbol is a built-in */ - struct Symbol const *scope; struct Section *section; char fileName[_MAX_PATH + 1]; /* File where the symbol was defined. */ uint32_t fileLine; /* Line where the symbol was defined. */ @@ -109,8 +108,8 @@ void sym_ForEach(void (*func)(struct Symbol *, void *), void *arg); int32_t sym_GetValue(struct Symbol const *sym); void sym_SetExportAll(bool set); -struct Symbol *sym_AddLocalReloc(char const *symName); -struct Symbol *sym_AddReloc(char const *symName); +struct Symbol *sym_AddLocalLabel(char const *symName); +struct Symbol *sym_AddLabel(char const *symName); void sym_Export(char const *symName); struct Symbol *sym_AddEqu(char const *symName, int32_t value); struct Symbol *sym_AddSet(char const *symName, int32_t value); @@ -125,7 +124,7 @@ void sym_Purge(char const *symName); void sym_Init(void); /* Functions to save and restore the current symbol scope. */ -struct Symbol *sym_GetCurrentSymbolScope(void); -void sym_SetCurrentSymbolScope(struct Symbol *newScope); +char const *sym_GetCurrentSymbolScope(void); +void sym_SetCurrentSymbolScope(char const *newScope); #endif /* RGBDS_SYMBOL_H */ diff --git a/src/asm/asmy.y b/src/asm/asmy.y index e09a0ada..50ef200a 100644 --- a/src/asm/asmy.y +++ b/src/asm/asmy.y @@ -562,8 +562,6 @@ static inline void failAssertMsg(enum AssertionType type, char const *msg) %left NEG /* negation -- unary minus */ %token T_LABEL -%type scoped_label -%type scoped_label_bare %token T_ID %token T_LOCAL_ID %type scoped_id @@ -674,41 +672,28 @@ line : label | pseudoop ; -scoped_label_bare : T_LABEL { - warning(WARNING_OBSOLETE, "Non-local labels without a colon are deprecated\n"); - strcpy($$, $1); - } - | T_LOCAL_ID { - strcpy($$, $1); - } -; -scoped_label : T_LABEL ':' { - strcpy($$, $1); - } - | T_LOCAL_ID ':' { - strcpy($$, $1); - } -; scoped_id : T_ID | T_LOCAL_ID ; label : /* empty */ - | scoped_label_bare { - if ($1[0] == '.') - sym_AddLocalReloc($1); - else - sym_AddReloc($1); + | T_LOCAL_ID { + sym_AddLocalLabel($1); } - | scoped_label { - if ($1[0] == '.') - sym_AddLocalReloc($1); - else - sym_AddReloc($1); + | T_LABEL { + warning(WARNING_OBSOLETE, "Non-local labels without a colon are deprecated\n"); + sym_AddLabel($1); } - | scoped_label ':' { - if ($1[0] == '.') - sym_AddLocalReloc($1); - else - sym_AddReloc($1); + | T_LOCAL_ID ':' { + sym_AddLocalLabel($1); + } + | T_LABEL ':' { + sym_AddLabel($1); + } + | T_LOCAL_ID ':' ':' { + sym_AddLocalLabel($1); + sym_Export($1); + } + | T_LABEL ':' ':' { + sym_AddLabel($1); sym_Export($1); } ; diff --git a/src/asm/section.c b/src/asm/section.c index e20d382e..3658dd67 100644 --- a/src/asm/section.c +++ b/src/asm/section.c @@ -17,13 +17,13 @@ #include "platform.h" // strdup struct SectionStackEntry { - struct Section *pSection; - struct Symbol *pScope; /* Section's symbol scope */ + struct Section *section; + char const *scope; /* Section's symbol scope */ uint32_t offset; struct SectionStackEntry *next; }; -struct SectionStackEntry *pSectionStack; +struct SectionStackEntry *sectionStack; uint32_t curOffset; /* Offset into the current section (see sect_GetSymbolOffset) */ static struct Section *currentLoadSection = NULL; uint32_t loadOffset; /* The offset of the LOAD section within its parent */ @@ -774,23 +774,21 @@ void out_BinaryFileSlice(char const *s, int32_t start_pos, int32_t length) */ void out_PushSection(void) { - struct SectionStackEntry *sect; + struct SectionStackEntry *sect = malloc(sizeof(*sect)); - sect = malloc(sizeof(struct SectionStackEntry)); if (sect == NULL) - fatalerror("No memory for section stack: %s<\n", strerror(errno)); - - sect->pSection = pCurrentSection; - sect->pScope = sym_GetCurrentSymbolScope(); + fatalerror("No memory for section stack: %s\n", strerror(errno)); + sect->section = pCurrentSection; + sect->scope = sym_GetCurrentSymbolScope(); sect->offset = curOffset; - sect->next = pSectionStack; - pSectionStack = sect; + sect->next = sectionStack; + sectionStack = sect; /* TODO: maybe set current section to NULL? */ } void out_PopSection(void) { - if (pSectionStack == NULL) + if (!sectionStack) fatalerror("No entries in the section stack\n"); if (currentLoadSection) @@ -798,12 +796,12 @@ void out_PopSection(void) struct SectionStackEntry *sect; - sect = pSectionStack; + sect = sectionStack; changeSection(); - pCurrentSection = sect->pSection; - sym_SetCurrentSymbolScope(sect->pScope); + pCurrentSection = sect->section; + sym_SetCurrentSymbolScope(sect->scope); curOffset = sect->offset; - pSectionStack = sect->next; + sectionStack = sect->next; free(sect); } diff --git a/src/asm/symbol.c b/src/asm/symbol.c index 535427d4..d7d219e6 100644 --- a/src/asm/symbol.c +++ b/src/asm/symbol.c @@ -36,7 +36,7 @@ HashMap symbols; -static struct Symbol *symbolScope; /* Current section symbol scope */ +static char const *labelScope; /* Current section's label scope */ static struct Symbol *PCSymbol; static char savedTIME[256]; static char savedDATE[256]; @@ -133,7 +133,6 @@ static struct Symbol *createsymbol(char const *s) symbol->isExported = false; symbol->isBuiltin = false; - symbol->scope = NULL; symbol->section = NULL; updateSymbolFilename(symbol); symbol->ID = -1; @@ -148,19 +147,20 @@ static struct Symbol *createsymbol(char const *s) * the name with the parent symbol's name. */ static void fullSymbolName(char *output, size_t outputSize, - char const *localName, const struct Symbol *scope) + char const *localName, char const *scopeName) { - const struct Symbol *parent = scope->scope ? scope->scope : scope; - int n = snprintf(output, outputSize, "%s%s", parent->name, localName); + int ret = snprintf(output, outputSize, "%s%s", scopeName, localName); - if (n >= (int)outputSize) - fatalerror("Symbol name is too long: '%s%s'\n", parent->name, localName); + if (ret < 0) + fatalerror("snprintf error when expanding symbol name: %s", strerror(errno)); + else if ((size_t)ret >= outputSize) + fatalerror("Symbol name is too long: '%s%s'\n", scopeName, localName); } /* * Find a symbol by name and scope */ -static struct Symbol *findsymbol(char const *s, struct Symbol const *scope) +static struct Symbol *findsymbol(char const *s, char const *scope) { char fullname[MAXSYMLEN + 1]; @@ -182,7 +182,7 @@ static struct Symbol *findsymbol(char const *s, struct Symbol const *scope) */ struct Symbol *sym_FindSymbol(char const *symName) { - return findsymbol(symName, symName[0] == '.' ? symbolScope : NULL); + return findsymbol(symName, symName[0] == '.' ? labelScope : NULL); } static inline bool isReferenced(struct Symbol const *sym) @@ -195,8 +195,7 @@ static inline bool isReferenced(struct Symbol const *sym) */ void sym_Purge(char const *symName) { - struct Symbol *scope = symName[0] == '.' ? symbolScope : NULL; - struct Symbol *symbol = findsymbol(symName, scope); + struct Symbol *symbol = sym_FindSymbol(symName); if (!symbol) { error("'%s' not defined\n", symName); @@ -205,6 +204,10 @@ void sym_Purge(char const *symName) } else if (isReferenced(symbol)) { error("Symbol \"%s\" is referenced and thus cannot be purged\n", symName); } else { + /* Do not keep a reference to the label's name after purging it */ + if (symbol->name == labelScope) + labelScope = NULL; + hash_RemoveElement(symbols, symbol->name); if (symbol->type == SYM_MACRO) free(symbol->macro); @@ -261,14 +264,14 @@ uint32_t sym_GetDefinedValue(char const *s) return 0; } -struct Symbol *sym_GetCurrentSymbolScope(void) +char const *sym_GetCurrentSymbolScope(void) { - return symbolScope; + return labelScope; } -void sym_SetCurrentSymbolScope(struct Symbol *newScope) +void sym_SetCurrentSymbolScope(char const *newScope) { - symbolScope = newScope; + labelScope = newScope; } /* @@ -358,74 +361,94 @@ struct Symbol *sym_AddSet(char const *symName, int32_t value) } /* - * Add a local (.name) relocatable symbol + * Add a label (aka "relocatable symbol") + * @param name The label's full name (so `.name` is invalid) + * @return The created symbol */ -struct Symbol *sym_AddLocalReloc(char const *symName) +static struct Symbol *addSectionlessLabel(char const *name) { - if (!symbolScope) { - error("Local label '%s' in main scope\n", symName); + assert(name[0] != '.'); /* The symbol name must have been expanded prior */ + struct Symbol *sym = findsymbol(name, NULL); /* Due to this, don't look for expansions */ + + if (!sym) { + sym = createsymbol(name); + } else if (sym_IsDefined(sym)) { + error("'%s' already defined in %s(%" PRIu32 ")\n", + name, sym->fileName, sym->fileLine); + return NULL; + } + /* If the symbol already exists as a ref, just "take over" it */ + sym->type = SYM_LABEL; + sym->callback = NULL; + sym->value = sect_GetSymbolOffset(); + if (exportall) + sym->isExported = true; + sym->section = sect_GetSymbolSection(); + updateSymbolFilename(sym); + + return sym; +} + +static struct Symbol *addLabel(char const *name) +{ + struct Symbol *sym = addSectionlessLabel(name); + + if (sym && !sym->section) + error("Label \"%s\" created outside of a SECTION\n", name); + return sym; +} + +/* + * Add a local (.name or Parent.name) relocatable symbol + */ +struct Symbol *sym_AddLocalLabel(char const *name) +{ + if (!labelScope) { + error("Local label '%s' in main scope\n", name); return NULL; } char fullname[MAXSYMLEN + 1]; - fullSymbolName(fullname, sizeof(fullname), symName, symbolScope); - return sym_AddReloc(fullname); + if (name[0] == '.') { + /* If symbol is of the form `.name`, expand to the full `Parent.name` name */ + fullSymbolName(fullname, sizeof(fullname), name, labelScope); + name = fullname; /* Use the expanded name instead */ + } else { + size_t i = 0; + + /* Otherwise, check that `Parent` is in fact the current scope */ + while (labelScope[i] && name[i] == labelScope[i]) + i++; + /* Assuming no dots in `labelScope` */ + assert(strchr(&name[i], '.')); /* There should be at least one dot, though */ + size_t parentLen = i + (strchr(&name[i], '.') - name); + + /* + * Check that `labelScope[i]` ended the check, guaranteeing that `name` is at least + * as long, and then that this was the entirety of the `Parent` part of `name`. + */ + if (labelScope[i] != '\0' || name[i] != '.') + error("Not currently in the scope of '%.*s'\n", parentLen, name); + if (strchr(&name[parentLen + 1], '.')) /* There will at least be a terminator */ + fatalerror("'%s' is a nonsensical reference to a nested local label\n", + name); + } + + return addLabel(name); } /* * Add a relocatable symbol */ -struct Symbol *sym_AddReloc(char const *symName) +struct Symbol *sym_AddLabel(char const *name) { - struct Symbol const *scope = NULL; - char *localPtr = strchr(symName, '.'); - - if (localPtr != NULL) { - if (!symbolScope) { - error("Local label in main scope\n"); - return NULL; - } - - scope = symbolScope->scope ? symbolScope->scope : symbolScope; - uint32_t parentLen = localPtr - symName; - - if (strchr(localPtr + 1, '.') != NULL) - fatalerror("'%s' is a nonsensical reference to a nested local symbol\n", - symName); - else if (strlen(scope->name) != parentLen - || strncmp(symName, scope->name, parentLen) != 0) - error("Not currently in the scope of '%.*s'\n", parentLen, symName); - } - - struct Symbol *sym = findsymbol(symName, scope); - - if (!sym) - sym = createsymbol(symName); - else if (sym_IsDefined(sym)) - error("'%s' already defined in %s(%" PRIu32 ")\n", - symName, sym->fileName, sym->fileLine); - /* If the symbol already exists as a ref, just "take over" it */ - - sym->type = SYM_LABEL; - sym->callback = NULL; - sym->value = sect_GetSymbolOffset(); - - if (exportall) - sym->isExported = true; - - sym->scope = scope; - sym->section = sect_GetSymbolSection(); - /* Labels need to be assigned a section, except PC */ - if (!sym->section && strcmp(symName, "@")) - error("Label \"%s\" created outside of a SECTION\n", symName); - - updateSymbolFilename(sym); + struct Symbol *sym = addLabel(name); /* Set the symbol as the new scope */ - /* TODO: don't do this for local labels */ - symbolScope = findsymbol(symName, scope); - return symbolScope; + if (sym) + labelScope = sym->name; + return sym; } /* @@ -471,19 +494,16 @@ struct Symbol *sym_Ref(char const *symName) if (nsym == NULL) { char fullname[MAXSYMLEN + 1]; - struct Symbol const *scope = NULL; if (symName[0] == '.') { - if (!symbolScope) + if (!labelScope) fatalerror("Local label reference '%s' in main scope\n", symName); - scope = symbolScope->scope ? symbolScope->scope : symbolScope; - fullSymbolName(fullname, sizeof(fullname), symName, symbolScope); + fullSymbolName(fullname, sizeof(fullname), symName, labelScope); symName = fullname; } nsym = createsymbol(symName); nsym->type = SYM_REF; - nsym->scope = scope; } return nsym; @@ -516,7 +536,7 @@ void sym_Init(void) struct Symbol *_NARGSymbol = sym_AddEqu("_NARG", 0); struct Symbol *__LINE__Symbol = sym_AddEqu("__LINE__", 0); - PCSymbol = sym_AddReloc("@"), + PCSymbol = addSectionlessLabel("@"); PCSymbol->isBuiltin = true; PCSymbol->callback = CallbackPC; _NARGSymbol->isBuiltin = true; @@ -571,7 +591,7 @@ void sym_Init(void) addString("__UTC_SECOND__", removeLeadingZeros(savedSECOND)); #undef addString - symbolScope = NULL; + labelScope = NULL; math_DefinePI(); } diff --git a/test/asm/label-macro-arg.err b/test/asm/label-macro-arg.err index 9350062d..70c50a22 100644 --- a/test/asm/label-macro-arg.err +++ b/test/asm/label-macro-arg.err @@ -1,5 +1,5 @@ ERROR: label-macro-arg.asm(44) -> label-macro-arg.asm::test_char(31): - Local label in main scope + Local label 'sizeof_.something' in main scope while expanding symbol "VAR_DEF" ERROR: label-macro-arg.asm(44) -> label-macro-arg.asm::test_char(31): syntax error diff --git a/test/asm/sym-scope.asm b/test/asm/sym-scope.asm new file mode 100644 index 00000000..2a704d34 --- /dev/null +++ b/test/asm/sym-scope.asm @@ -0,0 +1,21 @@ +SECTION "Scopes", ROM0 + +; Neither of these should be created +.tooSoon +Nice.try + +Parent: +.loc + dw Parent.loc ; This is what it should expand to +Parent.explicit + dw .explicit ; This should expand to the above + + +; None of the two locals below should manage to be created, being in the wrong scopes +; Note however that `Parentheses` begins with `Parent`, which string checks may fail to handle + +Parentheses.check + +Parentheses: + +Parent.check diff --git a/test/asm/sym-scope.err b/test/asm/sym-scope.err new file mode 100644 index 00000000..e77f43bc --- /dev/null +++ b/test/asm/sym-scope.err @@ -0,0 +1,9 @@ +ERROR: sym-scope.asm(4): + Local label '.tooSoon' in main scope +ERROR: sym-scope.asm(5): + Local label 'Nice.try' in main scope +ERROR: sym-scope.asm(17): + Not currently in the scope of 'Parentheses.check' +ERROR: sym-scope.asm(21): + Not currently in the scope of 'Parent.check' +error: Assembly aborted (4 errors)! diff --git a/test/asm/sym-scope.out b/test/asm/sym-scope.out new file mode 100644 index 00000000..e69de29b