#79 patch for bug #1273984

closed-accepted
None
5
2007-02-06
2007-01-26
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

  • 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

     
  • 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

     
  • Nobody/Anonymous

    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

     
  • 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.

     
  • Maarten Brock

    Maarten Brock - 2007-02-06

    Logged In: YES
    user_id=888171
    Originator: NO

    Something similar to the second patch is applied in SDCC 2.6.4 #4622.
    Thanks.

     
  • Maarten Brock

    Maarten Brock - 2007-02-06
    • assigned_to: nobody --> maartenbrock
    • status: open --> closed-accepted
     

Log in to post a comment.