68kMLA Classic Interface
This is a version of the 68kMLA forums for viewing on your favorite old mac. Visitors on modern platforms may prefer the main site.
| Click here to select a new forum. | | Mask and Bit Shift gotcha | Posted by: David Cook on 2024-04-09 07:21:16 I recently ran into a silent bug that I wanted to alert others to.
In my case, the code is looking for a destination drive and wants to skip remote volumes.

The little dashes on the far left side of the above image indicate where you can set a breakpoint in the debugger. Hey! Why can't I set a breakpoint in the code where I check the vMAttrib? In fact, when debugging, it skips over the code I've highlighted in yellow.
bHasExtFSVol is equal to 16 in Apple's Files.h header. (1 << bHasExtFSVol) should give me the bit mask for that, right?
Well, '1' is an int. An int is 16 bits by default in most classic Mac compiler implementation. Bits are labeled 0 to 15. So, 1 << 16 is one more bit beyond the length of an int. The compiler sees that as the value zero (all the bits are shifted outside the range of an int, leaving nothing.) volumeParms.vMAttrib & 0 is always false. The compiler just quietly removed the code since it is unreachable. Thus, no breakpoints.
I am using Metrowerks CodeWarrior 11. It does not produce a warning for the removed code, which is a bummer.
The fix is simple. Put an 'L' or 'UL' next to the 1 to indicate a long (or unsigned long) constant. That's 32 bits.
(1UL << bHasExtFSVol)
Fortunately, no one else would ever get caught by this bug!
Oops. Here is the bug in Apple's header for Movies.h. More embarrassingly, the correct syntax appears for a different value immediately below the bug.

They probably should not have used flags or bitfields longer than 16 bits. Besides avoiding this bug, shorter flag fields are easier for a human to examine. Trying to spot the 19th bit by eye is a pain.
Hope this helps,
- David | Posted by: Phipli on 2024-04-09 07:43:28
I recently ran into a silent bug that I wanted to alert others to.
In my case, the code is looking for a destination drive and wants to skip remote volumes.
View attachment 72318
The little dashes on the far left side of the above image indicate where you can set a breakpoint in the debugger. Hey! Why can't I set a breakpoint in the code where I check the vMAttrib? In fact, when debugging, it skips over the code I've highlighted in yellow.
bHasExtFSVol is equal to 16 in Apple's Files.h header. (1 << bHasExtFSVol) should give me the bit mask for that, right?
Well, '1' is an int. An int is 16 bits by default in most classic Mac compiler implementation. Bits are labeled 0 to 15. So, 1 << 16 is one more bit beyond the length of an int. The compiler sees that as the value zero (all the bits are shifted outside the range of an int, leaving nothing.) volumeParms.vMAttrib & 0 is always false. The compiler just quietly removed the code since it is unreachable. Thus, no breakpoints.
I am using Metrowerks CodeWarrior 11. It does not produce a warning for the removed code, which is a bummer.
The fix is simple. Put an 'L' or 'UL' next to the 1 to indicate a long (or unsigned long) constant. That's 32 bits.
(1UL << bHasExtFSVol)
Fortunately, no one else would ever get caught by this bug!
Oops. Here is the bug in Apple's header for Movies.h. More embarrassingly, the correct syntax appears for a different value immediately below the bug.
View attachment 72319
They probably should not have used flags or bitfields longer than 16 bits. Besides avoiding this bug, shorter flag fields are easier for a human to examine. Trying to spot the 19th bit by eye is a pain.
Hope this helps,
- David Does that mean dfTextColorHilite was removed during compilation for every program that ever included this header file? | Posted by: joevt on 2024-04-09 16:37:57
Does that mean dfTextColorHilite was removed during compilation for every program that ever included this header file? Depends on the compiler or compiler settings. I think newer CodeWarrior compilers will assume 32 bit? Or the PowerPC compiler does anyway.
@David Cook didn't show that there is a problem with that header file.
#include <SIOUX.h>
#include <Movies.h>
main() {
long x = dfTextColorHilite;
printf("%lx\n", x);
}
If there's no problem, then it should output 10000
If there's a problem, then it might output 0. | Posted by: David Cook on 2024-04-09 17:30:21
Depends on the compiler or compiler settings
Correct.
Nice code example, btw.
As expected, your code outputs 0 on the 2-Byte Ints setting.
When I switch to 4-Bytes Ints, it outputs 10000 (hex). Importantly, default projects seem to be set to 4-Byte Ints in this version of CodeWarrior.

No such option exists on the PowerPC side. Here's what Metrowerks says:

Although this is C standard, I feel like this is a compiler problem, though. The header is just defining a constant value, which should only become typed in context of the usage. To me, 12345 is not a short or a byte or a long.
Apple's source code uses 'int' for a wide variety of purposes. In some cases, it is clearly used as 4 bytes or 2 bytes. In other cases, the programmer didn't case (loops). In a few cases, they used it as a bool. Ha!
Inside Macintosh focused on Pascal. That's not a fair comparison. Yet, they use the word 'integer' to describe 2 byte values.

Does that mean dfTextColorHilite was removed
Not quite, but still quietly nasty. It just equates to 0 with two-byte ints. So, if you check the flag, it will be wrong. If you set the flag, it will not set.
long test = dfTextColorHilite; // Sets the value to 0, not the 0x10000. | Posted by: David Cook on 2024-04-09 18:05:20 Just to run this into the ground...
I first learned C on a dual-floppy Macintosh SE with Lightspeed C. Here's what that manual has to say:

After a couple of years, I stopped using ints and switched to short and long. Apple switched to more convoluted naming conventions, which were nevertheless based on char, short, and long.

But this brings me back to:
1. (1 << 16) should not resolve to zero regardless of the size of an int.
2. A warning should be generated.
I believe modern compilers will make the constant whatever size is necessary, and then produce a warning if the value is too big. Yup.

Still, anyone developing on older computers must accept the conditions at the time, I suppose. : )
- David | Posted by: sfiera on 2024-04-09 20:48:34
I believe modern compilers will make the constant whatever size is necessary, and then produce a warning if the value is too big. Yup. That’s a slightly different error. (1 << 16) isn’t zero there, since int is wide enough to represent the value. The issue is in converting that value down to the actual type. Overflow during the calculation gives you different errors:
% echo 'int main() { long long x = 1 << 32; }' | clang -x c -
<stdin>:1:30: warning: shift count >= width of type [-Wshift-count-overflow]
int main() { long long x = 1 << 32; }
^ ~~
1 warning generated.
% echo 'int main() { long long x = (1<<16) * (1<<16); }' | clang -x c -
<stdin>:1:36: warning: overflow in expression; result is 0 with type 'int' [-Winteger-overflow]
int main() { long long x = (1<<16) * (1<<16); }
^
1 warning generated.
% echo 'int main() { long long x = 1ll << 32; }' | clang -x c -
% echo 'int main() { long long x = (1ll<<16) * (1ll<<16); }' | clang -x c - | Posted by: cheesestraws on 2024-04-10 01:44:32
Apple switched to more convoluted naming conventions, which were nevertheless based on char, short, and long.
As someone who likes modern C's uint32_t etc types for clarity, I appreciate these Apple types quite a lot. Int types with unspecified widths are grotty. | Posted by: David Cook on 2024-04-10 07:51:31
That’s a slightly different error.
Ahh. How true! Good catch.
I appreciate these Apple types quite a lot.
I agree. When you are documenting a platform, the types and structures should be unambiguous to avoid bugs. | | 1 |
|