#771 Use of uninitialized var results in bad code

closed-fixed
5
2013-05-25
2004-06-12
No

Hi.

I know that using the uninitialized var is in general
an error. There are the cases however, when using the
uninitialized var should give the expected results.
The example below demonstrates that:
---
unsigned char ttt[] = {0xff, 1};

char main()
{
unsigned char a, i;
for (i = 0; i < sizeof(ttt); i++)
a |= ttt[i];
return a;
}
---
Obviously, even though the "a" is used uninitialized,
the result must be 0xff.
It seems sdcc doesn't allocate the storage for the
uninitialized vars, even if they are used. In this
example the result is 1 instead of 0xff. I beleive
this is a bug of sdcc.
The test case attached.

I would also like to get a warning out of it, but
that's really an RFE.

PS:
$ sdcc --wrong-opt orsh.c
-:0: warning: unknown compiler option '--wrong-opt'
ignored
-:0: warning: unknown compiler option '--wrong-opt'
ignored

The error message is reported twice - no good:)

Discussion

  • Stas Sergeev

    Stas Sergeev - 2004-06-12

    test case

     
  • Erik Petrich

    Erik Petrich - 2004-06-21

    Logged In: YES
    user_id=635249

    Fixed in src/SDCClrange.c 1.35

    Actually, it was allocating storage for the variable 'a',
    but due to a bug in the live range analysis, it assigned the
    same register used to hold the temporary value fetched from
    ttt[i].

    The duplicate unknown compiler option messages is fixed in
    src/SDCCmain.c 1.197

     
  • Erik Petrich

    Erik Petrich - 2004-06-21
    • assigned_to: nobody --> epetrich
    • labels: 101552 --> Live range problems
    • milestone: --> fixed
    • status: open --> closed-fixed
     
  • Stas Sergeev

    Stas Sergeev - 2004-06-23

    Another testcase

     
  • Stas Sergeev

    Stas Sergeev - 2004-06-23

    Logged In: YES
    user_id=501371

    Hi Erik, thanks for the fix.

    > Actually, it was allocating storage for the variable 'a',
    > but due to a bug in the live range analysis, it assigned the
    > same register used to hold the temporary value fetched from
    > ttt[i].
    OK, but then please take a look in another test-case I've
    attached.
    Yes, in that test-case the use of the uninitialized var is
    completely
    illegal, but again it generates the crap code like this:
    ---
    mov a,r2
    orl ar2,a
    mov a,r3
    orl ar3,a
    ---
    Is this intentional? Why does it still assign the value to
    the same regs
    where's the itemp sits?

     
  • Erik Petrich

    Erik Petrich - 2004-06-24

    Logged In: YES
    user_id=635249

    The intermediate code for tst1,c is something like:

    iTemp1 = (unsigned int) ttt;
    iTemp0 = iTemp0 | iTemp1;
    return iTemp0;

    The live range for iTemp1 ends at the same instruction that
    begins the live range for iTemp0, thus they can be safely
    given allocations that have registers in common as long as
    the registers in common are at the same byte positions. The
    resulting asm code looks silly, but seems to correct to me,
    given that iTemp0 previously had no meaningful value (and
    thus needed no registers before this point to hold this value).

    I agree that there should be a warning given for this kind
    of code; it is on my to-do list.

     
  • Stas Sergeev

    Stas Sergeev - 2004-06-25
    • status: closed-fixed --> open-fixed
     
  • Stas Sergeev

    Stas Sergeev - 2004-06-25

    New test-case

     
  • Stas Sergeev

    Stas Sergeev - 2004-06-25

    Logged In: YES
    user_id=501371

    Hi again.

    I think I should reopen this bug - attached is yet another
    test-case
    where the uninitialized var usage is legal, and the result
    is wrong again.

    > The live range for iTemp1 ends at the same instruction that
    > begins the live range for iTemp0
    But imho it is not exactly like that. It ends at the same
    line of C code,
    but not on the same asm instruction, as I understand it.
    My reservation comes from the following (probably wrong)
    considerations:
    ---
    [L-R of "a" starts here]
    mov a,r2
    orl ar2,a
    "ttt" (or its iTemp) is destroyed here - one of its bytes is
    reused
    mov a,r3
    orl ar3,a
    L-R of "ttt" (or its iTemp) ends here, but it was destroyed
    earlier
    ---
    So I thought it is a bug, even though I failed to create the
    test-case
    that could exploit and demonstrate it.

    > resulting asm code looks silly, but seems to correct to me,
    OK, I have to beleive you:)
    But anyway, my new test-case breaks sdcc again.

     
  • Erik Petrich

    Erik Petrich - 2004-08-05
    • status: open-fixed --> closed-fixed
     
  • Erik Petrich

    Erik Petrich - 2004-08-05

    Logged In: YES
    user_id=635249

    Fixed (completely, I hope); see ChangeLog 1.789 for the list
    of updated files.

    The live range boundaries are based on the intermediate
    code, not the C or assembly. The backend code generator is
    not required to preserve an operand to the very last
    assembly instruction of an iCode if the operand's live range
    ends at that iCode. When dealing with multibyte operands, it
    is possible that registers in a source operand will be
    reused for the destination operand; this is safe if these
    bytes are not needed as a source values again. The
    positionRegs function in ralloc.c ensures that the registers
    are arranged so that backend code generator can ignore the
    possible register overlap in most cases.

     
  • Borut Ražem

    Borut Ražem - 2007-12-31

    Logged In: YES
    user_id=568035
    Originator: NO

    I reopened the bug since it is not fixed for pic16 target. Regression test bug-971834.c fails:
    gen/pic16/bug-971834/bug-971834.out:18:--- FAIL: "Assertion failed" on orsh() == 0xff at gen/pic16/bug-971834/bug-971834.c:45
    gen/pic16/bug-971834/bug-971834.out:19:--- FAIL: "Assertion failed" on orsh1() == 0xff at gen/pic16/bug-971834/bug-971834.c:46

    Borut

     
  • Borut Ražem

    Borut Ražem - 2007-12-31
    • status: closed-fixed --> open-fixed
     
  • Raphael Neider

    Raphael Neider - 2008-09-15
    • status: open-fixed --> closed-fixed
     
  • Raphael Neider

    Raphael Neider - 2008-09-15

    Fixed for pic16 in SDCC 2.8.3, r5238.

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks