use of printf prevents segfault!

I'm writing a function that takes a file, reads the file into a buffer while keeping count of the lines, then takes each line and plugs the content into a simple structure. I did it that way because the number of lines is variable and will determine, obviously, the number of structs needed. So I get the number of lines (nofl) and in the middle of the function do this:

struct thestruct new[nofl];

I was getting a segfault, so to make sure nofl was correct, i inserted the following line RIGHT BEFORE the last one:

printf("--%d--\n", nofl);

AND NOW NO SEGFAULT!!! Nothing else is changed, but adding and subtracting that one line makes all the difference. Really, really, really. More observations:
1) Up until now I've been declaring variables (inc. structs) at the beginning of a function, but I've never heard tell that it must be so. Does this have anything to do with it?
2) The segfault actually occurs later when new[1] (ie. the 2nd struct in the array) is called on (it turns out not to exist).
3) "nofl" is determined on the line immediately preceeding the printf statement.
4) if I put the printf statement after the struct declaration, the segfault returns.

You could post the code to make things more clear (if that's possible).
1) You can declare variables in the middle like you do with new[] only with the C99 standard (i think)
2) So a seg fault occurs after all, so the printf doesn't actually solve the problem, eh?
3) As long as it is before it is ok
4) I ll admit I had similar occasions that printf changed the seg fault. But in any case it doesn't solve the problem. Post the code to find exactly the problem.

Sounds like a classic stack corruption. Can you post your code?

QuantumPete

Quote:

---

Originally Posted by QuantumPete

Sounds like a classic stack corruption. Can you post your code?

QuantumPete

---

Or quite possibly writing over the end of your allocated data at some point previously.

When problems change due to "irrelevant" changes in the code, it's usually because you are going across the line of some memory - adding/changing the code around will move the actual bits of memory used by different pieces of code, so the problem "moves", but it's actually not fixed, it's just overwriting something less harmful.

For example, calling printf will make sure the variable is stored on the stack, whilst not calling printf will not use the stack to store the local variable - so some bytes less used on the stack, so when you overwrite the end of an array, it no longer overwrites the variable, but some important data [such as the frame pointer or return address of the function].

--
Mats

Quote:

---

Originally Posted by matsp

Or quite possibly writing over the end of your allocated data at some point previously.

---

My money is still on stack corruption ;)

QuantumPete

Quote:

---

Originally Posted by QuantumPete

My money is still on stack corruption ;)

QuantumPete

---

Not at all saying you are wrong. Only reason I said overwriting dynamically allocated memory is that it's happening in new - but that's no good indication really.

--
Mats

Quote:

---

Originally Posted by matsp

it's happening in new

---

It looks to me like the variable is called new (how that got by the compiler I don't know), no dynamic allocation.

QuantumPete

Quote:

---

Originally Posted by QuantumPete

It looks to me like the variable is called new (how that got by the compiler I don't know), no dynamic allocation.

QuantumPete

---

Doh! Yes, since it's the C (rather than C++) forum, then it makes sense - and you're quite right. Must clean my glasses or put on my reading head or something ;-)

--
Mats

Sorry I didn't post the original code, it seemed tedious and I wanted an answer to the printf question, since that was clearly the "crux". b/t/w "new" is not the actual name of the variable...

"Sounds like a classic stack corruption." -- QuantumPete
If stack corruption is what causes compiler output including this:
======= Backtrace: =========
[...]
======= Memory map: ========
Then no, it's not stack corruption.

"For example, calling printf will make sure the variable is stored on the stack, whilst not calling printf will not use the stack to store the local variable" -- matsp
Admittedly I know very little about this, but it was my understanding that an uninitialized int goes into a stack frame and stays there after being initialized. However, yours is the only explanation I see.

"so some bytes less used on the stack, so when you overwrite the end of an array," -- matsp
This pair of phrases does not make sense to me. Could you clarify?

Anyway, in the end what solved the problem is just splitting the function so that the array of structs is defined at a beginning. I am pretty sure GNU glibc is C89, not C99? How this relates to the overwrite (assuming matsp is correct) I don't know.

However, that now means the struct array is outside main! But I guess there's a different problem...thanks for all your thoughts.

Unfortunately, stack corruption can cause all sorts of things - backtrace assumes that you end up in some sort of error handler in the C-library - this doesn't always happen for stack corruption, because the code that prints the "backtrace" is using the stack to produce that backtrace, and if the stack is sufficiently messsed up, then it will not be able to figure out what to produce and give up [or crash]. (For example, if you write NULL to the frame-pointer [1] then it will most likely never find your backtrace)

And to answer your direct questions:
A local variable doesn't have to take up space on the stack. It is entirely possible, under the right circumstances, for the compiler to only ever hold the value in a register - in which case the variable has no stack-space assigned to it.

Whether glibc is written in C89, C99 or some other C standard is pretty irrelevant - your code is what causes this problem. Splitting it into two functions should not fix any problem like this, but it may of course HIDE the problem.

Without seeing the actual code, it's pretty hard to know what is right and what is wrong with it. It's like talking to a mechanic on the phone about a car that has some sort of problem. The mechanic can make a guess as to what the problem could be, but it's certainly a lot easier to diagnose the problem when the mechanic and the car are next to each other.

[1]Frame-pointer is a pointer to the previous stack-frame that is maintained by the compiler so that a debugger or debug help function like the backtrace can figure out how you got there.

--
Mats

The code is just too long and I'm not going to bother trying to recreate the problem on a smaller scale since it now works, altho I completely agree with that point. FYI my problem started with me not knowing I could do this:

Code:

---

struct thestruct {
[...]
} ray[];
int main () {

---

to create a global array of structs that will grow "dynamically". But I still want to ask you...

"Whether glibc is written in C89, C99 or some other C standard is pretty irrelevant"
So what did C_ntua mean by "You can declare variables in the middle like you do with new[] only with the C99 standard (i think)"? Where would the problem be hiding once the segfault is gone and everything works smoothly, if my code must still be flawed? Can I ever rid myself of such a hidden flaw? Perhaps the overwrite was symptomatic of violating the standard.

About the mysteries of memory allocation: by "stored in a register" you mean the variable is retained in the CPU and never allocated memory? If "printf" causes it to be put on the stack, shouldn't "struct thestruct Knew[nofl]" also? In which case doing this in the middle of a function (against the standard) might cause an allocation problem whereas a previous printf would "make it legal". Am I thinking about this correctly?

Whether you can compile the C library with a Cxx or Cyy compliant compiler doesn't change whether the compiler you are using at the moment can compile your code or not. gcc is (nearly completely) C99 compliant in itself.

Further, for most intents and purposes, the code generated by:

Code:

---

void func()
{ 
    int x;
    ...  // Big chunk of code
    /// Calculations setting x
    ...
    printf("x=%d\n", x);
}

---

and

Code:

---

void func()
{ 
    ...  // Big chunk of code
    int x;
    /// Calculations setting x
    ...
    printf("x=%d\n", x);
}

---

is marginally different [there would be a bit more difference if x was a C++ object, but the space on the stack would probably be the same either way - it is allocated at the beginning of the function to make it easy for the rest of the code to know what's where and not unnecessarily shift the stack pointer up and down too much].

As to what goes on the stack and what goes in a CPU register is entirely up to the compiler. Of course, large variables such as a struct array, most likely goes into memory (there just ain't enough registers to waste them on stuff like storing structs in them).

Changing the code can change the choices the compiler makes for many reasons [compilers are often using heuristics - approximations/imperfect solutions to solve problems, and will have choices such as "if we have these three conditions true, we do it this way, else do it some other way" - those conditions may be which registers are actually available, the size of the code, and many other non-apparent reasons].

The other side is of course that the compiler can't take the address of a register, so if some variable is used as a target of & (address operator), then it's not valid to store it in a register.

Edit: If the compiler doesn't support C99, it will complain about C99 constructs such as local variables in the middle of code or dynamic sizing of variables.

And finally a question: What sort of number is nofl?

--
Mats

"Changing the code can change the choices the compiler makes for many reasons [compilers are often using heuristics - approximations/imperfect solutions to solve problems, and will have choices such as "if we have these three conditions true, we do it this way, else do it some other way" - those conditions may be which registers are actually available, the size of the code, and many other non-apparent reasons]."

This makes sense, since even with the printf (=no segfault) line the individual members of new[nofl] were, in fact, corrupted, whereas using what I presume to be a more standard practice (the global ray[]) they are not. However, this produces the hopefully harmless warning: array ‘ray’ assumed to have one element

And a final answer: nofl is/was a short int.

ps. please see my last comment on pg.2 if you like "ray[]", disagree with the next two comments, and noticed that QuatumPete didn't get it...

So ray[] is a singe element array, and using that for nofl > 0 is invalid. You need to fill in number of elements in the brackets to make sure you have enough space. If you can't determine the number you need, it may be better to use some sort of dynamic allocation (malloc and friends).

What value is nofl (not what type it is)?

--
Mats

Quote:

---

Originally Posted by MK27

to create a global array of structs that will grow "dynamically". But I still want to ask you...

---

There is your problem. You can't do that. Arrays can't grow dynamically. It's pretty much the definition of an array that it has a set size. Once an array has a certain set of dimensions it can't be changed.
Btw, there are no harmless warnings. All warnings should be treated as errors and in your case the warning tells you that you created your array with a size of 1. Then when you go to using it, you're most likely writing/reading past the end of the array, causing your segfault.

QuantumPete

"So ray[] is a singe element array, and using that for nofl > 0 is invalid. You need to fill in number of elements in the brackets to make sure you have enough space. If you can't determine the number you need, it may be better to use some sort of dynamic allocation (malloc and friends)." -matsp

"You can't do that. Arrays can't grow dynamically. It's pretty much the definition of an array that it has a set size. Once an array has a certain set of dimensions it can't be changed.
Btw, there are no harmless warnings." -- QuantumPete

Hmmm...this makes things rather interesting, and explains why I didn't know you could do what I did because evidently I am not suppose to. Which is a shame because

Code:

---

struct thestruct {
[...]
} ray[];
int main () {
    short int nofl, i;
    [...]
    for (i=0; i<nofl; i++) {
          ray[i].member = whatever;
    }

---

works so very, very well (except for the warning) it's almost perl. I am sure it's true that "there are no harmless warnings" but, while I am new to C (obviously) I've been compiling stuff from source in linux for years and years, and I have never ever seen a program of significant size, including the kernel, gcc and glibc itself, etc. that did not produce hundreds or thousands of such warnings while compiling and which evidently the people who designed them comnpletely expected, because it's de facto to find some comment in the README (or whereever) like this "Don't worry if you see a number of warnings produced by the compiler, these are normal." Also, I've already discovered that in my own standard issue, out of the Fedora 7 box libc 2.6, using "strcasestr" will produce a warning no matter what (just try it). Etc. So either I believe strcasestr is an illegal command, period, or assume that some warnings are less significant and appropriate than others.

Anyway, thanks for the warnings. b/t/w QuantumPete misunderstands -- this was the fix, not the problem: "the warning tells you that you created your array with a size of 1. Then when you go to using it, you're most likely writing/reading past the end of the array, causing your segfault". I haven't had a segfault since the change, just the warning, the array grows fine, and so on.

But if anyone has an example of how to grow the array using malloc and realloc "properly" please post it (how do you cast an array of structs -- (struct **)realloc(?bytes?!!) -- i haven't tried...).

In the meantime I heartily recommend people try ray[] for themselves...

nofl so far is less than 100.

ps. if this turns out to be a problem (it's going into a midsize GUI app which will get much play-testing) I promise I'll come back and admit it...

Try adding:

Code:

---

ray[];
int array[10];
...
// inside main:
{
for(i = 0; i < 10; i++)
{
  array[i] = i * 3;
};
... access ray[]
...
for(i = 0; i < 10; i++)
{
  if (array[i] != i * 3)
      printf("array corrupted at %d, got %d, expected %d\n", i, array[i], i * 3);
}

---

I bet you that you will get "array corrupted".

Arrays do not grow automatically in C or C++ - they may seem to, but only because you don't realize you've overwritten something else.

--
Mats

Quote:

---

Originally Posted by MK27

In the meantime I heartily recommend people try ray[] for themselves...

nofl so far is less than 100.

---

hmm... I suggest that you post a minimal example of what is actually in the [...].

I see two possibilities: you are accessing your array out of bounds but escaping unscathed by luck, or the C99 variable length array language feature is at work. If it is the latter, fine, but if it is the former, then you may pay a heavy price for your mistake in the future.

Quote:

---

Originally Posted by laserlight

hmm... I suggest that you post a minimal example of what is actually in the [...].

I see two possibilities: you are accessing your array out of bounds but escaping unscathed by luck, or the C99 variable length array language feature is at work. If it is the latter, fine, but if it is the former, then you may pay a heavy price for your mistake in the future.

---

Since it is a global variable, I expect that it won't be "variable length array". And the warning the compiler gives is explcitly saying that it's giving a size of 1 - so there is only one element in the array called ray.

--
Mats

Quote:

---

Originally Posted by MK27

I am sure it's true that "there are no harmless warnings" but, while I am new to C (obviously) I've been compiling stuff from source in linux for years and years, and I have never ever seen a program of significant size, including the kernel, gcc and glibc itself, etc. that did not produce hundreds or thousands of such warnings while compiling and which evidently the people who designed them comnpletely expected, because it's de facto to find some comment in the README (or whereever) like this "Don't worry if you see a number of warnings produced by the compiler, these are normal."

---

Yes, I've noticed this too, and it's a damned shame...
It's true that some warnings can be safely ignored in some instances (sometimes the compiler just over-reacts when it sees something potentially dangerous), but other times that same warning can indicate a real problem.
The problem with ignoring warning messages without fixing them is that eventually your project spits out hundreds (or thousands) of warnings, and if you make a change in the code and a new warning pops up in the middle of all that mess that is in fact a real problem, chances are you won't even notice it. So to that end, it's better to fix all warnings right from the start, or at least use #pragma's to silence specific warnings (in the smallest scope possible).

Quote:

---

Originally Posted by cpjust

Yes, I've noticed this too, and it's a damned shame...
It's true that some warnings can be safely ignored in some instances (sometimes the compiler just over-reacts when it sees something potentially dangerous), but other times that same warning can indicate a real problem.
The problem with ignoring warning messages without fixing them is that eventually your project spits out hundreds (or thousands) of warnings, and if you make a change in the code and a new warning pops up in the middle of all that mess that is in fact a real problem, chances are you won't even notice it. So to that end, it's better to fix all warnings right from the start, or at least use #pragma's to silence specific warnings (in the smallest scope possible).

---

Completely agree - projects should have no warnings [1]. And this is why certain projects use the rule that you have to use -Werror when compiling - that way, all warnings become errors, and you have to fix them. Nearly all warnings can be aovided one way or another - you may have to add some extra bits (a cast, an extra set of parenthesis, an initial value for a variable, etc) to make the warning go away, but generally it's possible and most often not enough extra work.


[1] Of course, there are situations where you are damned to eternity. I remember having a switch/case set where I had a default to cover "random acts of madness" in the compiler - gcc then comlained about "unreachable code". I removed it, and now it complained that "not all cases of the enum was covered" - how do you want it?

--
Mats

I'm not sure what matsp's example is suppose to demonstrate -- that "accessing ray[]" will corrupt the other array? Why? Because the compiler handles arrays in the same way and hence the best chance of an overwrite is with another array?

You & laserlight could be right about this, I'll let you know when my tish fits the hand. I'm not sure if ray[] will count in C99 "variable length array":
http://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html
But at least there's the possibility...boldly going and all that...

[...] includes the determination of nofl (to recap: I'm doing what must be a common task -- reading a text file of indeterminate length, each line of which (eg. "name number section") is translated into a struct. I guess I could set ray[10000] or something.

But as of yet, there's no problem...and this is in a gtk_main() context, so there's a lot of storage going on...

I may have to investigate this "pragma" thing.

nofl IS NOT global, and if you think about reading lines not, perhaps, even necessary. The warning about size of one is probably because ray[] appears to have no size at all. But in the for (i) loop, ray[i] does function and retain it's value globally.

ps. my point about the warnings intrinsic to compiling, eg. the linux kernel and C libraries, would be that even the people who built all this can't seem to get rid of them. The one with strcasestr is:

warning: assignment makes pointer from integer without a cast

which doesn't appear with strstr (tho their use is identical).

Quote:

---

But if anyone has an example of how to grow the array using malloc and realloc "properly" please post it (how do you cast an array of structs -- (struct **)realloc(?bytes?!!) -- i haven't tried...).

---

Don't cast the result of malloc/calloc/realloc in C! It's in the FAQ!

This reference isn't perfect, but it looks pretty good.

Regarding what you example is supposed to show: I choose an array because that the simplest way to create a few variables worth of memory that I could write some "pattern" to. Did you actually try to implement that?

Your code probably will crash when you reach the next 4K boundary of ray - since we don't know what ray contains, it's impossible to predict when that would happen (and it may well be that other data is stored AFTER ray, and that it just happens that overwriting that causes nothing bad that you can detect). This is the problem with undefined behaviour - it's like crossing the road when "don't walk"/"red man" showing - some days you'd get across the road fine, other days you'd get run over by a bus, and further other days, Police Constable Plod grabs you by the collar and tells you off [yeah, ok, that's unlikely - but still...].

As to strcasestr(), are you sure you added the red part in your source code?

Code:

---

#define _GNU_SOURCE
#include <string.h>

---

--
Mats

Quote:

---

nofl IS NOT global, and if you think about reading lines not, perhaps, even necessary. The warning about size of one is probably because ray[] appears to have no size at all. But in the for (i) loop, ray[i] does function and retain it's value globally.

---

Based on the available information (I still cannot fully understand variable length arrays as specified by C99 just by reading C99), I think matsp is right. ray is a global, so variable length arrays do not come into play here (from what I understand, the length of a variable length array is fixed for the duration of its lifetime: the length is variable in the sense that it is not fixed at compile time). Therefore it is just an array of length 1, in which case you have a case of array out of bounds access, which is a Bad Thing.

Okay! I tried it and matsp WAS WRONG!

Code:

---

#include <stdio.h>

struct thest {
        int one;
        int two;
} ray[];
int array[10];

int main () { 
        int i;
        for (i=0; i<10; i++) {
                array[i] = i*3;
        } 
        for (i=0; i<=4; i++) {
                ray[i].one = i;
                ray[i].two = i*2;
        } 
        for (i=0; i<=4; i++) printf("ray[&#37;d]: %d\t%d\n", i,ray[i].one,ray[i].two);
        for (i=0; i<10; i++) {
                if (array[i] != i*3) printf("array[%d] isn't %d, it's %d...\n",i,i*3,array[i]);
                else puts("No problem");
        }
}

---

Sorry the "code" tags don't work here (?!?). But the output is:

Code:

---

ray[0]: 0      0
ray[1]: 1      2
ray[2]: 2      4
ray[3]: 3      6
ray[4]: 4      8
No problem
No problem
No problem
No problem
No problem
No problem
No problem
No problem
No problem
No problem

---

But I clearly haven't hit a 4k boundary yet. Also,
#define _GNU_SOURCE
didn't make any difference with strcasestr.

Of course, now all subsequent unexplained segfaults will leave me paranoid...

A Very Bad Thing. Probably the only thing that is saving you is the fact that "ray" is a global. Thus it will be stored in the global part of memory, which on many architectures is at the bottom of the memory and will therefore likely grow into your heap. If you don't have much heap allocated memory, you might never see a problem, but it's definitly not safe and the slightest code change could break your program.
To which, I might add, that this kind of buffer overflow can be used as an exploit to gain control over your software and run arbitrary code. So it's not just unsafe but insecure as well.
On warnings: You should try to eliminate all warnings (and not by supressing them), the only time that you can ignore a warning is if you know why it's occuring, it's not trivial to fix and you understand the consequences of allowing the warning.

QuantumPete

Quote:

---

Originally Posted by MK27

Okay! I tried it and matsp WAS WRONG!

---

Erm, actually, no he's not. It's up to the compiler how to arrange your variables in memory and it could well have put array before ray. Maybe because it arranges them by descending size or because of alignment. Pick a different compiler and you may well get a different result.

QuantumPete

ARRRGHHHH!

Looks like i will be boldly returning in shame. I'm up over 200 lines in my little app and ray[] has created insurmountable problems. Oh well.

Thanks for all your input and tolerating my bull-headed ignorance.

What compiler are you using, and on what processor architecture, since with gcc-mingw shows the problem, and MS Visual Studio .Net refuses to build an executable (but if I change the [] to 1, it compiles correctly and produces the error messages I expect).

But as QuantumPete says, the compiler isn't obliged to put the variables in the order that you have them in the source - I've never seen a compiler do that, but it's not impossible.

--
Mats

gcc 4.1.2 for i386-redhat-linux

What I really should have been doing all along, as laserlight eventually points out in another thread, was this

Code:

---

struct thestruct {
[...]
} *ray;
int main () {
    short int nofl, i;
    [...determine nofl]
    ray = malloc(nofl * sizeof(*ray));
    for (i=0; i<nofl; i++) {
          ray[i].member = whatever;
    }

---

Sorry for wasting so much time...like I said, bullheaded and new to C. But I did pick up some stuff along the way.

Incidentally, must ray really be a global variable? If feasible, make it a local variable. Also, remember to free() what you malloc(), and to check that malloc() does not return NULL before using ray as a dynamically allocated array.