Problem Buggy Deltaglider code.

[thepenguin]

I am currently developing a vessel, but whenever I engaged the main engine, I was flung into NaN-space. I managed to track the problem down to this single line of code (when I comment it out I can fly freely), but I have no idea what is wrong with it.

If anyone has insights into what could be wrong, I would appreciate knowing how to fix it.

Never mind. Just didn't notice one of the bugs in the DeltaGlider code. This line:

for (i = 0; i < nabsc-1 && AOA[i+1] < aoa; i++);

should read

for (i = 0; i < nabsc-1 && AOA[i+1] < aoa; i++);
i--;

It's a minor issue until it bites you. I didn't notice it at first, but then the undefined behavior (out-of-array element access) flung my ship into NaN-space.

EDIT: updated the code a little more.

 

[Hielor]

I am currently developing a vessel, but whenever I engaged the main engine, I was flung into NaN-space. I managed to track the problem down to this single line of code (when I comment it out I can fly freely), but I have no idea what is wrong with it.

If anyone has insights into what could be wrong, I would appreciate knowing how to fix it.

Never mind. Just didn't notice one of the bugs in the DeltaGlider code. This line:

for (i = 0; i < nabsc-1 && AOA[i+1] < aoa; i++);

should read

for (i = 0; i < nabsc-2 && AOA[i+1] < aoa; i++);

It's a minor issue until it bites you. I didn't notice it at first, but then the undefined behavior (out-of-array element access) flung my ship into NaN-space.

Assuming that "nabsc" is the number of elements in the array, that shouldn't result in an array overrun...

 

[jedidia]

Assuming that "nabsc" is the number of elements in the array, that shouldn't result in an array overrun...

Yes, it would, since i+1 is checked. If nabsc is the number of elements in AOA, then checking AOA[i+1] is access outside the array (Preferably you'd get an exception thrown here, but if you're not accessing memory alocated by another process, you won't).

What I think happened is that in the DG-code nabsc is indeed the highest index in AOA, in which case the code would work flawlessly, but it would be easy to get confused when copying the code into another project and assign the number of elements instead, since that is the usual way to do it.

 

[Hielor]

Yes, it would, since i+1 is checked. If nabsc is the number of elements in AOA, then checking AOA[i+1] is access outside the array (Preferably you'd get an exception thrown here, but if you're not accessing memory alocated by another process, you won't).

What I think happened is that in the DG-code nabsc is indeed the highest index in AOA, in which case the code would work flawlessly, but it would be easy to get confused when copying the code into another project and assign the number of elements instead, since that is the usual way to do it.

Ah, you're right, clearly I shouldn't think about coding questions within the first ten minutes of waking up.

 

[Enjo]

Yes, it would, since i+1 is checked. If nabsc is the number of elements in AOA, then checking AOA[i+1] is access outside the array (Preferably you'd get an exception thrown here, but if you're not accessing memory alocated by another process, you won't).

A crash unortunately. Exceptions are only thrown if you access an STL container with .at() method, like AOA.at(i+1), but the AOA most probably isn't such a container to have the method anyway.

 

[jedidia]

A crash unortunately.

Ah yes, I was actually thinking of an access violation, not an exception

clearly I shouldn't think about coding questions within the first ten minutes of waking up.

For me, that's actually the best time in the day to do it. Unfortunately that time is no longer at my disposal to waste on such luxuries, there's usually kids wanting to get out of bed. :lol:

 

[Linguofreak]

(Preferably you'd get an exception thrown here, but if you're not accessing memory alocated by another process, you won't).

With current operating systems and processors, that's not really how it works. Each process has its own address space, and as far as the process can see, the only other entity in that address space is the kernel. A an access violation / segfault will be thrown if a process tries to access an address that hasn't yet been allocated to it by the kernel, but this won't catch every bad pointer bug because there is generally a fair amount of memory that has been allocated to the process, but that the user-space runtime hasn't yet allocated internally.

 

[dbeachy1]

Are you sure it's an array overrun? The code is:

const int [COLOR="red"]nabsc = 9[/COLOR];
static const double AOA[nabsc] = {-180*RAD,-60*RAD,-30*RAD, -2*RAD, 15*RAD,20*RAD,25*RAD,60*RAD,180*RAD}; [COLOR="Green"]// nine elements total in this array (valid indexes are 0-8)[/COLOR]
...
for (i = 0; i [COLOR="Red"]< nabsc-1[/COLOR] && AOA[i+1] < aoa; i++);

There are a total of nine elements in the AOA array, so the valid index values are 0-8. The code above is the same as:

for (i = 0; i [COLOR="Red"]< 8[/COLOR] && AOA[i+1] < aoa; i++);

...which is the same as:

for (i = 0; i [COLOR="Red"]<= 7[/COLOR] && AOA[i+1] < aoa; i++);

...so the max 'i+1' value is 7+1, or 8, which is the final (valid) element index of the AOA array -- it's still in bounds.

 

[Hielor]

So sounds like thepenguin copied some code without copying other code.

That said, modern C++ conventions eliminate the need for this kind of thing (and thus this bug) entirely. You don't need to declare the number of elements in a statically declared array, and then after declaring the array you could set nabsc to ARRAYSIZE of AOA.

Also, "nabsc" is an awful variable name.

 

[thepenguin]

Okay, I didn't upload the whole function here. Also, the only reason that it doesn't overrun in the real DG code is that the last value in AoA[] is 180*RAD, which is greater than all possible values of AoA.

dbeachy, judging by the behavior that I got, it definitely behaved like an array overrun. (If it looks like a duck and quacks like a duck, then it's a duck...)

Nevertheless, changing from nabsc-1 to nabsc-2 fixed the crashes, so that's all that really matters. (I added code for a similar mechanism with drag, and the drag coefficient got set to approx. -4E15, thus leading to NaN-spacing)

Let me upload that code...

---------- Post added at 09:39 PM ---------- Previous post was at 09:24 PM ----------

Okay, don't have access to my codebase right now, but I did write and test this code online at http://www.compileonline.com/compile_cpp11_online.php

I think it illustrates the problem. If the code were working right, the radian value of 60 degrees would have been printed to the output. However, the result is zero.

Dbeachy, I can't find anything wrong with the theory of your argument about the algorithm, but the compiler seems to disagree with you.

#include <iostream>

using namespace std;

int main() {
    double RAD = 0.0174532925;
    int i;
    double aoa = 90*RAD;
    const int nabsc = 8;
    static const double AOA[nabsc] = {-180*RAD,-60*RAD,-30*RAD, -2*RAD, 15*RAD,20*RAD,25*RAD,60*RAD};
    for (i = 0; i < nabsc-1 && AOA[i+1] < aoa; i++);
    cout << AOA[i+1];
}

---------- Post added at 09:42 PM ---------- Previous post was at 09:39 PM ----------

Wait, I got it:

for (i = 0; i < nabsc-1 && AOA[i+1] < aoa; i++);

"i" is incremented at the end of every cycle through the loop, regardless of how the condition evaluates. If the code to change "i" were inside the loop, it would have worked right, but now it always increments one step too far.

---------- Post added at 09:44 PM ---------- Previous post was at 09:42 PM ----------

It's equivalent to saying:

int i=0;
while (i<4) {
i++;
}
cout << i

This code would return four, not three. I guess this means that there is a better workaround after all.

 

[Hielor]

So the problem isn't with the code, it's with your understanding of the code.

Yes, obviously "i" will have the value that caused the array to terminate after the array has terminated. It will be the first value for which the condition was false, which in your case will be 8, which is expected.

Apparently the issue is that you were expecting the condition which was going to be explicitly false when the loop finished executing to instead be true?

Also, why do you *not* have 180*RAD in your AOA array? What happens to your vessel if the AOA is greater than 60 degrees?

 

[thepenguin]

Hielor, I think a lot of people in this thread (myself included) were somewhat confused by this statement. I doubt that many people would consider it a normal way of handling a loop.

Are you sure it's an array overrun?

There are a total of nine elements in the AOA array, so the valid index values are 0-8. The code above is the same as:

for (i = 0; i [COLOR="Red"]< 8[/COLOR] && AOA[i+1] < aoa; i++);

...which is the same as:

for (i = 0; i [COLOR="Red"]<= 7[/COLOR] && AOA[i+1] < aoa; i++);

...so the max 'i+1' value is 7+1, or 8, which is the final (valid) element index of the AOA array -- it's still in bounds.

Needless to say that he would be right under all but the most odd circumstances.

Also, if you don't want to change any values beyond 60 degrees, it does not make sense to need to specify to not do anything. A flat line should be, well, flat.

Regardless, if you have the vessel so turned around that your AOA is close to 180, then I would think there is something seriously wrong with the plane besides the aerodynamics, right? (of course there still should be valid values for these ranges)