VERY strange C anomaly

[Bernard Liang]

Consider the following excerpt below, with the included relevant
declarations.

Firstly, the lines marked with **, ***, **** at the beginning are not
supposed to have those stars at the beginning; they are only used to
direct attention.

Suppose, first of all, that the lines with the **** are not present.
p_num is somehow getting changed from its previous value to 0 by the
sscanf command (***) that doesn't even involve p_num. I cannot
understand it, how I could possibly be doing anything that would modify
its value. The lines marked ** were added to determine that that was
indeed the line causing the problem. (the value was 1 before, and 0 after).

I then added the lines marked **** to remedy the problem, but being of
the philosophy that such "cheap fixes" are very poor programming style,
I wanted to see if anyone could shed some insight on this.

Code follows.


int main(int argc, char** argv) {
int pktsize, interval, numgums, p_pktsize, p_interval, p_num;
char trial, p_trial;
int wtf;
char buffer[200];
FILE* fp;
fp = fopen(argv[1], "r");
....

while (fgets(buffer, 160, fp) != NULL) {
**printf("how did p_numgums suddenly change to 0=%d?!?!\n", p_num);
****wtf = p_num;
*** if (sscanf(buffer, "%d %d %d %1s %*d %lf", &pktsize,
&interval, &numgums, &trial, &commrate) == 5) {
****p_num = wtf;
**printf("ok, now what? %d\n", p_num);
printf("from the string |%s", buffer);
printf(" cur data set: pktsize=%d interval=%d num=%d trial=%c cmr=%f\n",
pktsize, interval, numgums, trial, commrate);
printf("prev data set: pktsize=%d interval=%d num=%d trial=%c cmr=%f\n",
p_pktsize, p_interval, p_num, p_trial, p_commrate);

if (count == -1) {
count = 1; ++j;
min = max = runtot = commrate;
} else if ((p_pktsize == pktsize) && (p_interval ==
interval) && (p_num == numgums)) {
++count; ++j;
runtot += commrate;
if (fcompare(commrate, min) < 0) min = commrate;
if (fcompare(max, commrate) < 0) max = commrate;
} else {
printf("%d\t%d\t%d\t%c\t%d\t%f\t%f\t%f\n",
p_pktsize, p_interval, p_num, p_trial, count,
runtot / count, max, min);
count = 1; j = 1;
min = max = runtot = commrate;
}
p_pktsize = pktsize;
p_interval = interval;
printf("numgums was changed from %d", p_num);
p_num = numgums;
printf("to %d\n", p_num);
p_trial = trial;
p_commrate = commrate;
}
}
return 0;
}



The head (-n10) of the file I am feeding it via argv[1] looks likes this:
----
1000 10 1 a 10 91.041515 1500
1000 10 1 b 11 91.036757 1491
1000 10 1 c 12 87.535014 1500
1000 10 3 a 10 51.517332 859
1000 10 3 a 11 54.095827 910
1000 10 3 a 12 52.226982 890
1000 10 3 b 20 37.465007 629
1000 10 3 b 21 47.486709 786
1000 10 3 b 22 49.602106 829
1000 10 3 c 30 36.402570 629

 

[Dann Corbit]

If you post the actual source you are compiling (and not some fragments) you
can get a more reasonable answer.

For instance, commrate is not defined, but the type informationis crucial to
see if you are overwriting something via a bad sscanf() format specifier.

 

[Ben Pfaff]

int pktsize, interval, numgums, p_pktsize, p_interval, p_num;
char trial, p_trial;
[...]

*** if (sscanf(buffer, "%d %d %d %1s %*d %lf", &pktsize,
&interval, &numgums, &trial, &commrate) == 5) {

%1s will store one non-white space character plus a terminating
null, but you only allocated room for one byte. Use %c instead
of %1s or declare trial as an array of 2 characters instead.

You didn't show the declaration of commrate. It should have type
double.

 

[Bernard Liang]

asdf, I thought I included everything. I guess not. Well here it is.
there might be some irrelevant decl's b/c I was modifying another script
I had written. but I don't think there's anything substantial that is in
here other than the commrate defn's.

#include <stdio.h>
#include <math.h>
#include <float.h>
#include "gpslib.h"

int main(int argc, char** argv) {
int pktsize, interval, numgums, p_pktsize, p_interval, p_num;
char trial, p_trial;
int wtf;
double commrate, p_commrate;

int count = 1, bufpos = 0, num, i, j; // count = # of gums
recorded, bufpos = position in fgets buffer
double min = DBL_MAX, max = DBL_MIN, runtot = 0.0;
char buffer[200];
char* bufptr;

FILE* fp;

if ((argc != 3) || ((num = atoi(argv[2])) > 10)) {
printf("%s [stat_file] [num (<=10)]\n", argv[0]);
return 1;
}
fp = fopen(argv[1], "r");

count = -1;
j=0;
p_num = -1;

while (fgets(buffer, 160, fp) != NULL) {
printf("how the f*** did p_numgums suddenly change to 0=%d?!?!\n", p_num);
wtf = p_num;
sscanf(buffer, "%d %d %d %1s %*d %lf", &pktsize, &interval, &numgums,
&trial, &commrate);
p_num = wtf;
printf("ok, now what? %d\n", p_num);
printf("from the string |%s", buffer);
printf(" cur dataset: pktsize=%d interval=%d num=%d trial=%c cmr=%f\n",
pktsize, interval, numgums, trial, commrate);
printf("prev dataset: pktsize=%d interval=%d num=%d trial=%c cmr=%f\n",
p_pktsize, p_interval, p_num, p_trial, p_commrate);

if (count == -1) {
count = 1; ++j;
min = max = runtot = commrate;
} else if ((p_pktsize == pktsize) && (p_interval == interval) &&
(p_num == numgums) && (p_trial == trial)) {
++count; ++j;
runtot += commrate;
if (fcompare(commrate, min) < 0) min = commrate;
if (fcompare(max, commrate) < 0) max = commrate;
} else {
printf("%d\t%d\t%d\t%c\t%d\t%f\t%f\t%f\n",
p_pktsize, p_interval, p_num, p_trial, count, runtot / count, max,
min);
count = 1; j = 1;
min = max = runtot = commrate;
}
p_pktsize = pktsize;
p_interval = interval;
printf("numgums was changed from %d", p_num);
p_num = numgums;
printf("to %d\n", p_num);
p_trial = trial;
p_commrate = commrate;

}
printf("%d\t%d\t%d\t%c\t%d\t%f\t%f\t%f\n", p_pktsize, p_interval, p_num,
p_trial, count, runtot / count, max, min);

return 0;
}

Consider the following excerpt below, with the included relevant
declarations.

Firstly, the lines marked with **, ***, **** at the beginning are not
supposed to have those stars at the beginning; they are only used to
direct attention.

Suppose, first of all, that the lines with the **** are not present.
p_num is somehow getting changed from its previous value to 0 by the
sscanf command (***) that doesn't even involve p_num. I cannot
understand it, how I could possibly be doing anything that would modify
its value. The lines marked ** were added to determine that that was
indeed the line causing the problem. (the value was 1 before, and 0 after).

I then added the lines marked **** to remedy the problem, but being of
the philosophy that such "cheap fixes" are very poor programming style,
I wanted to see if anyone could shed some insight on this.

Code follows.


int main(int argc, char** argv) {
int pktsize, interval, numgums, p_pktsize, p_interval, p_num;
char trial, p_trial;
int wtf;
char buffer[200];
FILE* fp;
fp = fopen(argv[1], "r");
...

while (fgets(buffer, 160, fp) != NULL) {
**printf("how did p_numgums suddenly change to 0=%d?!?!\n", p_num);
****wtf = p_num;
*** if (sscanf(buffer, "%d %d %d %1s %*d %lf", &pktsize,
&interval, &numgums, &trial, &commrate) == 5) {
****p_num = wtf;
**printf("ok, now what? %d\n", p_num);
printf("from the string |%s", buffer);
printf(" cur data set: pktsize=%d interval=%d num=%d trial=%c cmr=%f\n",
pktsize, interval, numgums, trial, commrate);
printf("prev data set: pktsize=%d interval=%d num=%d trial=%c cmr=%f\n",
p_pktsize, p_interval, p_num, p_trial, p_commrate);

if (count == -1) {
count = 1; ++j;
min = max = runtot = commrate;
} else if ((p_pktsize == pktsize) && (p_interval ==
interval) && (p_num == numgums)) {
++count; ++j;
runtot += commrate;
if (fcompare(commrate, min) < 0) min = commrate;
if (fcompare(max, commrate) < 0) max = commrate;
} else {
printf("%d\t%d\t%d\t%c\t%d\t%f\t%f\t%f\n",
p_pktsize, p_interval, p_num, p_trial, count, runtot
/ count, max, min);
count = 1; j = 1;
min = max = runtot = commrate;
}
p_pktsize = pktsize;
p_interval = interval;
printf("numgums was changed from %d", p_num);
p_num = numgums;
printf("to %d\n", p_num);
p_trial = trial;
p_commrate = commrate;
}
}
return 0;
}



The head (-n10) of the file I am feeding it via argv[1] looks likes this:
----
1000 10 1 a 10 91.041515 1500
1000 10 1 b 11 91.036757 1491
1000 10 1 c 12 87.535014 1500
1000 10 3 a 10 51.517332 859
1000 10 3 a 11 54.095827 910
1000 10 3 a 12 52.226982 890
1000 10 3 b 20 37.465007 629
1000 10 3 b 21 47.486709 786
1000 10 3 b 22 49.602106 829
1000 10 3 c 30 36.402570 629

 

[Eric Sosman]

Bernard Liang wrote On 07/20/06 17:32,:

Consider the following excerpt below, with the included relevant
declarations.

Firstly, the lines marked with **, ***, **** at the beginning are not
supposed to have those stars at the beginning; they are only used to
direct attention.

I've removed them; hope you don't mind.

p_num is somehow getting changed from its previous value to 0 by the
sscanf command (***) that doesn't even involve p_num. [...]

int pktsize, interval, numgums, p_pktsize, p_interval, p_num;
char trial, p_trial;
[...]
if (sscanf(buffer, "%d %d %d %1s %*d %lf", &pktsize,
&interval, &numgums, &trial, &commrate) == 5) {

The "%1s" conversion specifier skips white space,
reads one character, and stores it as a string starting
at the indicated place, in this case `&trial'. Observe
that I said "as a string," meaning that two characters
are stored: the input character (as the string's content)
plus a zero byte (to mark the end of the string). The
input character goes into `trial' as intended, but you
have not provided any memory to hold the zero byte. The
sscanf() function will try to store the zero anyhow, and
whatever happens next is in the laps of the gods.

What happened in this case is *probably* that sscanf()
stored the zero byte in the memory location immediately
after `trial', and *probably* that memory was part of the
memory used for `p_num', hence the mysterious value change.

If you want to read just one character and treat it as
a single character (not as a string), you should probably
use "%c" instead of "%s" (but note that "%c" does not skip
leading white space, if that's important). If you want
to read a one-character string into a two-character array
(one for the payload, one for the zero), keep "%1s" but
change `char trial' to `char trial[2]' and change `&trial'
to `trial'.

 

[Chris Torek]

Consider the following excerpt below, with the included relevant
declarations.

There is too much stuff missing from the excerpt to be sure that
this is the *only* problem, but I spotted one big problem right
away:

int pktsize, interval, numgums, p_pktsize, p_interval, p_num;
char trial, p_trial;
if (sscanf(buffer, "%d %d %d %1s %*d %lf", &pktsize,
&interval, &numgums, &trial, &commrate) == 5) ...

The three "%d" conversion directives write three "int"s into
pktsize, interval, and numgums respectively (this part is
OK); then the "%1s" directive writes two "char"s into the
(one) "char" named trial, and then the %lf conversion writes
a "double" into commrate.

When you write two "char"s into a one-"char"-wide array via
(&trial)[0] and (&trial)[1], the effect is undefined. In most
cases, some apparently unrelated variable gets clobbered.

Although %s and %c are very different conversions, the whitespace
directives you have included before and after the "%1s" mean that
this particular problem can be cured by changing "%1s" to "%c".
(Or you could declare "char trial[2]" and then use trial[0]
to look at the one non-'\0' character stored via %1s.)

 

[Dann Corbit]

Using this file:

#include <stdio.h>
#include <math.h>
#include <float.h>
/* #include "gpslib.h" */

int main(int argc, char **argv)
{
int pktsize,
interval,
numgums,
p_pktsize,
p_interval,
p_num;
char trial,
p_trial;
int wtf;
double commrate,
p_commrate;

int count = 1,
bufpos = 0,
num,
i,
j;
double min = DBL_MAX,
max = DBL_MIN,
runtot = 0.0;
char buffer[200];
char *bufptr;

FILE *fp;

if ((argc != 3) || ((num = atoi(argv[2])) > 10)) {
printf("%s [stat_file] [num (<=10)]\n", argv[0]);
return 1;
}
fp = fopen(argv[1], "r");

count = -1;
j = 0;
p_num = -1;

while (fgets(buffer, 160, fp) != NULL) {
printf("how the f*** did p_numgums suddenly change to 0=%d?!?!\n",
p_num);
wtf = p_num;
sscanf(buffer, "%d %d %d %1s %*d %lf", &pktsize, &interval,
&numgums,
&trial, &commrate);
p_num = wtf;
printf("ok, now what? %d\n", p_num);
printf("from the string |%s", buffer);
printf(" cur dataset: pktsize=%d interval=%d num=%d trial=%c
cmr=%f\n",
pktsize, interval, numgums, trial, commrate);
printf("prev dataset: pktsize=%d interval=%d num=%d trial=%c
cmr=%f\n",
p_pktsize, p_interval, p_num, p_trial, p_commrate);

if (count == -1) {
count = 1;
++j;
min = max = runtot = commrate;
} else if ((p_pktsize == pktsize) && (p_interval == interval) &&
(p_num == numgums) && (p_trial == trial)) {
++count;
++j;
runtot += commrate;
if (fcompare(commrate, min) < 0)
min = commrate;
if (fcompare(max, commrate) < 0)
max = commrate;
} else {
printf("%d\t%d\t%d\t%c\t%d\t%f\t%f\t%f\n",
p_pktsize, p_interval, p_num, p_trial, count, runtot /
count, max,
min);
count = 1;
j = 1;
min = max = runtot = commrate;
}
p_pktsize = pktsize;
p_interval = interval;
printf("numgums was changed from %d", p_num);
p_num = numgums;
printf("to %d\n", p_num);
p_trial = trial;
p_commrate = commrate;

}
printf("%d\t%d\t%d\t%c\t%d\t%f\t%f\t%f\n", p_pktsize, p_interval, p_num,
p_trial, count, runtot / count, max, min);

return 0;
}

Lint said:
--- Module: tt.c
_
if ((argc != 3) || ((num = atoi(argv[2])) > 10)) {
tt.c(33) : Error 1055: atoi undeclared, assumed to return int
tt.c(33) : Info 746: call to atoi() not made in the presence of a prototype
_
while (fgets(buffer, 160, fp) != NULL) {
tt.c(43) : Warning 668: Possibly passing a null pointer to function
fgets(char
*, int, struct _iobuf *), arg. no. 3
tt.c(43) : Warning 668: Possibly passing a null pointer to function
fgets(char
*, int, struct _iobuf *), arg. no. 3
_
p_pktsize, p_interval, p_num, p_trial, p_commrate);
tt.c(54) : Warning 530: p_pktsize (line 11) not initialized
tt.c(54) : Warning 530: p_interval (line 12) not initialized
tt.c(54) : Warning 530: p_trial (line 15) not initialized
tt.c(54) : Warning 530: p_commrate (line 18) not initialized
_
if (fcompare(commrate, min) < 0)
tt.c(65) : Error 1055: fcompare undeclared, assumed to return int
tt.c(65) : Info 746: call to fcompare() not made in the presence of a
prototype
_
printf("%d\t%d\t%d\t%c\t%d\t%f\t%f\t%f\n", p_pktsize, p_interval, p_num,
tt.c(86) : Warning 668: Possibly passing a null pointer to function
fgets(char
*, int, struct _iobuf *), arg. no. 3
_
}
tt.c(90) : Warning 550: num (line 22) not accessed
tt.c(90) : Warning 529: i (line 23) not subsequently referenced
tt.c(90) : Warning 550: j (line 24) not accessed
tt.c(90) : Warning 529: bufpos (line 21) not subsequently referenced
tt.c(90) : Warning 529: bufptr (line 29) not subsequently referenced

--- Wrap-up for Module: tt.c

Info 766: Header file 'C:\lang\VC98\include\math.h' not used in module
'tt.c'
Error 305: Unable to open module: tt.c0

And Splint said:
Splint 3.0.1.6 --- 11 Feb 2002

tt.c: (in function main)
tt.c(43,31): Possibly null storage fp passed as non-null param: fgets (...,
fp)
A possibly null pointer is passed as a parameter corresponding to a formal
parameter with no /*@null@*/ annotation. If NULL may be used for this
parameter, add a /*@null@*/ annotation to the function parameter
declaration.
(Use -nullpass to inhibit warning)
tt.c(37,10): Storage fp may become null
tt.c(46,9): Return value (type int) ignored: sscanf(buffer, "...
Result returned by function call is not used. If this is intended, can
cast
result to (void) to eliminate message. (Use -retvalint to inhibit warning)
tt.c(54,16): Variable p_pktsize used before definition
An rvalue is used that may not be initialized to a value on some execution
path. (Use -usedef to inhibit warning)
tt.c(54,27): Variable p_interval used before definition
tt.c(54,46): Variable p_trial used before definition
tt.c(54,55): Variable p_commrate used before definition
tt.c(65,17): Unrecognized identifier: fcompare
Identifier used in code has not been declared. (Use -unrecog to inhibit
warning)
tt.c(86,48): Variable p_pktsize used before definition
tt.c(86,59): Variable p_interval used before definition
tt.c(87,12): Variable p_trial used before definition
tt.c(21,21): Variable bufpos declared but not used
A variable is declared but never used. Use /*@unused@*/ in front of
declaration to suppress message. (Use -varuse to inhibit warning)
tt.c(23,21): Variable i declared but not used
tt.c(29,21): Variable bufptr declared but not used

Finished checking --- 13 code warnings

 

[Ben Pfaff]

Bernard Liang wrote On 07/20/06 17:32,:

p_num is somehow getting changed from its previous value to 0 by the
sscanf command (***) that doesn't even involve p_num. [...]

int pktsize, interval, numgums, p_pktsize, p_interval, p_num;
char trial, p_trial;
[...]
if (sscanf(buffer, "%d %d %d %1s %*d %lf", &pktsize,
&interval, &numgums, &trial, &commrate) == 5) {

What happened in this case is *probably* that sscanf()
stored the zero byte in the memory location immediately
after `trial', and *probably* that memory was part of the
memory used for `p_num', hence the mysterious value change.

I expect that Ancient_Hacker will be right along to tell us that
no compiler would order the variables that way.

 

[Barry Schwarz]

asdf, I thought I included everything. I guess not. Well here it is.
there might be some irrelevant decl's b/c I was modifying another script
I had written. but I don't think there's anything substantial that is in
here other than the commrate defn's.

#include <stdio.h>
#include <math.h>
#include <float.h>
#include "gpslib.h"

int main(int argc, char** argv) {
int pktsize, interval, numgums, p_pktsize, p_interval, p_num;
char trial, p_trial;
int wtf;
double commrate, p_commrate;

int count = 1, bufpos = 0, num, i, j; // count = # of gums
recorded, bufpos = position in fgets buffer

// comments don't do well in usenet.

double min = DBL_MAX, max = DBL_MIN, runtot = 0.0;
char buffer[200];
char* bufptr;

FILE* fp;

if ((argc != 3) || ((num = atoi(argv[2])) > 10)) {

atoi requires stdlib.h.

printf("%s [stat_file] [num (<=10)]\n", argv[0]);
return 1;
}
fp = fopen(argv[1], "r");

count = -1;
j=0;
p_num = -1;

while (fgets(buffer, 160, fp) != NULL) {
printf("how the f*** did p_numgums suddenly change to 0=%d?!?!\n", p_num);

On my system, it printed -1 as expected.

wtf = p_num;
sscanf(buffer, "%d %d %d %1s %*d %lf", &pktsize, &interval, &numgums,
&trial, &commrate);

You overflow trial. Undefined behavior, called buffer overflow. You
have no idea what the overflow stepped on.

p_num = wtf;
printf("ok, now what? %d\n", p_num);
printf("from the string |%s", buffer);
printf(" cur dataset: pktsize=%d interval=%d num=%d trial=%c cmr=%f\n",
pktsize, interval, numgums, trial, commrate);

This time you overrun trial. It is not a string since it is not
terminated with a '\0'.

printf("prev dataset: pktsize=%d interval=%d num=%d trial=%c cmr=%f\n",
p_pktsize, p_interval, p_num, p_trial, p_commrate);

Undefined behavior by evaluating uninitialized variables.

if (count == -1) {
count = 1; ++j;
min = max = runtot = commrate;
} else if ((p_pktsize == pktsize) && (p_interval == interval) &&
(p_num == numgums) && (p_trial == trial)) {
++count; ++j;
runtot += commrate;
if (fcompare(commrate, min) < 0) min = commrate;
if (fcompare(max, commrate) < 0) max = commrate;
} else {
printf("%d\t%d\t%d\t%c\t%d\t%f\t%f\t%f\n",
p_pktsize, p_interval, p_num, p_trial, count, runtot / count, max,
min);
count = 1; j = 1;
min = max = runtot = commrate;
}
p_pktsize = pktsize;

This code is executed too late for the printf flagged above.

p_interval = interval;
printf("numgums was changed from %d", p_num);
p_num = numgums;
printf("to %d\n", p_num);
p_trial = trial;
p_commrate = commrate;

}
printf("%d\t%d\t%d\t%c\t%d\t%f\t%f\t%f\n", p_pktsize, p_interval, p_num,
p_trial, count, runtot / count, max, min);

return 0;
}

The head (-n10) of the file I am feeding it via argv[1] looks likes this:
----
1000 10 1 a 10 91.041515 1500
1000 10 1 b 11 91.036757 1491
1000 10 1 c 12 87.535014 1500
1000 10 3 a 10 51.517332 859
1000 10 3 a 11 54.095827 910
1000 10 3 a 12 52.226982 890
1000 10 3 b 20 37.465007 629
1000 10 3 b 21 47.486709 786
1000 10 3 b 22 49.602106 829
1000 10 3 c 30 36.402570 629

Remove del for email

 

[Mark McIntyre]

while (fgets(buffer, 160, fp) != NULL) {
printf("how the f*** did p_numgums suddenly change to 0=%d?!?!\n", p_num);

It didn't change to anything. You never initialised it, the value is
garbage, and even reading it causes a serious fault.

Make sure you initialise stuff before you use it.

wtf = p_num;

Ditto here. You're initialising wtf with garbage.

sscanf(buffer, "%d %d %d %1s %*d %lf", &pktsize, &interval, &numgums,
&trial, &commrate);

See Ben's comment about reading in 1 character. %1s reads in one, but
needs space for two (a terminating null).


Turn up warning levels on your compiler, fix the warnings, then retest
--
Mark McIntyre

"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it."
--Brian Kernighan

 

[Dik T. Winter]

> On Thu, 20 Jul 2006 14:48:18 -0700, in comp.lang.c , Bernard Liang

>
> It didn't change to anything. You never initialised it, the value is
> garbage, and even reading it causes a serious fault.

Did you not see the line:?