Beware the hidden CAST

Go To Last Post
13 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Heres one that got me today.

given the following code fragment. (used to illustrate the bug)

unsigned long mask;
unsigned char shift;

for(shift=0;shift<24;shift++) {
    mask = 1<<shift;
    printf("%02d: %06lx", shift, mask);
}

Everything at first glance seems to be correct, and on a PC will compile and output what you would expect. Yet using IAR, the output suddenly begins to give the incorrect output once shift reaches 15.

output:

00: 000001
01: 000002
.
.
.
13: 002000
14: 004000
15: ff8000
16: 000000
.
.
.
23: 000000

You see I had forgotten that IAR treats lliterals as signed int's by default, and because of casting rules in the order of operations, the shift is done on a signed int, and then cast into a long. resulting in a 16 bit shift, and a sign extension to 32 bit operation.

Simply forcing the literal to ba a long corrects this.

    mask = 1L<<shift;

So beware of those hidden casts, tehy can both affect the efficiency of your code, and it's reliability. While mine got me on reliability, the following will get you on efficiency.

unsigned char mask;
unsigned char shift;

for(shift=0;shift<8;shift++) {
    mask = 1<<shift;
    printf("%02d: %02x", shift, mask);
}

While you will always get the correct result this time, the shift will still be performed as a 16bit shift, and then cast down to a char. the solution is to force the literal to be a char this time. (If you're lucky the optimizer might strip out the shift on the upper byte)

Also note, that while I used IAR, the same will happen on any compiler. It's just a matter of when, and that is determined by what the default datatype the compiler uses for its literals.

Note that these gotchas only apply if the result cannot be determined at compile time. If it can, the casting and resizing is done by the pre-processor, leaving an efficient set of a constant of the correct size.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

In fact, it is more surprising that your first example gives 'correct' result on a PC (did you try?) than the 'incorrect' result on an 'embedded' compiler. All operations should be performed on ints unless a long or double operand is used. The result of the operation is the size of the longest operand, converting to the size of the result variable is the last step.

mark

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Any ideas what GCC defaults to ???

/Bingo

Btw: 1L is casting to long , what is char / int ??

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

gizmo wrote:
In fact, it is more surprising that your first example gives 'correct' result on a PC (did you try?) than the 'incorrect' result on an 'embedded' compiler. All operations should be performed on ints unless a long or double operand is used. The result of the operation is the size of the longest operand, converting to the size of the result variable is the last step.

Most PC compilers these days have 32 bit integers, which is why it works on the PC. Even if you force the 1 to be a signed 16-bit int:

      mask = ((signed short)1) << shift;

the compiler will promote it back to a full (32 bit) int before doing the shift operation.

It is the default int size (which I am assuming is 16 bits for IAR), the fact that doing a 24 bit shift on a 16 bit int doesn't do the same thing as a 24 bit shift on a 32 bit int, and the fact that glitch was expecting 32 bit behavior out of a 16 bit compiler that caused this particular "glitch".

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would say that all operations default to the hardware default. On AVR this would be 8 bit. This is what Codevision does.
Must say it also got me. Shifting one is dangerous.

In general I would say "beware to cast". these embedded compilers are kind of "lazy casters". I guess this is an efficiency thing. If the compiler would do unnecessary casts to bigger units this would make the software unefficient without the user noticing until his app doesn't meet the specs.

I also thought that if I assign a value (result of operation) to a bigger data unit the compiler would automatically cast the whole operation. Well, no. And I guess it has it's purpose.
By now I'm kind of cautios as to these problems and I use () and (cast) rather redundantly than not enough.

Frustrating isn't it. When these things happen ;-)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

foo wrote:
It is the default int size (which I am assuming is 16 bits for IAR), the fact that doing a 24 bit shift on a 16 bit int doesn't do the same thing as a 24 bit shift on a 32 bit int, and the fact that glitch was expecting 32 bit behavior out of a 16 bit compiler that caused this particular "glitch".

It's not so much that I was expecting 32bit operation in a 16bit world, I had simply forgotten in that instance that the literals are signed 16bit values, and that the literal in this case would define the size of the operation. It's funny, because in the code that had the bug, the preceeding line was setting another 32bit variable, and in that case I was forcing the value to be long, even though it was well within the 16bit limit. It was just overlooked in the way the literal was used in this case. Now I have the joy of reviewing 100K+lines of code to make sure I didn't repeat the same brain fart anywhere else.

knoppy wrote:
I also thought that if I assign a value (result of operation) to a bigger data unit the compiler would automatically cast the whole operation. Well, no. And I guess it has it's purpose.

The problem is order of operations, if you place the cast before the operation, it would work when promoting the value, but would fail miserably when demoting it. Like anything else, the compiler must follow the rules, otherwise it would be impossible to write a reliable application, if no order of operation was implied.

Another solution that would have worked, and would eliminate the cast problem, would have been to re-write my code as follows:

msk = 1;
msk <<= shift;

In this case the shift will always be of the size of the msk variable, and the hidden cast is taken out of play, as it happens in the assignment on the previous line.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Bingo600 wrote:
Any ideas what GCC defaults to ???

Btw: 1L is casting to long , what is char / int ??

AVR GCC defaults to constants as being signed ints = 16 bits, just as IAR.

If you just need a char or int constant then just leave it as the default, i.e. 1.

And for everybody's information, if you're going to be bit shifting, the cast should be 1UL to make the constant an unsigned long, that way you don't get into any potential weirdness with shifting a signed value.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

glitch wrote:
Heres one that got me today.

Glitch - if there are gotchas still lurking that get you, what hope is there for mere mortals like me 8^/

Regards: Jim Ford

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

jimford wrote:
glitch wrote:
Heres one that got me today.

Glitch - if there are gotchas still lurking that get you, what hope is there for mere mortals like me 8^/

Yeah, but this is a relatively common issue when dealing with embedded systems. You have to know what sizes are your types, and what are the implicit rules that Standard C does to your code. In the PC world (and larger) you can get by with sloppy coding because of the greater amount of resources. (IMHO, this is one of the reasons why code keeps bloating on the PC, but that's another rant.)

This is one reason why the C99 standard has included the header file, which introduces standard types where you are guaranteed a certain bit width. Some of these types that the header defines are:

uint8_t = unsigned 8 bit integer
uint16_t = unsigned 16 bit integer
uint32_t = unsigned 32 bit integer
int8_t = signed 8 bit integer
int16_t = signed 16 bit integer
int32_t = signed 32 bit integer

There really should be no reason to make your own types, or to not use these. I think it's really easy to tell at a glance what size and type variables are. YMMV.

(As a side note, the GNU AVR toolset that includes avr-libc currently does not directly implement , it implements these types in which the standard says that file should include . So users of AVR GCC (or WinAVR) should #include to get these definitions.)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I hope I am asking the right question here, but since I recently worked through the casting thing with a basic rolling average function, and had some real funny results with different casts, does this topic

Sonos wrote:

I was wondering this because when a function I wrote for a rolling average required casting from signed int to float, I was getting garbage unless I initialized the unsigned int to 0 as a local.

K&R pg 87 wrote:
In the absence of explicit initialization, external and static variables are guaranteed to be initialized to zero;.. register variables have undefined (i.e. garbage) initial values.

but nothing is mentioned about casting in this context...

https://www.avrfreaks.net/phpBB2/...

have anything to do with this thread? If not, don't think I am asking you to tell me I don't know what what the hell your talking about.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

EW wrote:
In the PC world (and larger) you can get by with sloppy coding because of the greater amount of resources. (IMHO, this is one of the reasons why code keeps bloating on the PC, but that's another rant.)

Aha! So THAT's how Billy and his boys and girls could always get away with all that inefficient code! :-)

And to stay on topic: maybe they should have tried implementing Windoze on an AVR first. :-)

Thanks for clearing that up, Eric!

Ed.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

glitch wrote:
It's not so much that I was expecting 32bit operation in a 16bit world, I had simply forgotten in that instance that the literals are signed 16bit values, and that the literal in this case would define the size of the operation.

That's not exactly true... the operation size is defined by both sides of any binary operation. For example, if "shift" had been declared as "unsigned long", that would also have caused the operation to be 32 bit. Anyways, in a "16bit world" all operations are 16 bit unless you do something to cause it to be otherwise.

For people looking for implicit cast pitfalls, here's (in my opinion) a slightly nastier one to watch out for:

unsigned int x;
//...
x = 2;
if (x < -1) {
   printf("oops\n"); // This WILL be executed!
}

I.e., when signed and unsigned are mixed, unsigned wins (and there is no actual size promotion), so the results are a bit goofy.

glitch wrote:
Now I have the joy of reviewing 100K+lines of code to make sure I didn't repeat the same brain fart anywhere else.

Have fun with that! :P

(It might be easier to just write a program to do it...)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

foo wrote:

...
For people looking for implicit cast pitfalls, here's (in my opinion) a slightly nastier one to watch out for:

unsigned int x;
//...
x = 2;
if (x < -1) {
   printf("oops\n"); // This WILL be executed!
}

'avr-gcc -W' is your friend here:

$ avr-gcc -W -c test.c
test.c: In function `g':
testt.c:19: warning: comparison between signed and unsigned

Stefan