tags:

views:

1313

answers:

10

A colleague of mine recently got bitten badly by writing out of bounds to a static array on the stack (he added an element to it without increasing the array size). Shouldn't the compiler catch this kind of error? The following code compiles cleanly with gcc, even with the -Wall -Wextra options, and yet it is clearly erroneous:

int main(void)
{
  int a[10];
  a[13] = 3;  // oops, overwrote the return address
  return 0;
}

I'm positive that this is undefined behavior, although I can't find an excerpt from the C99 standard saying so at the moment. But in the simplest case, where the size of an array is known as compile time and the indices are known at compile time, shouldn't the compiler emit a warning at the very least?

A: 

There are some extension in gcc for that (from compiler side) http://www.doc.ic.ac.uk/~awl03/projects/miro/

on the other hand splint, rat and quite a few other static code analysis tools would have found that.

You also can use valgrind on your code and see the output. http://valgrind.org/

another widely used library seems to be libefence

It's simply a design decision ones made. Which now leads to this things.

Regards Friedrich

Friedrich
In the Windows world, recent versions of Visual Studio come with static analysis tools to help catch issues like this at compile-time. Here's a reference that I found after a brief search: http://msdn.microsoft.com/en-us/library/d3bbz7tz.aspx.
Reuben
+6  A: 

You're right, the behavior is undefined. C99 pointers must point within or just one element beyond declared or heap-allocated data structures.

I've never been able to figure out how the gcc people decide when to warn. I was shocked to learn that -Wall by itself will not warn of uninitialized variables; at minimum you need -O, and even then the warning is sometimes omitted.

I conjecture that because unbounded arrays are so common in C, the compiler probably doesn't have a way in its expression trees to represent an array that has a size known at compile time. So although the information is present at the declaration, I conjecture that at the use it is already lost.

I second the recommendation of valgrind. If you are programming in C, you should run valgrind on every program, all the time until you can no longer take the performance hit.

Norman Ramsey
+2  A: 

It's not a static array.

Undefined behavior or not, it's writing to an address 13 integers from the beginning of the array. What's there is your responsibility. There are several C techniques that intentionally misallocate arrays for reasonable reasons. Andhis situation is not unusual in incomplete compilation units.

Depending on your flag settings, there are a number of features of this program that would be flagged, such as the fact that the array is never used. And the compiler might just as easily optimize it out of existence and not tell you - a tree falling in the forest.

It's the C way. It's your array, your memory, do what you want with it. :)

(There are any number of lint tools for helping you find this sort of thing; and you should use them liberally. They don't all work through the compiler though; Compiling and linking are often tedious enough as it is.)

le dorfier
It is in fact undefined behavior. You are correct that it is writing to address 13 of the array and that several C techniques use do this...but that just means that the result can be reliably predicted in many situations.
Evan Teran
In fact, just *pointing* anywhere besides NULL and inside a properly allocated "object" (or one element beyond an array's bounds) is technically undefined behavior in itself. (though i agree nothing bad should happen if you are just pointing).
Evan Teran
@Evan: In C it's not undefined. Using arr[13]=1 is simply syntactic sugar on *(arr+13)=1 - C has never made the distinction. I seem to recall a number of legitimate things that did this sort of thing, like a struct with a tail element of arr[0], where the required space was malloc'd.
Software Monkey
Yes, the actual pointer operations are not a problem, but where does arr+13 point? And what will happen if you change that memory location? That is what is undefined in this example! After all, that's the place where the variable send_dirty_email_to_your_mother_flag could be stored.
Thomas Padron-McCarthy
Software Monkey: Where in the standard does it say this is legal? In C++, it is clearly undefined as Evan says. It'd surprise me if C allowed it, whether or not it is used in practice (which it is in a lot of C code)
jalf
This certainly is undefined behavior as is *(arr + 13), see §6.5.6p8. Note though that compilers are not required to provide a diagnostic for undefined behavior.
Robert Gamble
@Thomas: The pointer operation is a problem, as soon as the operation arr + 13 is executed, you create a pointer that points outside the bounds of the array that the original pointer pointed into and immediately invoke undefined behavior.
Robert Gamble
This is also undefined: int a[5][5]; a[1][7] = 10; even though it points to valid memory in object a, the object a[1] is an array of 5 ints and a[1][7] points outside of *that* array so the reference is undefined.
Robert Gamble
i don'T see how this can get 3 upvotes even though it *is* a static array (with static being what one understand with that. i.e not static storage duration), and it *is* undefined behavior
Johannes Schaub - litb
static has a well-defined meaning in C. (If you want to use some other meaning, you need to declare it. "on the stack" makes it ambiguous and confusing.) It was also the first answer to suggesting the compiler is optimizing the problem away - possibly the most likely so far.
le dorfier
Robert: Yes, I agree. My response was to Software Monkey, who seemed to say that arr[13] being interpreted as *(arr+13) would in some way help. Sorry for being unclear.
Thomas Padron-McCarthy
+1  A: 

shouldn't the compiler emit a warning at the very least?

No; C compilers generally do not preform array bounds checks. The obvious negative effect of this is, as you mention, an error with undefined behavior, which can be very difficult to find.

The positive side of this is a possible small performance advantage in certain cases.

Colin
The only performance advantage would be during compilation - this would have absolutely no effect on runtime performance. And I highly doubt that the performance hit during compilation would be non-negligible.
Adam Rosenfield
Some compilers can insert bounds checking code into the program to do checking at runtime in which case performance could definitely be affected but as Adam said, the effect of producing a warning during compile time would be at most a small, one time, hit.
Robert Gamble
A: 

I believe that some compilers do in certain cases. For example, if my memory serves me correctly, newer Microsoft compilers have a "Buffer Security Check" option which will detect trivial cases of buffer overruns.

Why don't all compilers do this? Either (as previously mentioned) the internal representation used by the compiler doesn't lend itself to this type of static analysis or it just isn't high enough of the writers priority list. Which to be honest, is a shame either way.

Evan Teran
A: 

-fbounds-checking option is available with gcc.

worth going thru this article http://www.doc.ic.ac.uk/~phjk/BoundsChecking.html

'le dorfier' has given apt answer to your question though, its your program and it is the way C behaves.

FL4SOF
+13  A: 

G++ does warn about this. But you need to do two things:

  1. Enable optimization. Without at least -O2, GCC is not doing enough analysis to know what the heck a is, and that you ran off the edge.
  2. Change your example so that a[] is actually used, otherwise GCC generates a no-op program and has completely discarded your assignment.

.

$ cat foo.c 
int main(void)
{
  int a[10];
  a[13] = 3;  // oops, overwrote the return address
  return a[1];
}
$ gcc -Wall -Wextra  -O2 -c foo.c 
foo.c: In function ‘main’:
foo.c:4: warning: array subscript is above array bounds

BTW: If you returned a[13] in your test program, that wouldn't work either, as GCC optimizes out the array again.

derobert
Good catch there with optimization - I'd forgotten that GCC doesn't do data flow analysis without optimizations turned on. I'll have to investigate my coworker's problem more thoroughly to see why it didn't warn there.
Adam Rosenfield
The question and your example use gcc but your introduction talks about g++, can you change this to gcc?
Robert Gamble
@Robert Gamble: Fixed.
derobert
The warning for this does not work too well in practice - http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35587 Also, it gives false positives if you use raw arrays with algorithms http://forum.gbadev.org/viewtopic.php?t=15505
rq
Good point: without optimization, enough analysis is not done.
Lazer
+3  A: 

Have you tried -fmudflap with GCC? These are runtime checks but are useful, as most often you have got to do with runtime calculated indices anyway. Instead of silently continue to work, it will notify you about those bugs.

-fmudflap -fmudflapth -fmudflapir For front-ends that support it (C and C++), instrument all risky pointer/array dereferencing operations, some standard library string/heap functions, and some other associated constructs with range/validity tests. Modules so instrumented should be immune to buffer overflows, invalid heap use, and some other classes of C/C++ programming errors. The instrumen‐ tation relies on a separate runtime library (libmudflap), which will be linked into a program if -fmudflap is given at link time. Run-time behavior of the instrumented program is controlled by the MUDFLAP_OPTIONS environment variable. See "env MUDFLAP_OPTIONS=-help a.out" for its options.

Use -fmudflapth instead of -fmudflap to compile and to link if your program is multi-threaded. Use -fmudflapir, in addition to -fmudflap or -fmudflapth, if instrumentation should ignore pointer reads. This produces less instrumentation (and there‐ fore faster execution) and still provides some protection against outright memory corrupting writes, but allows erroneously read data to propagate within a program.

Here is what mudflap gives me for your example:

[js@HOST2 cpp]$ gcc -fstack-protector-all -fmudflap -lmudflap mudf.c        
[js@HOST2 cpp]$ ./a.out
*******
mudflap violation 1 (check/write): time=1229801723.191441 ptr=0xbfdd9c04 size=56
pc=0xb7fb126d location=`mudf.c:4:3 (main)'
      /usr/lib/libmudflap.so.0(__mf_check+0x3d) [0xb7fb126d]
      ./a.out(main+0xb9) [0x804887d]
      /usr/lib/libmudflap.so.0(__wrap_main+0x4f) [0xb7fb0a5f]
Nearby object 1: checked region begins 0B into and ends 16B after
mudflap object 0x8509cd8: name=`mudf.c:3:7 (main) a'
bounds=[0xbfdd9c04,0xbfdd9c2b] size=40 area=stack check=0r/3w liveness=3
alloc time=1229801723.191433 pc=0xb7fb09fd
number of nearby objects: 1
[js@HOST2 cpp]$

It has a bunch of options. For example it can fork off a gdb process upon violations, can show you where your program leaked (using -print-leaks) or detect uninitialized variable reads. Use MUDFLAP_OPTIONS=-help ./a.out to get a list of options. Since mudflap only outputs addresses and not filenames and lines of the source, i wrote a little gawk script:

/^ / {
    file = gensub(/([^(]*).*/, "\\1", 1);
    addr = gensub(/.*\[([x[:xdigit:]]*)\]$/, "\\1", 1);
    if(file && addr) {
        cmd = "addr2line -e " file " " addr
        cmd | getline laddr
        print $0 " (" laddr ")"
        close (cmd)
        next;
    }
}

1 # print all other lines

Pipe the output of mudflap into it, and it will display the sourcefile and line of each backtrace entry.

Also -fstack-protector[-all] :

-fstack-protector Emit extra code to check for buffer overflows, such as stack smashing attacks. This is done by adding a guard variable to functions with vulnerable objects. This includes functions that call alloca, and functions with buffers larger than 8 bytes. The guards are initialized when a function is entered and then checked when the function exits. If a guard check fails, an error message is printed and the program exits.

-fstack-protector-all Like -fstack-protector except that all functions are protected.

Johannes Schaub - litb
+2  A: 

The reason C doesn't do it is that C doesn't have the information. A statement like

int a[10];

does two things: it allocates sizeof(int)*10 bytes of space (plus, potentially, a little dead space for alignment), and it puts an entry in the symbol table that reads, conceptually,

a : address of a[0]

or in C terms

a : &a[0]

and that's all. In fact, in C you can interchange *(a+i) with a[i] in (almost*) all cases with no effect BY DEFINITION. So your question is equivalent to asking "why can I add any integer to this (address) value?"

* Pop quiz: what is the one case in this this isn't true?

Charlie Martin
But the C compiler does keep track of the size, otherwise sizeof wouldn't work on arrays.
Matthew Crumley
See what sizeof does with an array that's not defined in the same file.
Charlie Martin
That's a good point. I'm not saying you're wrong, just that the compiler does remember the size within the compilation unit and could detect some overflows.
Matthew Crumley
+1  A: 

The C philosophy is that the programmer is always right. So it will silently allow you to access whatever memory address you give there, assuming that you always know what you are doing and will not bother you with a warning.

PolyThinker