1

While reviewing someone's code, I have encountered a situation similar to this following one where the error (which is basically some poor programming practice) is not quite directly visible. Depending on the compiler used, i/i++ might be either 0 or 1.

int foo(int n) {
    printf("Foo is %d\n", n);
    return (0);
}

int bar(int n) {
    printf("Bar is %d\n", n);
    return (0);
}

int main(int argc, char *argv[]) {

    int x = 0;
    int(*(my_array[3]))();
    int i = 1;
    int y = i/++i;

    printf("\ni/++i = %d, ", y);

    my_array[1] = foo;
    my_array[2] = bar;
    (my_array[++x])(++x);
}

Therefore, the output is either Foo is 2, or Bar is 2.

My questions may be considered too broad, but I want to know:

  1. Why is this happening / why is this allowed by the compiler? (I checked on several compilers and there was no warning at all)
  2. How can we correct this kind of strange behavior? (For instance, the project I was working for was huge; what will happen if worse things like heap exploitation / bss overflows or inconsistent synchronization are allowed by the compiler as well? One does not simply sleep well at night after realizing this.)
  3. I realize there are dozens of coding style books out there on the market, but how will another programmer decide which one of the outputs is the best? (supposing there is no expected output - Foo is 2 and Bar is 2 don't mean anything to the programmer working with the code)
George Netu
  • 2,758
  • 4
  • 28
  • 49
  • 5
    `i / ++i` is undefined behavior. The compiler is allowed to make anything happen. There is no other solution than re-writing the code into correct C. – 5gon12eder Dec 19 '14 at 01:01
  • 2
    http://en.wikipedia.org/wiki/Sequence_point – John Zwinck Dec 19 '14 at 01:02
  • 4
    BTW, GCC with `-Wall` *does* give a warning about this. – 5gon12eder Dec 19 '14 at 01:03
  • 1
    1 and 2 are dupes of every question about UB and sequence points. Question 3 appears to me to be, "supposing there is no expected output, how will another programmer decide what is the expected output". Well, they won't. They'll fail unless there are some clues allowing them to figure out what to expect, and you've supposed that away. – Steve Jessop Dec 19 '14 at 01:05
  • 1
    http://stackoverflow.com/questions/4176328/undefined-behavior-and-sequence-points and http://stackoverflow.com/questions/2397984/undefined-unspecified-and-implementation-defined-behavior are our FAQ references. I don't want to mark the question dupe though because that would dupehammer it, and someone might rescue question 3 by interpreting it more kindly than I have and come up with some genuine good answer to the question, "how can I resolve code with unclear intent, specifically this code?". – Steve Jessop Dec 19 '14 at 01:07
  • 1
    Considering the third question: generally speaking, the answer is probably: "it's impossible". But here is one case, in which you can make a good guess: if you know the conditions, which were used to produce a working program. Compile the program with a compiler (and it's specific options) that is believed to produce the correct result and examine, how the program behaves in that particular case. After that, of course, the UB should be fixed. – Mikhail Maltsev Dec 19 '14 at 02:21

2 Answers2

5

i/++i causes undefined behaviour, as does my_array[++x](++x).

It's not an operator precedence or order-of-evaluation issue; it's a sequencing issue. (The coder may have believed there was sequencing of side-effects that corresponded to the precedence, when in fact there isn't). See this thread for a more detailed description.

This sort of error can be avoided by not using increment or assignment operators as sub-expressions unless you are really sure about what you're doing. In these cases:

int y = i / (i+1);
++i;

and 

my_array[x+1](x+1);
x += 2;

or whatever was intended.

The compiler is not obligated to detect any of this, although some compilers do anyway. Recent versions of gcc should warn about i/++i. If you think this isn't good enough then consider joining up with the gcc development team :)

There are static code analysis tools around that will try to do a better job of detecting undefined behaviour.

Community
  • 1
  • 1
M.M
  • 138,810
  • 21
  • 208
  • 365
1

The only answer to your question is that the programmers should be aware about the sequence points. According to me most do not have any idea about the same and it causes a lot of issues in the industry. It is one of my favorite interview questions. Newbees would fail that question for sure but frankly many with long experience have also failed the question.

Sequence points exist to allow compilers some room for optimization i.e. a compiler is free to decide the order of evaluation between two sequence points based on the target processor. However, while allowing for optimization, it sets the rule for the compilers that all the required computation should be completed at certain points. This is so that at least something is predictable.

Operator precedence and associativity is a different matter all together.

However, one thing that you can do is to set rules - no multiple updates to a variable in the same expression. No mixing of update and usage of a variable in the same expression. Use parenthesis where possible to ensure readability and order of computation.

Yet another thing one can do is to write scripts to check the source code to catch such errors. Thus, neither the author nor the reviewer misses such a mistake.

You might also use static analysis tools see here

RcnRcf
  • 356
  • 1
  • 8