1

I was making a simple min() function when I suddenly asked myself a question which got a bit more complex than I though

I created a valid function, and got the idea to omit a condition between the two numbers. Here is my function:

int min(int x, int y) {
    if (x > y) {
        return y;
    }
    else if (x < y) {
        return x;
    }
}

So it works perfectly for different numbers, if I put 2 and 3 inside, it'll return 2. But what happen when I put two time the same number? e.g. : 2 and 2

Neither 2 > 2 and 2 < 2 are valid, so it's just returning... nothing?

That what I though, I compiled the program (VS2015), got a warning about not testing every cases (normal), and when it runs... it outputs 2. Why?

After talking with people on it (and checking the ASM code for this function), someone checked with Valgrind what was happening, and as he told me, it appears that it could be a memory leak. I don't exactly know how it works, so why returning no value make it returning 2? Which 2 is returned?

I also tested if one of these conditions were true for some reason with a simple std::cout, but none of theme are true, so that's not something about "simplifying" with if (x > y) {...} else {...}

So what is really happening here?

[EDIT] I don't wanna know how to "fix" it, because it's pretty obvious to me. The question is, why am I getting 2? I know that there should be an else statement, but I was curious about what will happen without it

  • 1
    If you compiled this with warnings enabled, you'd see something like `warning: control reaches end of non-void function`. If you're not seeing warnings like that, you should change your compiler settings to allow you to see them, because it's very useful. The key here is that you don't have a final else clause. – Random Davis Oct 07 '16 at 20:54
  • 1
    I really fail to see why people want to gamble on undefined behaviour when they can just fix the code... – Jean-François Fabre Oct 07 '16 at 20:55
  • @Baum mit Augen The proposed duplicate does not answer the question why the function in question is returning `2`. – Violet Giraffe Oct 07 '16 at 21:03
  • @VioletGiraffe Do you think so? The accepted answer correctly explains how the missing return value is produced from `eax` in a not so different example and that it depends on the calling convention. – Baum mit Augen Oct 07 '16 at 21:07
  • *The question is, why am I getting 2?* -- Generate the assembly code and take a look at it. – PaulMcKenzie Oct 07 '16 at 21:09
  • @BaummitAugen: sorry, you're right. It's on the very last line and I missed it. – Violet Giraffe Oct 07 '16 at 21:10
  • @BaummitAugen so why `int min(int x, int y) { if (x > y) { return y; } else if (x < y) { return x; } int z = 3; } int main() { std::cout << min(2, 2) << std::endl; return{ 0 }; } ` isn't returning 3? – Lucas Benard Oct 07 '16 at 21:17
  • 1
    @LucasBenard Please [see this](http://rextester.com/AUZBQ80385). With optimizations turned on Visual Studio 2015 returns garbage. So you're going to spend time finding out why garbage is returned? – PaulMcKenzie Oct 07 '16 at 21:20
  • @LucasBenard The assignment gets probably optimized away by dead code optimization. If you care about the precise inner working of this flavor of UB in this special case, read up on the calling convention in use and check the generated assembly. – Baum mit Augen Oct 07 '16 at 21:20
  • @PaulMcKenzie so the only reason why I'm getting 2, is that VS15 just take the last number? With another compiler, it should give me what the link you gave me returned? – Lucas Benard Oct 07 '16 at 21:22
  • 2
    @LucasBenard The compiler used at the link **is** Visual Studio 2015, but with optimizations tuned on (not "debug" mode). The point being is that it gets pointless in spending time on something you can't predict. Maybe in the next version of Visual Studio, you get -32434, 0, 13, 8927, 2, you don't know. – PaulMcKenzie Oct 07 '16 at 21:23

4 Answers4

3

Flowing off the end of a function results in undefined behavior in a value-returning function. The behavior of your program is undefined.

In practice this means the caller is going to try to read the returned int according to whatever calling convention is being used, and fail to do so.

GManNickG
  • 494,350
  • 52
  • 494
  • 543
1

You are not returning anything when x and y are equal. As a consequence, the program has undefined behavior when x is equal to y.

Use:

int min(int x, int y) {
    if (x > y) {
        return y;
    }
    else {
        return x;
    }
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • 1
    This was a point worth making, and the right way to implement this. – Juan Tomas Oct 07 '16 at 20:56
  • @JuanTomas: it's tagged C++. The right way is `std::min`. Or at least the one-liner: `return x < y ? x : y;` – Violet Giraffe Oct 07 '16 at 20:57
  • I think the point of the exercise is to write your own `min()`. But yeah, the ternary operator is arguably the better way. What I zeroed in on was the mistaken `else if`. – Juan Tomas Oct 07 '16 at 21:03
1

To understand how and why exactly it behaves, you need to study the calling convention used here.

I believe the typical x86 C/C++ calling convention is to push (as in, the ASM command) the arguments onto the stack, call the function, and then pop the result. In case you skip both if branches and don't call return, there is no push instruction generated. But the caller will still pop. What will it pop? Whatever ends up on the stack in that spot, which could be anything - there's no way of knowing. And I think it will corrupt the stack in doing so, or more precisely - put it in the state that the other stack users don't expect. The program may stop working properly altogether, the way I see it, since your stack is mixed up now. Bottom line: don't let non-void function exit without a return statement, ever.

Update: I looked at the actual assembly using Godbolt (e. g. https://godbolt.org/z/8emYlB) and I can now see I was wrong about the calling convention typically used; looks like the stack isn't used for return values, eax register is. So at least a missing return will not cause heap corruption, but you will still get an undefined return value - whatever happened to be in the accumulator register at that moment.

Violet Giraffe
  • 32,368
  • 48
  • 194
  • 335
0

You just need to add an else statement because the 2=2 neither fulfills the if(x > y) condition, nor it fulfills the else if(x < y) condition. So it can't return anything. Just add an else and return the same x value.

int min(int x, int y) 
{
    if (x > y)
    {
        return y;
    }
    else 
    {
        return x;
    }
}