#79 patch for bug #1273984

closed-accepted
Maarten Brock
None
5
2007-02-06
2007-01-26
Günther Jehle
No

See https://sourceforge.net/tracker/index.php?func=detail&aid=1273984&group_id=599&atid=100599 for details to the bug.

In the function geniCodeSEParms gets the parameters converted to OPERANDs. The operands should have the same type as the original parameter. But if the parameter is an assignment, the function ast2iCode returns the right symbol. And the right symbol could have another type as the parameter.

The solution:
Check if the parameter and the operand have different types. If they have, add a cast.

Discussion

1 2 > >> (Page 1 of 2)
  • Günther Jehle
    Günther Jehle
    2007-01-26

    patch produced by svn diff SDCCicode.c

     
  • Günther Jehle
    Günther Jehle
    2007-01-26

    Logged In: YES
    user_id=1301993
    Originator: YES

    File Added: bug-1273984.c

     
  • Günther Jehle
    Günther Jehle
    2007-01-26

    Regression test

     
    Attachments
  • Borut Ražem
    Borut Ražem
    2007-02-03

    Logged In: YES
    user_id=568035
    Originator: NO

    Günther,

    the attached regression test doesn't fail on ds390 and mcs51-large platforms (using no patched SDCC). Since you provided a patch which is target independent, you should prove that the bug exists on all targets. Could be this done by adding two additional dummy char parameters: one before and the other after "val", for example:

    ...

    void fooInt(char pre, {sign} int val, char post) {
    ASSERT(pre==0x55);
    ASSERT(val==3);
    ASSERT(val==0x55);
    }

    ...

    testAssignInFunctioncall(void)
    {
    ...
    fooInt(0x55, intVal=charVal, 0x55);
    ...
    ?

    I haven't tried it yet...

    Borut

     
  • Logged In: NO

    Hi Borut,

    the problem is, that the failure is only reproducible when the parameters in a function call are passed on the stack (with --stack-auto or a platform like z80 which passes parameters on stack). When they are passed in registers or global ram locations, the failure will not occur.

    Currently it is not possible to pass a option for a regression test, so it is not possible to use the --stack-auto flag for all platforms.

    Regards
    Günther

     
  • Maarten Brock
    Maarten Brock
    2007-02-06

    Logged In: YES
    user_id=888171
    Originator: NO

    I don't think that a cast is the correct fix. It seems like the right side of the assignment is passed as the parameter where it should use the left side. Consider the following:

    int a = 10;
    bool b;
    void foo(int, int);

    foo(a, b=a);

    IMO this should pass int 1 [ (int)(bool)(int 10) ] and not int 10 (no cast according to your rules).

    NB: There is an mcs51-small-stack-auto regression test that is run by default and there is even an mcs51-small-xstack-auto you could run. The mcs51 target only passes the first (non-bool) parameter by registers.

     
  • Günther Jehle
    Günther Jehle
    2007-02-06

    Logged In: YES
    user_id=1301993
    Originator: YES

    Hi Maarten,

    i think you are right. In case of assign in the ast2icode function is the right operand returned. It should be the left operand. See added patch for my correction.

    Regards
    Günther
    File Added: assign_in_functioncall_2.patch

     
  • Günther Jehle
    Günther Jehle
    2007-02-06

    new patch produced by svn diff SDCCicode.c

     
  • Borut Ražem
    Borut Ražem
    2007-02-06

    Logged In: YES
    user_id=568035
    Originator: NO

    > Currently it is not possible to pass a option for a regression test,
    > so it is not possible to use the --stack-auto flag for all platforms.

    An other prove that we have to implement the RFE "[ 1651820 ] Reg. tests: support of different SDCC command line options"...

    Borut

     
  • Bernhard Held
    Bernhard Held
    2007-02-06

    Logged In: YES
    user_id=203539
    Originator: NO

    > Currently it is not possible to pass a option for a regression test,
    > so it is not possible to use the --stack-auto flag for all platforms.
    There's a 'port' mcs51-stack-auto in the regression tests. There's no need to force _all_ ports to use --stack-auto. It's even more interesting to run a single test with as much as possible ports and/or compile options.

    And now my hint: here you don't necessarily need --stack-auto, you could also use the modifier __reentrant.

     
1 2 > >> (Page 1 of 2)