Bug 6432 - Undefined behavior
Status: RESOLVED FIXED
Alias: None
Product: ioquake3
Classification: Unclassified
Component: Platform
Version: GIT MASTER
Hardware: All All
: P3 normal
Assignee: Zachary J. Slater
QA Contact: ioquake3 bugzilla mailing list
URL:
Depends on:
Blocks:
 
Reported: 2016-06-22 10:19 EDT by Eugene C.
Modified: 2016-09-07 19:56:58 EDT
1 user (show)

See Also:

Attachments
Suggested solution (2.61 KB, patch)
2016-06-22 10:19 EDT, Eugene C.

Description Eugene C. 2016-06-22 10:19:19 EDT 
Created attachment 3558 [details]
Suggested solution

Undefined behavior in shift operation if shift counter is greater or equal 32 (MAX_DLIGHTS atm): 

R_RecursiveWorldNode( tr.world->nodes, 15, ( 1 << tr.refdef.num_dlights ) - 1 );

So, for example, on x86 arch ((1<<[volatile]32)-1) produces 0x0 instead of 0xFFFFFFFF because shift counter is masked to 5 bits
Comment 1 Eugene C. 2016-06-22 13:53:02 EDT 
Here is another jumpless macro that correctly handles overflows (i.e. always returns 0xFFFFFFFF) for input values in range [33..63]: 
#define MASK_32BITS(shift) ( ( ( (1 << (shift)) )  & ~( (((shift) & 0xFFFFFFE0 ) >> 5) << shift ) ) - 1 )
Jumpless version that correctly handles 64-bit integers but not overflows[33..63]:
#define MASK_32BITS(shift) ( ( ( (1 << (shift)) ) & ~( ((shift) & 32) >> 5 ) ) - 1 )
Comment 2 James Canete 2016-09-07 19:56:58 EDT 
I don't like the (1 << (shift)) that's in all these macros, it's still undefined.

So I used (1ULL << shift) instead.

https://github.com/ioquake/ioq3/commit/497a74f22a39cbf8694e5d8567f3113f03ba3620