#1420 explicit typecast is ineffective for unsigned char parameter

closed-fixed
Borut Ražem
7
2013-05-25
2008-01-18
Laszlo BORTEL
No

The compiler seems to ignore the explicit (unsigned char) typecast when it is applied to the actual parameter of a function with variable arguments. (Explicit (unsigned char) typecast is necessary even for unsigned char type otherwise the compiler would automatically push a two byte integer onto the stack.) The bug for example blocks the use of printf() function with "%bu" format specifier.

C:\>sdcc -v
SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.4 #4972 (Nov 23 2007) (MINGW32)

C:\>ver

Microsoft Windows XP [Version 5.1.2600]

Discussion

1 2 > >> (Page 1 of 2)
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-01-18

     
    Attachments
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-01-19

     
    Attachments
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-01-19

    Logged In: YES
    user_id=1063737
    Originator: YES

    I have increased the priority from the default and attached the list file. From the file it can be seen that the compiler generates absolutely the same code for parameter passing in both cases. Two bytes pushed onto the stack for unsigned char variable:

    0008 146 _main:
    147 ; BugReport18.c:5: printf ("%u", u);//correct
    0008 74 01 148 mov a,#0x01
    000A C0 E0 149 push acc
    000C E4 150 clr a
    000D C0 E0 151 push acc
    000F 74r00 152 mov a,#__str_0
    0011 C0 E0 153 push acc
    0013 74s00 154 mov a,#(__str_0 >> 8)
    0015 C0 E0 155 push acc
    0017 74 80 156 mov a,#0x80
    0019 C0 E0 157 push acc
    001B 12s00r00 158 lcall _printf
    001E E5 81 159 mov a,sp
    0020 24 FB 160 add a,#0xfb
    0022 F5 81 161 mov sp,a
    162 ; BugReport18.c:6: printf ("%bu", (unsigned char)u);//buggy
    0024 74 01 163 mov a,#0x01
    0026 C0 E0 164 push acc
    0028 E4 165 clr a
    0029 C0 E0 166 push acc
    002B 74r03 167 mov a,#__str_1
    002D C0 E0 168 push acc
    002F 74s00 169 mov a,#(__str_1 >> 8)
    0031 C0 E0 170 push acc
    0033 74 80 171 mov a,#0x80
    0035 C0 E0 172 push acc
    0037 12s00r00 173 lcall _printf
    003A E5 81 174 mov a,sp
    003C 24 FB 175 add a,#0xfb
    003E F5 81 176 mov sp,a
    0040 22 177 ret

    File Added: BugReport18.lst

     
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-01-19

    • priority: 5 --> 7
     
  • Borut Ražem
    Borut Ražem
    2008-01-19

    • assigned_to: nobody --> borutr
    • milestone: --> 100455
    • labels: --> C-Front End
    • status: open --> pending-works-for-me
     
  • Borut Ražem
    Borut Ražem
    2008-01-19

    Logged In: YES
    user_id=568035
    Originator: NO

    Hi Laszlo,

    I think that the behavior is correct: unsigned char is promoted to int.

    See ISO/IEC 9899 standard, chapter 6.5.2.2 Function calls, item #6:

    "If the expression that denotes the called function has a type that does not include a
    prototype, the integer promotions are performed on each argument, and arguments that
    have type float are promoted to double. These are called the default argument
    promotions. ..."

    In item #7 is written:

    "... The ellipsis notation in a function prototype declarator causes
    argument type conversion to stop after the last declared parameter.
    The default argument promotions are performed on trailing arguments. ..."

    Which exactly explains your case.

    Borut

     
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-01-21

    Logged In: YES
    user_id=1063737
    Originator: YES

    Dear Borut,

    While understanding your reference to the ISO/IEC 9899 standard I still think that there is a bug somewhere around. Please take a look at the new file "BugReport18b.c" that I added. It seems that the default argument promotion from unsigned char to int can be supressed by explicit (unsigned char) typecast when the actual parameter is unsigned int (variable u in the file) and can't be supressed when the actual parameter is unsigned char (variable c in the file).
    Explicit double-typecast (see last printf(); call) can be used as workaround, but it should not be necessary in my opinion.

    Regards,
    Laszlo
    File Added: BugReport18b.c

     
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-01-21

    • status: pending-works-for-me --> open-works-for-me
     
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-01-21

     
    Attachments
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-01-21

     
    Attachments
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-01-21

    Logged In: YES
    user_id=1063737
    Originator: YES

    File Added: BugReport18b.lst

     
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-02-02

     
    Attachments
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-02-02

    Logged In: YES
    user_id=1063737
    Originator: YES

    Hi Borut,

    let me add one more important bit of information to this bug report. In the extended file "BugReport18c.c" it can be seen that explicit (unsigned char) typecast is NOT effective for static variable 'unsigned char d', but effective for static structure member 'unsigned char s.d', which is shocking, because both d and s.d are of the same type, so there sould be no diffrence in their treatment.
    I would expect consistent behaviour in all the cases listed in the 'BugReport18c.c' file: the compiler should apply default argument promotion according to ISO/IEC 9899 UNLESS there is an explicit typecast in the actual parameter list of the function, in which case the explicit typecast should override/supress default argument promotion.
    What I expect is not against the mentioned standard, so I ask you to reconsider your opinion and correct the bug.

    Regards,
    Laszlo
    File Added: BugReport18c.c

     
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-02-02

     
    Attachments
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-02-02

    Logged In: YES
    user_id=1063737
    Originator: YES

    File Added: BugReport18c.lst

     
  • Borut Ražem
    Borut Ražem
    2008-02-03

    Logged In: YES
    user_id=568035
    Originator: NO

    Hi Laszlo,

    my interpretation of the standard is different of yours: I think that explicit casting to unsigned char shouldn't change anything: the argument has still to be promoted to an int, as required by the integer promotion rule, which says that *in the absence of a function prototype*, _Bool, char and short are promoted either to int or unsigned int when used as, or in, function arguments.

    But I agree that there is a bug in SDCC. I think that we need an additional opinions to decide what is right and what is wrong before fixing anything.

    SDCC developers and users: can you help us here?

    My interpretation is as follows:

    --8<---------------

    void printf(char* str, ...) {str;}

    struct {
    unsigned char d;
    } s={0x78};

    unsigned char d=0x89;

    void main (void) {
    unsigned char c=0x56;
    unsigned int u=0x1234;
    //correct: neither explicit typecast, nor automatic promotion
    printf ("%u", u);

    //buggy: explicit typecast shuld not override automatic promotion to integer
    printf ("%bu", (unsigned char)u);

    //correct: automatic promotion to integer is performed
    printf ("%u", c);

    //correct: explicit typecast doesn't override automatic promotion to integer
    printf ("%bu", (unsigned char)c);

    //buggy: explicit typecasts (doesn't matter how many they are) should not override automatic promotion to integer
    printf ("%bu", (unsigned char)(unsigned int)c);

    //correct: automatic promotion to integer is performed on static variable
    printf ("%u", d);

    //correct: explicit typecast doesn't override automatic promotion to integer
    printf ("%bu", (unsigned char)d);

    //correct: automatic promotion to integer is performed
    printf ("%u", s.d);

    //buggy: explicit typecast should not override automatic promotion to integer
    printf ("%bu", (unsigned char)s.d);
    }

    -->8---------------

    Borut

     
  • Borut Ražem
    Borut Ražem
    2008-02-03

    • milestone: 100455 -->
    • status: open-works-for-me --> open
     
  • Borut Ražem
    Borut Ražem
    2008-02-03

    Logged In: YES
    user_id=568035
    Originator: NO

    An other indication that I might be right is:

    "In function calls not in the scope of a function prototype, integer arguments have the integer
    promotions applied and float arguments are widened to double. *It is not possible in such a
    call to pass an unconverted char or float argument.*"

    as stated in Rationale for International Standard — Programming Languages — C, Revision 5.10, April-2003, chapter 6.7.5.3p15.

    The document can be found at:
    http://www.open-std.org/JTC1/SC22/WG14/www/docs/C99RationaleV5.10.pdf

    Borut

     
  • Maarten Brock
    Maarten Brock
    2008-02-03

    Logged In: YES
    user_id=888171
    Originator: NO

    I agree that ellipis parameters should be cast up to int. Mainly because I don't see how to solve this otherwise without losing other optimizations. The price to be paid is unnecessary upcasts when printing char values. This also means that the SDCC specific %b formatter becomes useless (see also regression test snprintf.c) and that the manual must be updated (1.4 Compatibility with previous versions).

     
  • Laszlo BORTEL
    Laszlo BORTEL
    2008-02-04

    Logged In: YES
    user_id=1063737
    Originator: YES

    Hi All,

    I am not familiar with different standards of the C language - so I will not bring you such counter argument. The problem I raised is very practical: I just wanted to use the %b formatter with char argument in the printf() function. Then I realised: sometimes it is possible with explicit typecast, sometimes it is not. My argument to support my interpretation is that it is better to leave control in the hand of the programmer and consistently let him override automatic promotion if he wishes so than deprive him of this possibility and drop this neat feature of byte-sized arguments in print formatting.
    I think it is not easy to decide between my practical and Borut's pragmatic interpretation. One idea: isn't it feasible to introduce a command-line switch for the compiler to select between the two possible consistent behaviours? (--explicit-typecast-overrides-automatic-promotion).

    Regards,
    Laszlo

     
  • Borut Ražem
    Borut Ražem
    2008-02-23

    Logged In: YES
    user_id=568035
    Originator: NO

    Maarten, what do you think about the command-line switch proposed by Laszlo? I have to say that I'm not very favorable to it...

    I've found the piece of code which disables the integer promotion of varargs in case of explicit type cast: it is in SDCCast.c, function processParms(), condition statement around the comment /* Parameter was explicitly typecast; don't touch it. */. If the condition is removed the integer propagation is done even in case of explicit type casting.

    And now a question: as far as I could find, the %b and %B printf modifiers, which take one byte parameter, are available only in device/lib/printf_large.c. Is this correct?

    BTW: I found the %b modifier also in pic libraries: device/lib/pic16/libc/stdio, files printf_tiny.c and vfprintf.c, but the meaning is totally different: it means output in binary format.

    Borut

     
  • Maarten Brock
    Maarten Brock
    2008-02-24

    Logged In: YES
    user_id=888171
    Originator: NO

    I don't like to add another command-line switch for this.

    I looked at the history of that piece that disables the promotion and it was added together with varargs support in revision #541. I also seem to remember that I once bumped into this issue of a lost cast when changing some code, but I can't find it anymore. Somehow I have the feeling that I broke this behaviour by removing 'unnecessary' casts in favour of better optimization. But this might just as well have been during a local test and was never commited.

    Even though this is non-standard behaviour I would somehow like to keep it for SDCC. SDCC is targeted for small (8 bit) devices and I think this is a very useful extension.

    Yes %b modifiers are only in printf_large as far as I know. And they have been there since the initial revision. See
    http://sdcc.svn.sourceforge.net/viewvc/sdcc/trunk/sdcc/device/lib/vprintf.c?revision=3&view=markup
    The pics use %b for something else as you found out. Maybe we should split and merge them. How about %B for bytes and %b for bits?

    Maarten

     
  • Borut Ražem
    Borut Ražem
    2008-02-24

    Logged In: YES
    user_id=568035
    Originator: NO

    Maarten, if I understand you correctly, you are more favorable to the solution proposed by Lazslo. In this case the following condition should be removed from file SDCCasc.c, function decoreteType(), case CAST:

    /* if 'from' and 'to' are the same remove the superfluous cast, */
    /* this helps other optimizations */
    if (compareTypeExact (LTYPE(tree), RTYPE(tree), -1) == 1)
    {
    return tree->right;
    }

    This lines were added in svn revision #4124 by Bernhard Held on 2006-04-25 with the comment: "partial fix for RFE 1475769, remove cast to same type". If we revert this change, we will probably have other problems and lose some optimizations, as you wrote, and we would not be compliant with the ISO C standard.

    Maybe introduction of a command line switch is not so bad idea after all...

    Borut

     
  • Borut Ražem
    Borut Ražem
    2008-02-24

    Logged In: YES
    user_id=568035
    Originator: NO

    The better solution would be to execute the if (compareTypeExact(...) == 1) condition only in case if the left operand of cast is not actual function parameter:

    if (!ACTUAL_FUNCTION_PARAMETER (tree->right) && compareTypeExact (LTYPE(tree), RTYPE(tree), -1) == 1)
    {
    return tree->right;
    }

    Now the problem is how the ACTUAL_FUNCTION_PARAMETER() should be implemented: the CAST is evaluated before the CALL, so we probably don't know that the operand is a function parameter?

    Any idea?

    Borut

     
  • Maarten Brock
    Maarten Brock
    2008-02-24

    Logged In: YES
    user_id=888171
    Originator: NO

    Apart from introducing a new command line switch I don't see a solution by Laszlo. SDCC has had special behaviour for this from the beginning and allthough non-standard I don't see a reason to reject it now.

    But I'm really not in favour of yet another switch. And thinking of it now, why not use --std-cXX vs. --std-sdccXX switches. If this is a SDCC specific language extension then it should be enabled by --std-sdccXX and disabled by --std-cXX.

    Is this code in decorateType() the part that removes the cast in this case? Maybe we should find a way to remember that it was an explicit or implicit cast. And also use that in processParms like literalFromCast.

    Maarten

     
1 2 > >> (Page 1 of 2)