Writing bugs

[syzygy]

aside from what has been mentioned already, C operator precedence:
& has lower priority than != (I never understood why bitwise ops are lower than relational to be honest),
so the expression evaluates as x & (1 != engine) instead of the intended (x & 1) != engine
or maybe it was intended?

https://www.bell-labs.com/usr/dmr/www/chist.html

In B one writes

if (a==b & c) ...

to check whether a equals b and c is non-zero; in such a conditional expression it is better that & have lower precedence than ==. In converting from B to C, one wants to replace & by && in such a statement; to make the conversion less painful, we decided to keep the precedence of the & operator the same relative to ==, and merely split the precedence of && slightly from &. Today, it seems that it would have been preferable to move the relative precedences of & and ==, and thereby simplify a common C idiom: to test a masked value against another value, one must write

if ((a&mask) == b) ...

where the inner parentheses are required but easily forgotten.

(This is an explanation of how it came about but not a valid excuse.)

Funnily enough I made the same mistake today, but the compiler caught it for me. (See here.)

[syzygy]

x <= MAX_MOVES
x < MAX_MOVES

i < MAX_MOVES

[syzygy]

(This is an explanation of how it came about but not a valid excuse.)

An earlier and somewhat more complete explanation by Dennis Ritchie can be found here.

So early C only had & and |, which were interpreted as logical or bitwise operators depending on the context. Then Ritchie added && and || but felt uneasy about changing their precedence because it would create problems for existing programs:

Primarily at the urging of Alan Snyder, the && and || operators were added. This successfully separated the concepts of bitwise operations and short-circuit Boolean evaluation. However, I had cold feet about the precedence problems. For example, there were lots of programs with things like

if (a==b & c==d) ...

In retrospect it would have been better to go ahead and change the precedence of & to higher than ==, but it seemed safer just to split & and && without moving & past an existing operator. (After all, we had several hundred kilobytes of source code, and maybe 3 installations....)

So not fixing the precedence of bitwise operators saved having to check and fix "several hundred kilobytes of source code"... Ouch.

[mar]

So not fixing the precedence of bitwise operators saved having to check and fix "several hundred kilobytes of source code"... Ouch.

So now we know It's basically an excuse for laziness.

It's not only C, C++ but also D, C#, Java. It seems they fixed this problem in Rust and Go, however, which makes sense for new languages.

Imagine the amount of code that would be affected if one would attempt to fix this today,
but maybe it would still fix more code rather than break
We all made mistakes like this in the past, thinking of future generations.

Several hundred kb is nothing by todays standards, but maybe it was a big deal back then (storage media + number of files affected).

[Joost Buijs]

I wouldn't say adding unnecessary dummy variables is "the right way to do it".

Sometimes adding dummy variables can enhance readability, especially with complex expressions, but that is not the case here, I agree. They won't give overhead, because the compiler optimizes them away anyway.

The for (x = 0; x <= MAX_MOVES; ...) also looks fishy, one immediately expects that there is a buffer[MAX_MOVES] somewhere, which could give a nice buffer overflow.

[Ras]

Why didn't your compiler warn you?

Because e.g. for GCC, that requires -Wparentheses to be enabled, which it isn't by default. You need to enable it with -Wall and pay attention to the warnings. Then again, using compiler warnings would have spoiled hours of debugging fun.

[hgm]

In my experience complying with the -Wparentheses desires does more harm than good. Code with excessive numbers of parentheses gets completely unreadable. I know the operator priorities, thank you!

[chrisw]

Today I had a new one, took me hours to see it.

While parsing a PGN file looking for the moves of a certain engine I wrote the following (pseudo) code:

Code: Select all

printf ("Type engine"); gets(player);

if ((process_pgn_file())!=0) { all done }

if (strcmp(player,white)==0) engine=0;    // decide the color
if (strcmp(player,black)==0) engine=1; 

// Processing the moves

for (x=0; x<=MAX_MOVES; x++) { 
if (x&1 != engine) continue;
do_something_with_the_move_of_player();
}

So what was wrong?

Hi Ed,

I didn’t read any answers, so here my clean 2c in pseudocodelisch ...

if engine!=0 or 1 you have undefined situation (could arise if white or black is misspelt or missing)
if MAXMOVES is a constant, you may have some array[MAXMOVES] somewhere which could overflow, because test is >= MAXMOVES.
you’re playing one more move than exists (maybe) because >=

depending on what exactly MAXMOVES represents the code may well work fine on pass one, but danger of overflow reading higher moves left behind from previous passes.

Only stuk for three hours? I am stuck for three days at the moment, trying to work out why the latest “change” broke. I recommend very much writing code in Python with calls to C and running parallel whilst also allocating memory at start for use by the C calls. If you like mind numbing bugs.

Chris

[chrisw]

Oooff! Just noticed the operator precedence comments. I've been caught by those so many ties, I just liberally use brackets on everything.

[Rebel]

Hey Chris,

Through the years I noticed that most of such bugs one writes are in the lines you don't question, as in this case.

I felt stupid when I (as a sort of last resort) changed:

Code: Select all

if (x&1 != engine) continue;

into:

Code: Select all

y = x & 1;
if (y != engine) continue;

And was surprised it gave a different result.

What ?!

The code suddenly even performed as intended.

And I started to mistrust my compiler

Sometimes you can be totally blind for what's right before your nose.