From fe9b00fb61dc71f48cb6416e608b5b6882bbc35f Mon Sep 17 00:00:00 2001 From: MaartenBrock Date: Sat, 10 Jun 2006 20:18:57 +0000 Subject: [PATCH] * as/hc08/asmain.c (asexit), * as/hc08/lkmain.c (lkexit), * as/mcs51/asmain.c (asexit), * as/mcs51/lkmain.c (lkexit), * src/SDCCglue.c (DEFSETFUNC), * src/SDCCmain.c (linkEdit, assemble), * support/librarian/sdcclib.c (AddRel), replaced unlink() by standard C remove() * src/SDCC.y: replaced removePostIncDecOps() by createRMW() * src/SDCCast.c (replaceAstWithTemporary, createRMW, gatherImplicitVariables): new, added to fix bug 608752, (createFunction): added gatherImplicitVariables() * src/SDCCast.h: added createRMW prototype * src/SDCCsymt.h (struct symbol): added infertype * support/regression/tests/bug608752.c: new, added git-svn-id: https://sdcc.svn.sourceforge.net/svnroot/sdcc/trunk/sdcc@4212 4a8a32a2-be11-0410-ad9d-d568d2c75423 --- ChangeLog | 18 +++ as/hc08/asmain.c | 2 +- as/hc08/lkmain.c | 9 +- as/mcs51/asmain.c | 2 +- as/mcs51/lkmain.c | 9 +- src/SDCC.y | 34 ++--- src/SDCCast.c | 197 +++++++++++++++++++++++---- src/SDCCast.h | 1 + src/SDCCglue.c | 2 +- src/SDCCmain.c | 12 +- src/SDCCsymt.h | 1 + support/librarian/sdcclib.c | 4 +- support/regression/tests/bug608752.c | 45 ++++++ 13 files changed, 266 insertions(+), 70 deletions(-) create mode 100644 support/regression/tests/bug608752.c diff --git a/ChangeLog b/ChangeLog index e4a43556..686cc748 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,21 @@ +2006-06-10 Maarten Brock + + * as/hc08/asmain.c (asexit), + * as/hc08/lkmain.c (lkexit), + * as/mcs51/asmain.c (asexit), + * as/mcs51/lkmain.c (lkexit), + * src/SDCCglue.c (DEFSETFUNC), + * src/SDCCmain.c (linkEdit, assemble), + * support/librarian/sdcclib.c (AddRel), + replaced unlink() by standard C remove() + * src/SDCC.y: replaced removePostIncDecOps() by createRMW() + * src/SDCCast.c (replaceAstWithTemporary, createRMW, + gatherImplicitVariables): new, added to fix bug 608752, + (createFunction): added gatherImplicitVariables() + * src/SDCCast.h: added createRMW prototype + * src/SDCCsymt.h (struct symbol): added infertype + * support/regression/tests/bug608752.c: new, added + 2006-06-10 Raphael Neider * src/pic16/gen.c (pic16_aopOp): use WREG as destination even for diff --git a/as/hc08/asmain.c b/as/hc08/asmain.c index fb12b4f7..0332a438 100644 --- a/as/hc08/asmain.c +++ b/as/hc08/asmain.c @@ -375,7 +375,7 @@ int i; if (i) { // remove output file printf ("removing %s\n", relFile); - unlink(relFile); + remove(relFile); } exit(i); } diff --git a/as/hc08/lkmain.c b/as/hc08/lkmain.c index 401916a5..4f6d0ff1 100644 --- a/as/hc08/lkmain.c +++ b/as/hc08/lkmain.c @@ -42,11 +42,6 @@ void Timer(int action, char * message) } #endif -/* yuck - but including unistd.h causes problems on Cygwin by redefining - * Addr_T. - */ -extern int unlink(const char *); - /*)Module lkmain.c * * The module lkmain.c contains the functions which @@ -116,7 +111,7 @@ void Areas51 (void) * * The function main() evaluates the command line arguments to * determine if the linker parameters are to input through 'stdin' - * or read from a command file. The functiond as_getline() and parse() + * or read from a command file. The functions as_getline() and parse() * are to input and evaluate the linker parameters. The linking process * proceeds by making the first pass through each .rel file in the order * presented to the linker. At the end of the first pass the setbase(), @@ -427,7 +422,7 @@ int i; copyfile(xfp,dfp); fclose(xfp); fclose(dfp); - unlink("temp.cdb"); + remove("temp.cdb"); }*/ exit(i); } diff --git a/as/mcs51/asmain.c b/as/mcs51/asmain.c index 13664abc..0f40fe84 100644 --- a/as/mcs51/asmain.c +++ b/as/mcs51/asmain.c @@ -375,7 +375,7 @@ int i; if (i) { // remove output file printf ("removing %s\n", relFile); - unlink(relFile); + remove(relFile); } exit(i); } diff --git a/as/mcs51/lkmain.c b/as/mcs51/lkmain.c index 9adbde04..43392b83 100644 --- a/as/mcs51/lkmain.c +++ b/as/mcs51/lkmain.c @@ -42,11 +42,6 @@ void Timer(int action, char * message) } #endif -/* yuck - but including unistd.h causes problems on Cygwin by redefining - * Addr_T. - */ -extern int unlink(const char *); - /*)Module lkmain.c * * The module lkmain.c contains the functions which @@ -155,7 +150,7 @@ void Areas51 (void) * * The function main() evaluates the command line arguments to * determine if the linker parameters are to input through 'stdin' - * or read from a command file. The functiond as_getline() and parse() + * or read from a command file. The functions as_getline() and parse() * are to input and evaluate the linker parameters. The linking process * proceeds by making the first pass through each .rel file in the order * presented to the linker. At the end of the first pass the setbase(), @@ -470,7 +465,7 @@ lkexit(int i) copyfile(xfp,dfp); fclose(xfp); fclose(dfp); - unlink("temp.cdb"); + remove("temp.cdb"); }*/ exit(i); } diff --git a/src/SDCC.y b/src/SDCC.y index 0277e533..49788ecb 100644 --- a/src/SDCC.y +++ b/src/SDCC.y @@ -392,45 +392,37 @@ assignment_expr $$ = newNode($2,$1,$3); break; case MUL_ASSIGN: - $$ = newNode('=',removePostIncDecOps(copyAst($1)), - newNode('*',removePreIncDecOps(copyAst($1)),$3)); + $$ = createRMW($1, '*', $3); break; case DIV_ASSIGN: - $$ = newNode('=',removePostIncDecOps(copyAst($1)), - newNode('/',removePreIncDecOps(copyAst($1)),$3)); + $$ = createRMW($1, '/', $3); break; case MOD_ASSIGN: - $$ = newNode('=',removePostIncDecOps(copyAst($1)), - newNode('%',removePreIncDecOps(copyAst($1)),$3)); + $$ = createRMW($1, '%', $3); break; case ADD_ASSIGN: - $$ = newNode('=',removePostIncDecOps(copyAst($1)), - newNode('+',removePreIncDecOps(copyAst($1)),$3)); + $$ = createRMW($1, '+', $3); break; case SUB_ASSIGN: - $$ = newNode('=',removePostIncDecOps(copyAst($1)), - newNode('-',removePreIncDecOps(copyAst($1)),$3)); + $$ = createRMW($1, '-', $3); break; case LEFT_ASSIGN: - $$ = newNode('=',removePostIncDecOps(copyAst($1)), - newNode(LEFT_OP,removePreIncDecOps(copyAst($1)),$3)); + $$ = createRMW($1, LEFT_OP, $3); break; case RIGHT_ASSIGN: - $$ = newNode('=',removePostIncDecOps(copyAst($1)), - newNode(RIGHT_OP,removePreIncDecOps(copyAst($1)),$3)); + $$ = createRMW($1, RIGHT_OP, $3); break; case AND_ASSIGN: - $$ = newNode('=',removePostIncDecOps(copyAst($1)), - newNode('&',removePreIncDecOps(copyAst($1)),$3)); + $$ = createRMW($1, '&', $3); break; case XOR_ASSIGN: - $$ = newNode('=',removePostIncDecOps(copyAst($1)), - newNode('^',removePreIncDecOps(copyAst($1)),$3)); + $$ = createRMW($1, '^', $3); break; case OR_ASSIGN: - /* $$ = newNode('=',$1,newNode('|',removeIncDecOps(copyAst($1)),$3)); */ - $$ = newNode('=',removePostIncDecOps(copyAst($1)), - newNode('|',removePreIncDecOps(copyAst($1)),$3)); +/* $$ = newNode('=',$1,newNode('|',removeIncDecOps(copyAst($1)),$3)); */ +/* $$ = newNode('=',removePostIncDecOps(copyAst($1)), + newNode('|',removePreIncDecOps(copyAst($1)),$3)); */ + $$ = createRMW($1, '|', $3); break; default : $$ = NULL; diff --git a/src/SDCCast.c b/src/SDCCast.c index c0f85e9c..04d0a63b 100644 --- a/src/SDCCast.c +++ b/src/SDCCast.c @@ -254,7 +254,7 @@ exit: return dest; } - +#if 0 /*-----------------------------------------------------------------*/ /* removeIncDecOps: remove for side effects in *_ASSIGN's */ /* "*s++ += 3" -> "*s++ = *s++ + 3" */ @@ -325,9 +325,91 @@ ast *removePostIncDecOps (ast * tree) { return tree; } +#endif +/*-----------------------------------------------------------------*/ +/* replaceAstWithTemporary: Replace the AST pointed to by the arg */ +/* with a reference to a new temporary variable. Returns*/ +/* an AST which assigns the original value to the */ +/* temporary. */ +/*-----------------------------------------------------------------*/ +static ast *replaceAstWithTemporary(ast **treeptr) +{ + symbol *sym = newSymbol (genSymName(NestLevel), NestLevel ); + ast *tempvar; + + /* Tell gatherImplicitVariables() to automatically give the + symbol the correct type */ + sym->infertype = 1; + sym->type = NULL; + sym->etype = NULL; + + tempvar = newNode('=', newAst_VALUE(symbolVal(sym)), *treeptr); + *treeptr = newAst_VALUE(symbolVal(sym)); + + addSymChain(&sym); + + return tempvar; +} + +/*-----------------------------------------------------------------*/ +/* createRMW: Create a read-modify-write expression, using a */ +/* temporary variable if necessary to avoid duplicating */ +/* any side effects, for use in e.g. */ +/* foo()->count += 5; becomes */ +/* tmp = foo(); tmp->count = tmp->count + 5; */ +/*-----------------------------------------------------------------*/ +ast * createRMW (ast *target, unsigned op, ast *operand) +{ + ast *readval, *writeval; + ast *tempvar1 = NULL; + ast *tempvar2 = NULL; + ast *result; + + if (!target || !operand) { + return NULL; + } + + /* we need to create two copies of target: one to read from and + one to write to. but we need to do this without duplicating + any side effects that may be contained in the tree. */ + + if (IS_AST_OP(target)) { + /* if this is a dereference, put the referenced item in the temporary */ + if (IS_DEREF_OP(target) || target->opval.op == PTR_OP) { + /* create a new temporary containing the item being dereferenced */ + if (hasSEFcalls(target->left)) + tempvar1 = replaceAstWithTemporary(&(target->left)); + } else if (target->opval.op == '[') { + /* Array access is similar, but we have to avoid side effects in + both values [WIML: Why not transform a[b] to *(a+b) in parser?] */ + if (hasSEFcalls(target->left)) + tempvar1 = replaceAstWithTemporary(&(target->left)); + if (hasSEFcalls(target->right)) + tempvar2 = replaceAstWithTemporary(&(target->right)); + } else { + /* we would have to handle '.', but it is not generated any more */ + wassertl(target->opval.op != '.', "obsolete opcode in tree"); + + /* no other kinds of ASTs are lvalues and can contain side effects */ + } + } + + readval = target; + writeval = copyAst(target); + + result = newNode('=', writeval, newNode(op, readval, operand)); + if (tempvar2) + result = newNode(',', tempvar2, result); + if (tempvar1) + result = newNode(',', tempvar1, result); + + return result; + +} /*-----------------------------------------------------------------*/ -/* hasSEFcalls - returns TRUE if tree has a function call */ +/* hasSEFcalls - returns TRUE if tree has a function call, */ +/* inc/decrement, or other side effect */ /*-----------------------------------------------------------------*/ bool hasSEFcalls (ast * tree) @@ -2254,6 +2336,73 @@ getLeftResultType (ast *tree, RESULT_TYPE resultType) } } +/*------------------------------------------------------------------*/ +/* gatherImplicitVariables: assigns correct type information to */ +/* symbols and values created by replaceAstWithTemporary */ +/* and adds the symbols to the declarations list of the */ +/* innermost block that contains them */ +/*------------------------------------------------------------------*/ +void +gatherImplicitVariables (ast * tree, ast * block) +{ + if (!tree) + return; + + if (tree->type == EX_OP && tree->opval.op == BLOCK) + { + /* keep track of containing scope */ + block = tree; + } + if (tree->type == EX_OP && tree->opval.op == '=' && + tree->left->type == EX_VALUE && tree->left->opval.val->sym) + { + symbol *assignee = tree->left->opval.val->sym; + + /* special case for assignment to compiler-generated temporary variable: + compute type of RHS, and set the symbol's type to match */ + if (assignee->type == NULL && assignee->infertype) { + ast *dtr = decorateType (resolveSymbols(tree->right), RESULT_TYPE_NONE); + + if (dtr != tree->right) + tree->right = dtr; + + assignee->type = copyLinkChain(TTYPE(dtr)); + assignee->etype = getSpec(assignee->type); + SPEC_SCLS (assignee->etype) = S_AUTO; + SPEC_OCLS (assignee->etype) = NULL; + SPEC_EXTR (assignee->etype) = 0; + SPEC_STAT (assignee->etype) = 0; + SPEC_VOLATILE (assignee->etype) = 0; + SPEC_ABSA (assignee->etype) = 0; + + wassertl(block != NULL, "implicit variable not contained in block"); + wassert(assignee->next == NULL); + if (block != NULL) { + symbol **decl = &(block->values.sym); + + while (*decl) { + wassert(*decl != assignee); /* should not already be in list */ + decl = &( (*decl)->next ); + } + + *decl = assignee; + } + } + } + if (tree->type == EX_VALUE && !(IS_LITERAL(tree->opval.val->etype)) && + tree->opval.val->type == NULL && + tree->opval.val->sym && + tree->opval.val->sym->infertype) + { + /* fixup type of value for compiler-inferred temporary var */ + tree->opval.val->type = tree->opval.val->sym->type; + tree->opval.val->etype = tree->opval.val->sym->etype; + } + + gatherImplicitVariables(tree->left, block); + gatherImplicitVariables(tree->right, block); +} + /*--------------------------------------------------------------------*/ /* decorateType - compute type for this tree, also does type checking.*/ /* This is done bottom up, since type has to flow upwards. */ @@ -2330,24 +2479,22 @@ decorateType (ast * tree, RESULT_TYPE resultType) tree->opval.val->etype = tree->opval.val->sym->etype = copyLinkChain (INTTYPE); } + else if (tree->opval.val->sym->implicit) + { + /* if implicit i.e. struct/union member then no type */ + TTYPE (tree) = TETYPE (tree) = NULL; + } else { + /* copy the type from the value into the ast */ + COPYTYPE (TTYPE (tree), TETYPE (tree), tree->opval.val->type); - /* if impilicit i.e. struct/union member then no type */ - if (tree->opval.val->sym->implicit) - TTYPE (tree) = TETYPE (tree) = NULL; - - else - { - - /* else copy the type */ - COPYTYPE (TTYPE (tree), TETYPE (tree), tree->opval.val->type); - - /* and mark it as referenced */ - tree->opval.val->sym->isref = 1; - } + /* and mark the symbol as referenced */ + tree->opval.val->sym->isref = 1; } } + else + wassert(0); /* unreached: all values are literals or symbols */ return tree; } @@ -3647,7 +3794,7 @@ decorateType (ast * tree, RESULT_TYPE resultType) changePointer(LTYPE(tree)); checkTypeSanity(LETYPE(tree), "(cast)"); - /* if 'from' and 'to' are the same remove the superflous cast, */ + /* if 'from' and 'to' are the same remove the superfluous cast, */ /* this helps other optimizations */ if (compareTypeExact (LTYPE(tree), RTYPE(tree), -1) == 1) { @@ -5799,21 +5946,23 @@ createFunction (symbol * name, ast * body) stackPtr = 0; xstackPtr = -1; + gatherImplicitVariables (body, NULL); /* move implicit variables into blocks */ + /* allocate & autoinit the block variables */ processBlockVars (body, &stack, ALLOCATE); - /* save the stack information */ - if (options.useXstack) - name->xstack = SPEC_STAK (fetype) = stack; - else - name->stack = SPEC_STAK (fetype) = stack; - /* name needs to be mangled */ SNPRINTF (name->rname, sizeof(name->rname), "%s%s", port->fun_prefix, name->name); body = resolveSymbols (body); /* resolve the symbols */ body = decorateType (body, RESULT_TYPE_NONE); /* propagateType & do semantic checks */ + /* save the stack information */ + if (options.useXstack) + name->xstack = SPEC_STAK (fetype) = stack; + else + name->stack = SPEC_STAK (fetype) = stack; + ex = newAst_VALUE (symbolVal (name)); /* create name */ ex = newNode (FUNCTION, ex, body); ex->values.args = FUNC_ARGS(name->type); @@ -5976,8 +6125,8 @@ void ast_print (ast * tree, FILE *outfile, int indent) } else { fprintf(outfile,"SYMBOL "); } - fprintf(outfile,"(%s=%p)", - tree->opval.val->sym->name,tree); + fprintf(outfile,"(%s=%p @ %p)", + tree->opval.val->sym->name, tree, tree->opval.val->sym); } if (tree->ftype) { fprintf(outfile," type ("); diff --git a/src/SDCCast.h b/src/SDCCast.h index 9e8ac526..a3fb3d3c 100644 --- a/src/SDCCast.h +++ b/src/SDCCast.h @@ -189,6 +189,7 @@ ast *removePreIncDecOps (ast *); ast *removePostIncDecOps (ast *); value *sizeofOp (sym_link *); value *evalStmnt (ast *); +ast *createRMW (ast *, unsigned, ast *); ast *createFunction (symbol *, ast *); ast *createBlock (symbol *, ast *); ast *createLabel (symbol *, ast *); diff --git a/src/SDCCglue.c b/src/SDCCglue.c index 2e616548..04f12922 100644 --- a/src/SDCCglue.c +++ b/src/SDCCglue.c @@ -91,7 +91,7 @@ DEFSETFUNC (rmTmpFiles) int ret; if (name) { - ret = unlink (name); + ret = remove (name); assert(ret == 0); Safe_free (name); } diff --git a/src/SDCCmain.c b/src/SDCCmain.c index 222acfc2..770cbff0 100644 --- a/src/SDCCmain.c +++ b/src/SDCCmain.c @@ -1859,7 +1859,7 @@ linkEdit (char **envp) options.out_fmt ? ".S19" : ".ihx", sizeof(scratchFileName)); if (strcmp (fullDstFileName, scratchFileName)) - unlink (fullDstFileName); + remove (fullDstFileName); rename (scratchFileName, fullDstFileName); strncpyz (buffer, fullDstFileName, sizeof(buffer)); @@ -1875,14 +1875,14 @@ linkEdit (char **envp) *q = 0; strncatz(buffer, ".map", sizeof(buffer)); if (strcmp (scratchFileName, buffer)) - unlink (buffer); + remove (buffer); rename (scratchFileName, buffer); *p = 0; strncatz (scratchFileName, ".mem", sizeof(scratchFileName)); *q = 0; strncatz(buffer, ".mem", sizeof(buffer)); if (strcmp (scratchFileName, buffer)) - unlink (buffer); + remove (buffer); rename (scratchFileName, buffer); if (options.debug) { @@ -1891,13 +1891,13 @@ linkEdit (char **envp) *q = 0; strncatz(buffer, ".cdb", sizeof(buffer)); if (strcmp (scratchFileName, buffer)) - unlink (buffer); + remove (buffer); rename (scratchFileName, buffer); /* and the OMF file without extension: */ *p = 0; *q = 0; if (strcmp (scratchFileName, buffer)) - unlink (buffer); + remove (buffer); rename (scratchFileName, buffer); } } @@ -1950,7 +1950,7 @@ assemble (char **envp) port->linker.rel_ext, sizeof(scratchFileName)); if (strcmp (scratchFileName, fullDstFileName)) - unlink (fullDstFileName); + remove (fullDstFileName); rename (scratchFileName, fullDstFileName); } } diff --git a/src/SDCCsymt.h b/src/SDCCsymt.h index bcf6e418..8455dbcf 100644 --- a/src/SDCCsymt.h +++ b/src/SDCCsymt.h @@ -256,6 +256,7 @@ typedef struct symbol in the symbol and not in v_struct or the declarator */ unsigned implicit:1; /* implicit flag */ unsigned undefined:1; /* undefined variable */ + unsigned infertype:1; /* type should be inferred from first assign */ unsigned _isparm:1; /* is a parameter */ unsigned ismyparm:1; /* is parameter of the function being generated */ unsigned isitmp:1; /* is an intermediate temp */ diff --git a/support/librarian/sdcclib.c b/support/librarian/sdcclib.c index 70d1bda6..80aedc10 100644 --- a/support/librarian/sdcclib.c +++ b/support/librarian/sdcclib.c @@ -367,8 +367,8 @@ void AddRel(void) fclose(lib); fclose(libindex); - unlink(LibNameTmp); - unlink(IndexName); + remove(LibNameTmp); + remove(IndexName); } void ExtractRel(void) diff --git a/support/regression/tests/bug608752.c b/support/regression/tests/bug608752.c new file mode 100644 index 00000000..c3c541d5 --- /dev/null +++ b/support/regression/tests/bug608752.c @@ -0,0 +1,45 @@ +/* OpAssign tests + */ +#include + +#ifdef SDCC +#include +#else +#define _STATMEM +#endif + +typedef struct +{ + char a; + char n; +} item_type; + +item_type t; + + +_STATMEM item_type* get_next_item(void) +{ + /* have a side effect */ + t.n++; + + /* keep things easy, not implementing a list. + Using a true list would break things + even more pointedly: + a) reading beyond end of the list and + b) intermixing list members */ + return &t; +} + + +void +testOpAssign(void) +{ + t.a = 0; + t.n = 0; + + /* get_next_item() should be called only once */ + get_next_item()->a |= 42; + + ASSERT (t.a == 42); + ASSERT (t.n == 1); +} -- 2.30.2