Interesting bug

D

Dave Vandervies

I just fixed a bug that some of the correctness pedants around here may
find useful as ammunition.


The problem was that some code would, very occasionally, die with a
segmentation violation error. (Not as infrequent as some bugs that
have been discussed here in the past, but maybe once in an overnight
run of the program when it was configured to aggressively exercise the
section that the bug was in.) It was easy enough to trap the error
(using compiler features that are beyond the scope of this newsgroup)
and retry, and the retry would always work, but the glitch was still
Annoying to me and one other correctness pedant I work with.

Turns out that the offending code looked something like this:

for(i=0;i<num;i++)
if(check(array))
break;
if(check(array))
continue;

array was a pointer to a malloc'd array of num structs; check() was
an expression that compared a member of the struct to another value.
(If the values matched, we didn't need to do anything more in the
next-outer loop.)

But if the for loop failed to exit abnormally, the check after it exited
would attempt to read the value just past the end of the malloc'd space.

My best guess about what the problem was is that normally this wasn't a
problem, since the bogus value was Highly Unlikely to match what it was
being checked against and the program was allowed to read that memory
(and didn't try to write to it), but occasionally the mallocd space would
be just at the high end of the process's memory space and attempting to
read a few bytes past it would access memory that the process didn't own,
and the OS would trap it.


This may be useful the next time somebody comes around and tries to
claim that "It works on my system" or something similar.


Comments?


dave
 
J

Joona I Palaste

Dave Vandervies said:
I just fixed a bug that some of the correctness pedants around here may
find useful as ammunition.
(snip)

Turns out that the offending code looked something like this:
for(i=0;i<num;i++)
if(check(array))
break;
if(check(array))
continue;


(snip)

Comments?


As soon as I saw that code (and before I saw your explanation of what it
did) alarm bells went off my head. "If that for loop exits normally,
array will be out of bounds." I'm amazed none of your colleagues
managed to spot it. It's like a flashing red light and a siren saying
(effectively) "Danger, Will Robinson".
 
C

Chris Torek

[much snippage]
Dave Vandervies said:
for(i=0;i<num;i++)
if(check(array))
break;
if(check(array))
continue;


As soon as I saw that code (and before I saw your explanation of what it
did) alarm bells went off my head. "If that for loop exits normally,
array will be out of bounds." I'm amazed none of your colleagues
managed to spot it. It's like a flashing red light and a siren saying
(effectively) "Danger, Will Robinson".


Errors are often a great deal easier to spot after all the irrelevant
distractions have been removed. :)

It is also easy to read what *should* have been written, rather
than what was actually written. This is particular true if the
debugging is being done by the original programmer.
 
D

Dave Vandervies

Dave Vandervies said:
I just fixed a bug that some of the correctness pedants around here may
find useful as ammunition.
(snip)

Turns out that the offending code looked something like this:
for(i=0;i<num;i++)
if(check(array))
break;
if(check(array))
continue;


(snip)

Comments?


As soon as I saw that code (and before I saw your explanation of what it
did) alarm bells went off my head. "If that for loop exits normally,
array will be out of bounds." I'm amazed none of your colleagues
managed to spot it. It's like a flashing red light and a siren saying
(effectively) "Danger, Will Robinson".


So was I, once I found it. (It's incredible how much easier these
problems are to find when you've narrowed them down to a few lines of
code.) The problem was that we had another 2500 lines of code in that
module that were doing all sorts of interesting things with pointers,
so we were looking for problems with losing track of the pointers there,
not looking for things like this one. (But I figured that neither
the newsgroup nor my employer would be terribly impressed if I posted
2500 lines of code in my description of the problem instead of just a
paraphrase of the snippet where the bug actually was.)

Once we had the time (because we were looking for something else in that
part of the code anyways) to add checkpoints every few lines and I saw
which checkpoints the problem was happening between, it took about five
minutes to identify and fix the problem.


dave
 
C

Christopher Benson-Manica

Chris Torek said:
Errors are often a great deal easier to spot after all the irrelevant
distractions have been removed. :)

The most aggravating are stupid typos - I had a recent bug caused by
misspelling "error" as "errror" that had to be pointed out to me
because I could not grasp such a simple mistake ;)
 
C

CBFalconer

Dave said:
.... snip ...

Turns out that the offending code looked something like this:

for(i=0;i<num;i++)
if(check(array))
break;
if(check(array))
continue;

array was a pointer to a malloc'd array of num structs; check()
was an expression that compared a member of the struct to another
value. (If the values matched, we didn't need to do anything more
in the next-outer loop.)

But if the for loop failed to exit abnormally, the check after it
exited would attempt to read the value just past the end of the
malloc'd space.


So why didn't the code read:

while (somecondition) {
for (i = 0; i < num; i++)
if (check(array)) break;
if (i < num) continue;
/* check(array) must be false for all i < num */
}
 
C

Christian Bau

Christopher Benson-Manica said:
The most aggravating are stupid typos - I had a recent bug caused by
misspelling "error" as "errror" that had to be pointed out to me
because I could not grasp such a simple mistake ;)

I always wonder how many switch statements contain a

defualt:

label. And it is legal C as well.
 
C

Christian Bau

CBFalconer said:
Dave said:
... snip ...

Turns out that the offending code looked something like this:

for(i=0;i<num;i++)
if(check(array))
break;
if(check(array))
continue;

array was a pointer to a malloc'd array of num structs; check()
was an expression that compared a member of the struct to another
value. (If the values matched, we didn't need to do anything more
in the next-outer loop.)

But if the for loop failed to exit abnormally, the check after it
exited would attempt to read the value just past the end of the
malloc'd space.


So why didn't the code read:

while (somecondition) {
for (i = 0; i < num; i++)
if (check(array)) break;
if (i < num) continue;
/* check(array) must be false for all i < num */
}


Sometimes the end condition is not that simple; the posted code seems to
be several hundred lines of code, condensed down to the actual error.
But it would be simple to introduce a boolean variable that is set to
TRUE when you break from the loop, and I would probably do that in cases
when the logic of the loop itself gets complicated.
 
P

Peter Nilsson

Christian Bau said:
I always wonder how many switch statements contain a

defualt:

label. And it is legal C as well.

But at least one compiler can pick it up...

% gcc -Wall default.c
default.c: In function `main':
default.c:8: warning: label `defualt' defined but not used
 
E

Eric Sosman

Christian said:
CBFalconer said:
So why didn't the code read:

while (somecondition) {
for (i = 0; i < num; i++)
if (check(array)) break;
if (i < num) continue;
/* check(array) must be false for all i < num */
}


Sometimes the end condition is not that simple; the posted code seems to
be several hundred lines of code, condensed down to the actual error.
But it would be simple to introduce a boolean variable that is set to
TRUE when you break from the loop, and I would probably do that in cases
when the logic of the loop itself gets complicated.


This is an irksome weakness of C (and most other languages
I've programmed in): A loop with multiple termination conditions
gives no direct evidence of which condition actually caused it
to terminate. You find yourself at the statement after the loop
with no notion of how you got there, and you usually wind up
either re-testing a condition already tested (ick) or testing
a state flag that summarizes the earlier test (ick again).

Some languages have an "alternate exit" formalism for such
situations, but in most that I have encountered it looked an
awful lot like an unrestricted goto. Some languages have
"exception" mechanisms, but it's usually been a fairly heavy-
weight construct, too expensive for routine use ("exceptional"
shouldn't be "routine").

However, C lacks such a facility. It follows that nearly
every loop with multiple termination conditions should be
followed promptly by a test; if there's no test, there's most
likely a bug.
 
D

Dan Pop

In said:
This is an irksome weakness of C (and most other languages
I've programmed in): A loop with multiple termination conditions
gives no direct evidence of which condition actually caused it
to terminate. You find yourself at the statement after the loop
with no notion of how you got there, and you usually wind up
either re-testing a condition already tested (ick) or testing
a state flag that summarizes the earlier test (ick again).

Some languages have an "alternate exit" formalism for such
situations, but in most that I have encountered it looked an
awful lot like an unrestricted goto. Some languages have
"exception" mechanisms, but it's usually been a fairly heavy-
weight construct, too expensive for routine use ("exceptional"
shouldn't be "routine").

However, C lacks such a facility.

Nope, it's called goto.
It follows that nearly
every loop with multiple termination conditions should be
followed promptly by a test; if there's no test, there's most
likely a bug.

I have yet to understand this irrational fear of using goto where it is
called for (i.e. any place where it helps simplifying the code structure).

Dan
 
C

CBFalconer

Eric said:
Christian said:
CBFalconer said:
So why didn't the code read:

while (somecondition) {
for (i = 0; i < num; i++)
if (check(array)) break;
if (i < num) continue;
/* check(array) must be false for all i < num */
}


Sometimes the end condition is not that simple; the posted code
seems to be several hundred lines of code, condensed down to the
actual error. But it would be simple to introduce a boolean
variable that is set to TRUE when you break from the loop, and I
would probably do that in cases when the logic of the loop
itself gets complicated.


This is an irksome weakness of C (and most other languages
I've programmed in): A loop with multiple termination conditions
gives no direct evidence of which condition actually caused it
to terminate. You find yourself at the statement after the loop
with no notion of how you got there, and you usually wind up
either re-testing a condition already tested (ick) or testing
a state flag that summarizes the earlier test (ick again).

Some languages have an "alternate exit" formalism for such
situations, but in most that I have encountered it looked an
awful lot like an unrestricted goto. Some languages have
"exception" mechanisms, but it's usually been a fairly heavy-
weight construct, too expensive for routine use ("exceptional"
shouldn't be "routine").

However, C lacks such a facility. It follows that nearly
every loop with multiple termination conditions should be
followed promptly by a test; if there's no test, there's most
likely a bug.


The presence of this multiple termination in itself is a strong
hint that a dummy value is needed to simplify the loop condition,
followed by a simple test. In the case of the above it might have
been, with a suitable expansion of array[]:

while (somecondition) {
array[num] = suitablevalue; i = 0;
while (!check(array)) i++;
if (i < num) continue;
/* check(array) must be false for all i < num */
}
 
E

Eric Sosman

Dan said:
Nope, it's called goto.

Perhaps my use of "such a facility" was unclear. I meant
it to mean "a facility that solves the problem posed in the
first paragraph without the objections mentioned in the second."
I have yet to understand this irrational fear of using goto where it is
called for (i.e. any place where it helps simplifying the code structure).

My fear of goto is not irrational, but some consider it
unnatural: there is disagreement about whether zero is a
natural number. My first programming language was FORTRAN II,
which would probably have driven me to another field of endeavor
had I suffered from gotophobia.
 
D

Dan Pop

In said:
natural number. My first programming language was FORTRAN II,
which would probably have driven me to another field of endeavor
had I suffered from gotophobia.

Back then, gotophobia had not been yet invented ;-)

I started with FORTRAN IV, followed by several assembly languages: having
gotos with symbolic destinations (instead of F-IV's 1 to 5 digits) was
like being in programmers' heaven... (but I had to do all the assemblying
by hand, having no access to any kind of hosted platform...).

Dan
 
J

Joona I Palaste

Back then, gotophobia had not been yet invented ;-)
I started with FORTRAN IV, followed by several assembly languages: having
gotos with symbolic destinations (instead of F-IV's 1 to 5 digits) was
like being in programmers' heaven... (but I had to do all the assemblying
by hand, having no access to any kind of hosted platform...).

Right on the money, Dan. I started programming on C=64 BASIC V2, which
is an extremely cut-down version of BASIC by 1980s standards. Back then
GOTO with a symbolic label rather than a hard-coded line number as the
destination was like science fiction...

--
/-- Joona Palaste ([email protected]) ------------- Finland --------\
\-- http://www.helsinki.fi/~palaste --------------------- rules! --------/
"'So called' means: 'There is a long explanation for this, but I have no
time to explain it here.'"
- JIPsoft
 
K

Kenneth Brody

Chris Torek wrote:
[...]
Errors are often a great deal easier to spot after all the irrelevant
distractions have been removed. :)

It is also easy to read what *should* have been written, rather
than what was actually written. This is particular true if the
debugging is being done by the original programmer.

Your brain knows what the code is supposed to be doing, and so you
"see" code that should be behaving. It's not uncommon for someone
to be debugging their own code for hours, only to call someone else
over who spots the error in under 30 seconds. (I've been on both
sides of that scenario.)
 
R

Richard Delorme

Joona I Palaste a écrit :
Right on the money, Dan. I started programming on C=64 BASIC V2, which
is an extremely cut-down version of BASIC by 1980s standards. Back then
GOTO with a symbolic label rather than a hard-coded line number as the
destination was like science fiction...

Gotophobia started in the sixties. Dijkstra's paper¹ was published in
1968 and refered to older papers published in 1966. Fortran II and IV
appeared respectively in 1958 and 1962 so their ignorance is forgivable,
whereas C-64 Basic v2 (and many other Basics of this time) appeared when
Dijkstra's paper had irremediably changed minds.

¹ Go To Statement Considered Harmful. Communications of the ACM, Vol.
11, No. 3, March 1968, pp. 147-148. Available here :
http://www.acm.org/classics/oct95/
 
J

Joona I Palaste

Richard Delorme said:
Joona I Palaste a écrit :
Gotophobia started in the sixties. Dijkstra's paper¹ was published in
1968 and refered to older papers published in 1966. Fortran II and IV
appeared respectively in 1958 and 1962 so their ignorance is forgivable,
whereas C-64 Basic v2 (and many other Basics of this time) appeared when
Dijkstra's paper had irremediably changed minds.
¹ Go To Statement Considered Harmful. Communications of the ACM, Vol.
11, No. 3, March 1968, pp. 147-148. Available here :
http://www.acm.org/classics/oct95/

Yes, but what were you going to do about it? C-64 Basic v2 had pretty
much the bare minimum of control structures. IF... THEN was only for
single lines, not blocks of lines, and there was no ELSE. The only loop
available was FOR... NEXT. Every other kind of loop had to be simulated
with IF... THEN GOTO. And like I said, GOTOs had to have hard-coded line
numbers as destinations. There was GOSUB... RETURN which I think could
be used to have "poor man's subroutines", because they behaved
otherwise like subroutines but had no concept of local variable scope.
So it's not like you could write the kind of structured programming
Dijkstra would have wanted in C-64 Basic v2.
I have never programmed in Fortran, but I think it was about the same
situation with the Fortran version Dan Pop grew up with.
 
D

Dan Pop

In said:
Gotophobia started in the sixties. Dijkstra's paper¹ was published in
1968 and refered to older papers published in 1966. Fortran II and IV
appeared respectively in 1958 and 1962 so their ignorance is forgivable,
whereas C-64 Basic v2 (and many other Basics of this time) appeared when
Dijkstra's paper had irremediably changed minds.

The main purpose of many BASIC implementations was to make the best use
of the *limited* resources of 8-bit micros. One of the most popular
micros of the early eighties had 8k of ROM and 1k of RAM in its standard
configuration. Creative minds managed amazing feats on this
configuration.

By the time BASIC moved to relatively resource-rich PC's, it also acquired
structured programming features (and even dropped the line numbers that
were supposed to replace the need for a "sophisticated" text editor).

Then again, no matter how many minds Dijkstra's paper might have changed,
there is plenty of proof that well structured code can be written with
gotos and badly structured code without. The tool might help, but it
cannot replace the skill of the craftsman.

Dan
 
R

Richard Delorme

Joona I Palaste a écrit :
Richard Delorme said:
Back then, gotophobia had not been yet invented ;-) [...]
I started programming on C=64 BASIC V2, which
is an extremely cut-down version of BASIC by 1980s standards.
[...]
> what were you going to do about it?

When reading the two previous sentences, it sounds to me that
"gotophobia" has been invented past the 1980s. This is why I tried to
restablish the chronology.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads


Members online

No members online now.

Forum statistics

Threads
473,774
Messages
2,569,598
Members
45,151
Latest member
JaclynMarl
Top