From 1f12a06f7cc2ae79a800fe3faed727513364091b Mon Sep 17 00:00:00 2001 From: Piotr Fusik Date: Mon, 13 Feb 2017 19:41:05 +0100 Subject: [PATCH 1/8] Disallow global variable declarations with an initializer. E.g. extern int i = 42; --- src/cc65/compile.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/cc65/compile.c b/src/cc65/compile.c index 9f1ab29f5..f01ddf6c5 100644 --- a/src/cc65/compile.c +++ b/src/cc65/compile.c @@ -144,17 +144,14 @@ static void Parse (void) ** ** - if it is not a typedef or function, ** - if we don't had a storage class given ("int i") - ** - if the storage class is explicitly specified as static, - ** - or if there is an initialization. + ** or the storage class is explicitly specified as static. ** ** This means that "extern int i;" will not get storage allocated. */ if ((Decl.StorageClass & SC_FUNC) != SC_FUNC && (Decl.StorageClass & SC_TYPEMASK) != SC_TYPEDEF && ((Spec.Flags & DS_DEF_STORAGE) != 0 || - (Decl.StorageClass & (SC_EXTERN|SC_STATIC)) == SC_STATIC || - ((Decl.StorageClass & SC_EXTERN) != 0 && - CurTok.Tok == TOK_ASSIGN))) { + (Decl.StorageClass & (SC_EXTERN|SC_STATIC)) == SC_STATIC)) { /* We will allocate storage */ Decl.StorageClass |= SC_STORAGE | SC_DEF; From 730d01a25f3440bd5bdca3a3cf8068287ff52f6a Mon Sep 17 00:00:00 2001 From: Piotr Fusik Date: Mon, 13 Feb 2017 21:04:45 +0100 Subject: [PATCH 2/8] Global uninitialized variable is only a tentative definition. Closes #204 --- src/cc65/compile.c | 55 +++++++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/src/cc65/compile.c b/src/cc65/compile.c index f01ddf6c5..85a2e9939 100644 --- a/src/cc65/compile.c +++ b/src/cc65/compile.c @@ -154,7 +154,7 @@ static void Parse (void) (Decl.StorageClass & (SC_EXTERN|SC_STATIC)) == SC_STATIC)) { /* We will allocate storage */ - Decl.StorageClass |= SC_STORAGE | SC_DEF; + Decl.StorageClass |= SC_STORAGE; } /* If this is a function declarator that is not followed by a comma @@ -187,6 +187,9 @@ static void Parse (void) /* Allow initialization */ if (CurTok.Tok == TOK_ASSIGN) { + /* This is a definition */ + Entry->Flags |= SC_DEF; + /* We cannot initialize types of unknown size, or ** void types in ISO modes. */ @@ -234,18 +237,14 @@ static void Parse (void) Entry->Flags &= ~(SC_STORAGE | SC_DEF); } - /* Allocate storage if it is still needed */ - if (Entry->Flags & SC_STORAGE) { - - /* Switch to the BSS segment */ - g_usebss (); - - /* Define a label */ - g_defgloblabel (Entry->Name); - - /* Allocate space for uninitialized variable */ - g_res (Size); - } + /* A global (including static) uninitialized variable + ** is only a tentative definition. For example, this is valid: + ** int i; + ** int i; + ** static int j; + ** static int j = 42; + ** Code for these will be generated in FinishCompile. + */ } } @@ -300,7 +299,7 @@ void Compile (const char* FileName) struct tm* TM; /* Since strftime is locale dependent, we need the abbreviated month names - ** in english. + ** in English. */ static const char MonthNames[12][4] = { "Jan", "Feb", "Mar", "Apr", "May", "Jun", @@ -397,20 +396,26 @@ void Compile (const char* FileName) void FinishCompile (void) /* Emit literals, externals, debug info, do cleanup and optimizations */ { - SymTable* SymTab; - SymEntry* Func; + SymEntry* Entry; - /* Walk over all functions, doing cleanup, optimizations ... */ - SymTab = GetGlobalSymTab (); - Func = SymTab->SymHead; - while (Func) { - if (SymIsOutputFunc (Func)) { + /* Walk over all global symbols: + ** - for functions do cleanup, optimizations ... + ** - generate code for uninitialized global variables + */ + for (Entry = GetGlobalSymTab ()->SymHead; Entry; Entry = Entry->NextSym) { + if (SymIsOutputFunc (Entry)) { /* Function which is defined and referenced or extern */ - MoveLiteralPool (Func->V.F.LitPool); - CS_MergeLabels (Func->V.F.Seg->Code); - RunOpt (Func->V.F.Seg->Code); + MoveLiteralPool (Entry->V.F.LitPool); + CS_MergeLabels (Entry->V.F.Seg->Code); + RunOpt (Entry->V.F.Seg->Code); + } else if ((Entry->Flags & (SC_STORAGE | SC_DEF | SC_STATIC)) == (SC_STORAGE | SC_STATIC)) { + /* Tentative definition of uninitialized global variable */ + g_usebss (); + g_defgloblabel (Entry->Name); + g_res (SizeOf (Entry->Type)); + /* Mark as defined, so that it will be exported not imported */ + Entry->Flags |= SC_DEF; } - Func = Func->NextSym; } /* Output the literal pool */ From 31f19fbc6579a57186cc44e06e8494a34b9de057 Mon Sep 17 00:00:00 2001 From: Piotr Fusik Date: Mon, 13 Feb 2017 21:10:21 +0100 Subject: [PATCH 3/8] Issue an error for duplicate global variables. Previously it was an assembler error. --- src/cc65/compile.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cc65/compile.c b/src/cc65/compile.c index 85a2e9939..9cdeaf7e5 100644 --- a/src/cc65/compile.c +++ b/src/cc65/compile.c @@ -188,6 +188,10 @@ static void Parse (void) if (CurTok.Tok == TOK_ASSIGN) { /* This is a definition */ + if (SymIsDef (Entry)) { + Error ("Global variable `%s' has already been defined", + Entry->Name); + } Entry->Flags |= SC_DEF; /* We cannot initialize types of unknown size, or From 5988ec37cdce1655b14159a3a20a5493ba5459fd Mon Sep 17 00:00:00 2001 From: Piotr Fusik Date: Wed, 15 Feb 2017 18:51:27 +0100 Subject: [PATCH 4/8] Revert "Disallow global variable declarations with an initializer." This reverts commit 1f12a06f7cc2ae79a800fe3faed727513364091b. --- src/cc65/compile.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/cc65/compile.c b/src/cc65/compile.c index 9cdeaf7e5..48a5c29d3 100644 --- a/src/cc65/compile.c +++ b/src/cc65/compile.c @@ -144,14 +144,17 @@ static void Parse (void) ** ** - if it is not a typedef or function, ** - if we don't had a storage class given ("int i") - ** or the storage class is explicitly specified as static. + ** - if the storage class is explicitly specified as static, + ** - or if there is an initialization. ** ** This means that "extern int i;" will not get storage allocated. */ if ((Decl.StorageClass & SC_FUNC) != SC_FUNC && (Decl.StorageClass & SC_TYPEMASK) != SC_TYPEDEF && ((Spec.Flags & DS_DEF_STORAGE) != 0 || - (Decl.StorageClass & (SC_EXTERN|SC_STATIC)) == SC_STATIC)) { + (Decl.StorageClass & (SC_EXTERN|SC_STATIC)) == SC_STATIC || + ((Decl.StorageClass & SC_EXTERN) != 0 && + CurTok.Tok == TOK_ASSIGN))) { /* We will allocate storage */ Decl.StorageClass |= SC_STORAGE; From d2c89d2ba9d0030c538257a74a5d732cbdca6a0b Mon Sep 17 00:00:00 2001 From: Piotr Fusik Date: Thu, 9 Mar 2017 19:14:31 +0100 Subject: [PATCH 5/8] "static int n; int n;" is an error. Fixes test/err/static-4.c regression. --- src/cc65/symtab.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cc65/symtab.c b/src/cc65/symtab.c index fdf459873..3275332c5 100644 --- a/src/cc65/symtab.c +++ b/src/cc65/symtab.c @@ -821,7 +821,7 @@ SymEntry* AddGlobalSym (const char* Name, const Type* T, unsigned Flags) } /* An extern declaration must not change the current linkage. */ - if (IsFunc || (Flags & (SC_EXTERN | SC_DEF)) == SC_EXTERN) { + if (IsFunc || (Flags & (SC_EXTERN | SC_STORAGE)) == SC_EXTERN) { Flags &= ~SC_EXTERN; } From ce0cf853867e7f1c51075499c264e7d48e845190 Mon Sep 17 00:00:00 2001 From: Piotr Fusik Date: Thu, 9 Mar 2017 20:40:20 +0100 Subject: [PATCH 6/8] Add regression test for #204. --- test/val/static-fwd-decl.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 test/val/static-fwd-decl.c diff --git a/test/val/static-fwd-decl.c b/test/val/static-fwd-decl.c new file mode 100644 index 000000000..420640d97 --- /dev/null +++ b/test/val/static-fwd-decl.c @@ -0,0 +1,35 @@ +/* + !!DESCRIPTION!! static forward declarations + !!ORIGIN!! cc65 regression tests + !!LICENCE!! Public Domain + !!AUTHOR!! Bob Andrews +*/ + +/* + see: https://github.com/cc65/cc65/issues/204 +*/ + +#pragma warn(error, on) + +typedef struct _DIRMENU +{ + const char *name; + struct _DIRMENU *dest; +} DIRMENU; + +static DIRMENU rmenu; + +static DIRMENU lmenu = { + "left", + &rmenu +}; + +static DIRMENU rmenu = { + "right", + &lmenu +}; + +int main(void) +{ + return lmenu.dest == &rmenu && rmenu.dest == &lmenu ? 0 : 1; +} From 6c3873316b31f86b61ea11b67055ac66b2b66ebd Mon Sep 17 00:00:00 2001 From: Piotr Fusik Date: Thu, 9 Mar 2017 20:49:20 +0100 Subject: [PATCH 7/8] Add regression tests for duplicate global/static variables detected by the compiler. --- test/err/duplicate-global.c | 20 ++++++++++++++++++++ test/err/duplicate-static.c | 20 ++++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 test/err/duplicate-global.c create mode 100644 test/err/duplicate-static.c diff --git a/test/err/duplicate-global.c b/test/err/duplicate-global.c new file mode 100644 index 000000000..bd4fcc24a --- /dev/null +++ b/test/err/duplicate-global.c @@ -0,0 +1,20 @@ +/* + !!DESCRIPTION!! duplicate globals + !!ORIGIN!! cc65 regression tests + !!LICENCE!! Public Domain + !!AUTHOR!! Piotr Fusik +*/ + +/* + see: https://github.com/cc65/cc65/issues/191 +*/ + +#pragma warn(error, on) + +int n = 0; +int n = 0; /* should give an error */ + +int main(void) +{ + return n; +} diff --git a/test/err/duplicate-static.c b/test/err/duplicate-static.c new file mode 100644 index 000000000..394cc1e09 --- /dev/null +++ b/test/err/duplicate-static.c @@ -0,0 +1,20 @@ +/* + !!DESCRIPTION!! duplicate static variables + !!ORIGIN!! cc65 regression tests + !!LICENCE!! Public Domain + !!AUTHOR!! Piotr Fusik +*/ + +/* + see: https://github.com/cc65/cc65/issues/191 +*/ + +#pragma warn(error, on) + +static int n = 0; +static int n = 0; /* should give an error */ + +int main(void) +{ + return n; +} From 05d1b8072b580b1a68e09a76f21c9b8599af7028 Mon Sep 17 00:00:00 2001 From: Piotr Fusik Date: Thu, 9 Mar 2017 21:18:48 +0100 Subject: [PATCH 8/8] Add regression tests for duplicate globals with different linkage. --- test/err/duplicate-global-static.c | 18 ++++++++++++++++++ test/err/duplicate-static-global.c | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 test/err/duplicate-global-static.c create mode 100644 test/err/duplicate-static-global.c diff --git a/test/err/duplicate-global-static.c b/test/err/duplicate-global-static.c new file mode 100644 index 000000000..6aa27f5b7 --- /dev/null +++ b/test/err/duplicate-global-static.c @@ -0,0 +1,18 @@ +/* + !!DESCRIPTION!! global duplicated with static variable + !!ORIGIN!! cc65 regression tests + !!LICENCE!! Public Domain + !!AUTHOR!! Piotr Fusik +*/ + +/* + see: https://github.com/cc65/cc65/issues/191 +*/ + +int n = 0; +static int n = 0; /* should give an error */ + +int main(void) +{ + return n; +} diff --git a/test/err/duplicate-static-global.c b/test/err/duplicate-static-global.c new file mode 100644 index 000000000..6e5e70a37 --- /dev/null +++ b/test/err/duplicate-static-global.c @@ -0,0 +1,18 @@ +/* + !!DESCRIPTION!! static duplicated with global variable + !!ORIGIN!! cc65 regression tests + !!LICENCE!! Public Domain + !!AUTHOR!! Piotr Fusik +*/ + +/* + see: https://github.com/cc65/cc65/issues/191 +*/ + +static int n = 0; +int n = 0; /* should give an error */ + +int main(void) +{ + return n; +}