Jump to content
Search In
  • More options...
Find results that contain...
Find results in...
Sign in to follow this  
Quasar

Undefined behavior

Recommended Posts

   span.xfrac = 
      (unsigned int)((-plane.pviewy + plane.yoffset + (-plane.pviewcos * realy)
                      + ((x1 - view.xcenter) * xstep)) * plane.fixedunit);
   span.yfrac = 
      (unsigned int)((plane.pviewx + plane.xoffset + (plane.pviewsin * realy)
                      + ((x1 - view.xcenter) * ystep)) * plane.fixedunit);
   span.xstep = (unsigned int)(xstep * plane.fixedunit);
   span.ystep = (unsigned int)(ystep * plane.fixedunit);
Evidently this code (from EE's Cardboard renderer) has the potential for undefined behavior.

C99 Standard says:
When a finite value of real floating type is converted to an integer type other than _Bool, the fractional part is discarded (i.e., the value is truncated toward zero).If the value of the integral part cannot be represented by the integer type, the behavior is undefined.

This would seem to mean that conversion of a negative float value to unsigned int is undefined, unlike the conversion of a negative int to unsigned.

Anybody know if this is true?

The reason I ask is that the above code seems to be responsible for causing this:
http://maniacsvault.net/eternitymac.png

This only happens on Mac. Not in Windows, and not on Linux 32-bit nor 64-bit.

Share this post


Link to post

Generally speaking I would never rely on the behavior of automatic conversion between unsigned and signed when using different types. This is a prime ground for inter-compiler variation.

Can't you simply abs() the result before casting to unsigned int?

Share this post


Link to post

Try to use (int) instead of (unsigned int)

Revision: 2925
Message: Fix weird type casting bug on Mac.
----
Modified : /trunk/prboom2/src/r_main.c

angle_t R_PointToAngleEx(fixed_t x, fixed_t y)
{
...
- oldresult = (angle_t)(atan2(y, x) * ANG180/M_PI);
+ oldresult = (int)(atan2(y, x) * ANG180/M_PI);


Similar code from prboom-plus:

dsvars->xstep = (fixed_t)(viewsin * slope * viewfocratio);
dsvars->ystep = (fixed_t)(viewcos * slope * viewfocratio);

dsvars->xfrac =  viewx + xoffs + (int)(viewcos * realy) + (x1 - centerx) * dsvars->xstep;
dsvars->yfrac = -viewy + yoffs - (int)(viewsin * realy) + (x1 - centerx) * dsvars->ystep;

Share this post


Link to post
Quasar said:

This would seem to mean that conversion of a negative float value to unsigned int is undefined, unlike the conversion of a negative int to unsigned.

How about a two-step conversion? First from float to signed int, and then from signed int to unsigned int.

span.xstep = (unsigned int)((int)(xstep * plane.fixedunit));

Share this post


Link to post
DaniJ said:

Generally speaking I would never rely on the behavior of automatic conversion between unsigned and signed when using different types. This is a prime ground for inter-compiler variation.

Can't you simply abs() the result before casting to unsigned int?

Only SoM could answer this question, because he understands precisely how the mask & shift business is working in the span drawers. I'm still not quite clear myself on what this is supposed to do when the result is a negative number.

Share this post


Link to post

Ah I see. In that case I would hazard a guess that the intention is a low end clamp to zero unless spans in cardboard wrap somehow.

Share this post


Link to post

Int cast doesn't work at all, in fact,

(unsigned int)((int)fabs(expression)) duplicates the mac bug, so I need to figure out what VC++ is doing and try to force that method.

Share this post


Link to post
Quasar said:

This only happens on Mac. Not in Windows, and not on Linux 32-bit nor 64-bit.

Which type of Mac? Using which architecture? Using which version of GCC?

Really, it is undefined to do cast a negative number to an unsigned type. It completely depends on the compiler and the target system. Usually casting of any type of the same size is just the same result, i.e. *((UInt32*)(&Int32Var)). However, on types of not the same size it really depends. Ints casted to a higher size type get a bunch of 0xFFs added to the high bytes. UInts casted to a higher size type will get 0x00s added to the high bytes. When casting a negative int to an unsigned higher int, the result is undefined really since it would depend on the compiler and/or the system. But everyone casts signed and unsigned types to each other so nobody really cares about it and just a representation of the same data in memory is now to be expected.

Anyway, maybe you could try inverting all the data, that is positive becomes negative and negative becomes positive. 10 + -20 = -10, -10 + 20 = 10. Any negative resulting value would be positive. Then you can negative the unsigned int (depending on implementation) or duplicate it's behavior without compiler dependent casting (maybe ((int - 1) ^ ~0)?).

You could also try some printfs, one of the way other compilers do it and the way the mac does it. Unless you are the "that is not the proper way to do it, the proper way is to put a breakpoint in that spot and stare blankly at the data for everytime it stops and then do the same for the other systems and compare the data. Only idiots would print all the data into a list that can easily be compared to another list even though printf can print the data in the way I want it to be presented and I won't have to blindly step through a debugger when this point of failure isn't really a point of failure if the data is the exact same so I would have wasted my time digging around manually but that's me and I like to use the debugger when some temporary code injection would be more useful. Despite my fear of committing that code when I know I would never do that anyway." kinda person.

Share this post


Link to post
GhostlyDeath said:

Really, it is undefined to do cast a negative number to an unsigned type.

Strictly speaking yes, however it is practically impossible to write any C program that never casts a signed value to an unsigned value. On all platforms I am aware of, casting int to unsigned int results in a reinterpretation of the bit pattern as one would expect.

GhostlyDeath said:

Anyway, maybe you could try inverting all the data, that is positive becomes negative and negative becomes positive. 10 + -20 = -10, -10 + 20 = 10. Any negative resulting value would be positive. Then you can negative the unsigned int (depending on implementation) or duplicate it's behavior without compiler dependent casting (maybe ((int - 1) ^ ~0)?).

I totally don't see what this would accomplish.

GhostlyDeath said:

You could also try some printfs, one of the way other compilers do it and the way the mac does it.

I don't have access to a Mac, genius. Thank you, come again.

SoM has an attempt at a fix in the SVN right now, but unfortunately he has accidentally committed some experimental branch dependencies into trunk, and so EE cannot currently compile.

Share this post


Link to post
magicsofa said:

Lol this is way over my head but the bug looks really cool :P

Basically in C you have two kinds of numbers. One is integer, and only represents whole numbers like 1, 2, 4, or 1337. The other is floating point, and can have decimal values, like 0.6667. If you cast a floating point to an integer, you lose the decimal part of it, and, if the floating point value is too large or has an invalid value for the integer type you're goinog to convert it into, the results are unfortunately considered variously "undefined" or "implementation dependent" by the C standard.

That means it can work differently on every single compiler :/

This particular code has *two* such problems. The first is that the numbers it generates can be too large in terms of absolute value. The second is that they can be negative (less than 0) when trying to assign to a value that only represents numbers greater than zero.

It does look pretty weird though ;)

Share this post


Link to post
SoM said:

Int cast doesn't work at all, in fact,

(unsigned int)((int)fabs(expression)) duplicates the mac bug, so I need to figure out what VC++ is doing and try to force that method.

Interesting. In that case it sounds like VC++ is merely flipping the sign where as the Mac is shifting/sign contracting. Are only really large/small values problematic or is it uniform?

Have you tried writing your own method to perform the value conversion per-bit? You could do this on your own dev system to at least figure out what logic you are relying on.

Share this post


Link to post
Quasar said:

I don't have access to a Mac, genius. Thank you, come again


Ask some of your mac buddies for SSH, that's how I got ReMooD working on a mac.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
Sign in to follow this  
×