60

I'm having a disagreement with some co-workers over the following code:

int foo ( int a, int b )
{
    return b > 0 ? a / b : a;
}

Does this code exhibit undefined behavior?

EDIT: The disagreement started from what appears to be a bug in an overly-eager optimizing compiler, where the b > 0 check was optimized out.

Luchian Grigore
  • 253,575
  • 64
  • 457
  • 625
  • 16
    Is there any reason to think it does? – juanchopanza Oct 21 '16 at 09:14
  • 11
    I mean it only is UB when you execute a division by 0. You don't here. – Hayt Oct 21 '16 at 09:14
  • @juanchopanza apparently... – Luchian Grigore Oct 21 '16 at 09:18
  • 23
    @LuchianGrigore I guess his point was, if your coworkers (or you) take the position this exhibits UB, perhaps the reason for *why* they (or you) think so would be a worthy addition to your question. – WhozCraig Oct 21 '16 at 09:21
  • 45
    Huh? Is is `return p ? p->flag_value : false` UB when `p` is null? No. *All* code would be broken if that were the case. – Kerrek SB Oct 21 '16 at 09:37
  • @KerrekSB, BTW I would rather write that as: `return (p != nullptr && p->flag_value);`. :-) – iammilind Oct 21 '16 at 10:07
  • 7
    It would also be UB if the operation generated overflow. But for integer division that can only happen if you divide INT_MIN by -1, and the guard (b>0) also avoids that case. – Hans Olsson Oct 21 '16 at 14:11
  • 3
    Maybe it is because some other programming languages would execute both expressions in the `? :` and especially in the `&&` case. The co-workers might not know that C/C++ behaves in this way or fear that there are compilers which behave non-standard-conforming. – Martin Rosenau Oct 21 '16 at 15:31
  • No, but instead I can define Foo to return `int?` or `int.MaxValue` in the case of a division by zero. Note in math it's considered a undefined operation. Also don't understand why you cannot divide by negative. – jean Oct 21 '16 at 15:50
  • @jean - I've not come across your int.MaxValue or int? before in C++. What are they exactly ? – Ian Cook Oct 21 '16 at 17:57
  • @IanCook No such things exist in C++. jean must be thinking of C# et al. – underscore_d Oct 21 '16 at 18:44
  • @underscore_d Yep sorry guys, just get distracted a bit. The point is: OP is basically creating a custom definition where there's no one. That can have serious implications in BL – jean Oct 21 '16 at 19:14
  • As a comparative exercise: is there an executable difference between `return b > 0 ? a / b : a` and `if (b > 0) { return a / b; } else { return a; }`? How does your co-worker (judging by your 'apparently...' comment, it is the coworker that thinks there's a problem) do `null` value checking before usage, if not `return (x != null) ? x.Property : [null case value]` nor `if (x != null) { return x.Property; } else { return [null case value]; }`, nor an equivalent? – Daevin Oct 21 '16 at 19:33
  • 1
    Why do you not answer @WhozCraig’s request to add the reasoning of those who hold this to exhibit undefined behaviour? I’d vote it ▲ for that, but till then it is ▼! – PJTraill Oct 21 '16 at 21:45
  • 2
    Wow I certainly didn't expect this kind of traction. If you're curious, the disagreement was sparked by what I believe is a bug in a compiler - it would agressively optimize out the check, as it proves incorrectly (which seems clear to me, but not others, ergo the disagreement). – Luchian Grigore Oct 21 '16 at 21:55
  • 3
    Afaik not even `int f() { return 1/0; }` exhibits undefined behavior. You must call it to invoke undefined behavior. – Johannes Schaub - litb Oct 21 '16 at 22:23
  • Looking at the function in a header only, the return result when b = 0 is UNEXPECTED behavior but not undefined (I wouldn't expect the return result the OP is returning). In my opinion returning "a" is wrong and can lead to very hard to find bugs. Also, it might be better to compare (b === 0) instead of (b > 0) to allow for negative integer input values... which in this example would also return very unexpected results. – Phil M Oct 21 '16 at 23:04
  • @PhilMobley: There's actually numeric code where such patterns occur. Roughly speaking, in numerical optimization algorithms, you might have a step size `a` that you occasionally want to lower, when the function to optimize has some problematic points with strong gradient changes. You might define a step reduction factor `b` which in well-behaved cases is <1, but potentially ends up negative. You can then of course define a `b_positive = std::max(1,b)` but that is equivalent to the code above. – MSalters Oct 22 '16 at 13:01
  • 1
    Do they think that checking `argc > 1` before using `argv[1]` also has UB? If so, avoiding UB would be impossible, and the C standard would be absolutely useless. The UB of division by zero is about *evaluated* expressions, and because of the sequencing specified for the conditional operator, `a / b` is never evaluated when `b == 0`. –  Oct 22 '16 at 15:39

4 Answers4

111

No.

Quotes from N4140:

§5.16 [expr.cond]/1

Conditional expressions group right-to-left. The first expression is contextually converted to bool. It is evaluated and if it is true, the result of the conditional expression is the value of the second expression, otherwise that of the third expression. Only one of the second and third expressions is evaluated.

Further:

§5 [expr]/4

If during the evaluation of an expression, the result is not mathematically defined or not in the range of representable values for its type, the behavior is undefined.

This clearly does not happen here. The same paragraph mentions division by zero explicitly in a note, and, although it is non-normative, it's making it even more clear that its pertinent to this situation:

[ Note: most existing implementations of C++ ignore integer overflows. Treatment of division by zero, forming a remainder using a zero divisor, and all floating point exceptions vary among machines, and is usually adjustable by a library function. —end note ]

There's also circumstantial evidence reinforcing the above point: the conditional operator is used to conditionally make behavior undefined.

§8.5 [dcl.init]/12.3

int f(bool b) {
  unsigned char c;
  unsigned char d = c; // OK, d has an indeterminate value
  int e = d; // undefined behavior
  return b ? d : 0; // undefined behavior if b is true
}

In the above example, using d to initialize int (or anything other than unsigned char) is undefined. Yet it is clearly stated that the UB occurs only if the UB branch is evaluated.

Going out of language-lawyer perspective: if this could be UB, then any division could be treated as UB, since the divisor could potentially be 0. This is not the spirit of the rule.

gsamaras
  • 71,951
  • 46
  • 188
  • 305
krzaq
  • 16,240
  • 4
  • 46
  • 61
  • 25
    @LuchianGrigore The essence of the answer to your question is entirely in the italicized sentence (*Only one of the second and third expressions is evaluated*). If someone doesn't understand that property of the conditional operation, they may generate many more questions like yours where division by zero is replaced by dereferencing a null pointer, out of bounds access, an expected-but-not-observed important side-effect, etc. – Leon Oct 21 '16 at 10:16
  • 1
    -1: The question (as written in the title) does not ask whether the division is evaluated - it asks whether the division can be UB despite not being evaluated. – user253751 Oct 22 '16 at 13:23
  • 1
    @immibis it does, see comment by Leon. – Ruslan Oct 22 '16 at 14:13
  • 1
    @immibis It follows from the second quote in the answer: the reason why division by zero is UB is because of `§5 [expr]/4`. Note the words "during the *evaluation* of an expression" and "mathematically defined". The first quote tells when the "not mathematically defined" expression is evaluated, which is: never. – milleniumbug Oct 22 '16 at 15:05
  • To most people who know C(++) it is obvious that the branch not taken is not *evaluated* - i.e. a function call there would not be executed. But I find it not obvious that code there cannot lead to undefined behavior, since UB can frequently "leak" or "time-travel". I could imagine a compiler sees this and then assumes b cannot be zero (*assuming absense of UB*, the indirect way UB often bites you)... then removing a check for `b != 0` in later code. – jdm Oct 23 '16 at 10:38
  • 1
    @jdm I assume you're referring to Raymond Chen's article when speaking of time travel. You'll notice that the problem there is that the UB is invoked before the check. An analogue here would be `cout << "a/b: " << a/b << endl; return b != 0 ? a / b : a;`. – krzaq Oct 23 '16 at 16:57
  • What if the processor executes a branch which is finally discared because of a failed branch prediction? – ABu Oct 26 '16 at 18:07
  • @jdm: At least in C, the way the Implementation Limits are included, nothing would forbid an implementation where the amount of stack space allocated by a function would for some bizarre reason be higher if there were any code path within the function that would always cause a division by zero, even if the amount of increase was sufficiently large as to make all such functions bomb the stack. I think the C++ rules for implementation limits are less loosy-goosy, but I'm not sure the same principle wouldn't apply. – supercat Oct 27 '16 at 18:30
15

There is no way of dividing with zero in the example code. When the processor executes a / b, it has already checked that b > 0, therefore b is non-zero.

It should be noted that if a == INT_MIN and b == -1, then a/b is undefined behaviour too. But this is prevented anyway because the condition evaluates to false in that case.

Although I am not really sure you meant return b != 0 ? a / b : a; and not return b > 0 ? a / b : a; If b is less than zero, the division is still valid, unless it is the condition described above.

glauxosdever
  • 652
  • 4
  • 11
  • 2
    Hans Olsen points out a case where `a / b` can still be undefined for negative `b` in the comments. – chepner Oct 21 '16 at 14:42
10

Does this code exhibit undefined behavior?

No. It doesn't. The expression 

return b > 0 ? a / b : a;

is equivalent to 

if(b > 0)
    return a/b;     // this will be executed only when b is greater than 0
else
    return a;

Division only performed when b is greater than 0.

haccks
  • 104,019
  • 25
  • 176
  • 264
  • 1
    -1: The question (as written in the title) does not ask whether the division is evaluated - it asks whether the division can be UB despite not being evaluated. – user253751 Oct 22 '16 at 13:24
  • 2
    @immibis; *The question (as written in the title) does not ask whether the division is evaluated*: true, but answer lies in the evaluation of the division expression. There is no division by zero here. – haccks Oct 22 '16 at 13:38
  • It's a statement, not an expression. – Alex Celeste Oct 22 '16 at 15:48
  • 1
    @Leushenko; `a/b` is an expression. – haccks Oct 22 '16 at 16:00
3

If this were UB then so would

if(a != null && *a == 42)
{
 .....
}

And the sequencing of ifs , ands and ors is clearly designed to specifically allow this type of construct. I cant imagine your colleagues would argue with that

pm100
  • 48,078
  • 23
  • 82
  • 145