#79 patch for bug #1273984

[Günther Jehle]

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.

[Günther Jehle]

patch produced by svn diff SDCCicode.c

[Günther Jehle]

Logged In: YES
user_id=1301993
Originator: YES

File Added: bug-1273984.c

[Günther Jehle]

Regression test

[Borut Ražem]

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]

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]

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]

new patch produced by svn diff SDCCicode.c

[Borut Ražem]

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]

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]

Logged In: YES
user_id=888171
Originator: NO

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