I like to help beginner programmers on StackOverflow. I thought I would do a series of articles on common errors, mainly in C but with some c++ and maybe a few other languages. Why?
- Its interesting to see the things that people get stuck on
- Maybe potential posters to SO will look here and find answers.
First error is trying to use pointers to data that only existed on the stack of a function. A typical instance might look like this:
char *getName(){
char nameBuff[50];
printf("please enter your name: ");
if(scanf("%49s", nameBuff) == 1){
return nameBuff;
} else {
return NULL;
}
}
This attempts to return a pointer to a char array on the stack of getName. Although this will compile (see later) and run it invokes Undefined Behavior (UB) and will almost certainly cause errors later.
When compiled using VisualStudio 2022 this warning is produced:
warning C4172: returning address of local variable or temporary: nameBuff
gcc says:
warning: function returns address of local variable [-Wreturn-local-addr]
[Interesting aside: gcc here actually returns NULL rather than the meaningless pointer to the soon to be discarded stack area]
The problem is that most new programmers ignore warnings, treating them as nags from a parent rather than helpful hints from the compiler.
More subtle instances are not spotted by the compilers:
struct product {
char* name;
int price;
};
void getProductName(struct product *prod) {
char nameBuff[50];
printf("please enter product name: ");
if (scanf("%49s", nameBuff) == 1) {
prod->name = nameBuff;
}
}
Here the bad pointer is stored in a struct that was passed to the function, presumably to be used later, but is not actually returned. Neither vs2022 nor gcc spotted this.
So far, I have only shown strings, but this kind of error is very commonly found in assignments for creating things like linked lists, where a function assembles a node that may have several pointers (value, next, prior,..).
struct node{
int value;
struct node *next;
};
void newNode(struct node *head, int val){
struct node n;
n.value = val;
n.next = head->next;
head->next = &n;
}
Here we have created a new node object on the stack and linked it into the chain of nodes (head.next points to data on the stack of newNode)Again, not detected by the compilers.
Why this is fails.
Lets use godbolt.org to examine the machine code produced by the first example (compiled with -O2 as this produces simpler code).
.LC0:
.string "please enter your name: "
.LC1:
.string "%49s"
getName:
sub rsp, 72 // reserve stack space
mov edi, OFFSET FLAT:.LC0
xor eax, eax
call printf
mov rsi, rsp // pass stack space address
mov edi, OFFSET FLAT:.LC1
xor eax, eax
call __isoc99_scanf
xor eax, eax // note forced return 0 regardless
add rsp, 72 // release stack space
ret
.LC2:
.string "hello: %s"
main:
sub rsp, 8
xor eax, eax
call getName
mov edi, OFFSET FLAT:.LC2
mov rsi, rax
xor eax, eax
call printf
xor eax, eax
add rsp, 8
ret
Here’s the source
#include <stdio.h>
#include <string.h>
char* getName() {
char nameBuff[50];
printf("please enter your name: ");
if (scanf("%49s", nameBuff) == 1) {
return nameBuff;
}
else {
return NULL;
}
}
int main(){
char *name = getName();
printf("hello: %s", name);
}
On x86_64, ie Intel/AMD 64 bit processor, there are a few things to know about how the stack works.
- the RSP register points to the top of the stack
- the stack grows down in memory. The deeper we get into the stack the lower RSP gets.
- when a function is called the
callinstruction puts the return address on the stack (and decrements RSP by 8). - The
retinstruction takes the saved instruction pointer off the top of the stack and increments RSP.
So in this code we see main calling getName.
When we enter that function the first instruction decrements RSP by 72. This makes space for all the stack memory this function needs. We have to do this so that when it calls printf the saving of the instruction counter plus any other stack space used by printf does not overwrite the stack memory of this function.
Note that getName only needs 50 bytes as far we are concerned. The other space reserved is for the stack space need to call scanf and printf
Note that RSP is moved to RSI before calling scanf , this is the address of our stack space reserved for nameBUff
Once the function is complete RSP is incremented back to the value it had before.
The contents of nameBuff is still there on the stack, but RSP now points to an address above (higher) than it. This means that the next stack use will overwrite it. This is what happens when we call printf in main
We have produced what is call Undefined Behavior. Its possible that printf may not try to use the stack space we used, but it probably will.
Final note — see the xor eax, eax , that is a return NULL in machine code. GCC detected we are trying to return nonsense so in fact just returned NULL as noted earlier
How to fix it
For the original getName function there are several choices. The point is that we must store the entered name in memory that will last beyond the lifetime of the function call of getName.
Choice #1 — store on the heap
The simplest way to do this is to use one of my favorite, but seemingly not widely known, functions strdup(3) — Linux manual page (man7.org). This does all the heavy lifting needed to allocate space and copy a string. So getName becomes:
char *getName(){
char nameBuff[50];
printf("please enter your name: ");
if(scanf("%49s", nameBuff) == 1){
return strdup(nameBuff);
} else {
return NULL;
}
}
[note, VS2022 calls this function _strdup]
If you don’t want to use the magic of strdup then we can do it the hard way
char *getName(){
char nameBuff[50];
printf("please enter your name: ");
if(scanf("%49s", nameBuff) == 1){
char *returnName = malloc(strlen(nameBuff) + 1);
if(returnName == NULL)
return NULL;
return strcpy(returnName, nameBuff);
} else {
return NULL;
}
}
The big issue with this is that we now have data on the heap. This means that the caller of getName now takes ownership of this memory and must release it once they have finished with it. i.e.:
char* name = getName();
printf("hello %s\n", name);
free(name);
failure to do this puts us into memory leak territory. In this case its trivail , but real life cases are much more complex.
Choice #2 — caller provides memory
Here the caller provides a memory buffer to be used for the returned name. This means we also have to change the signature of getName
char * getNameWithCallerBuffer(char*buffer, int length){
char nameBuff[50];
printf("please enter your name: ");
if(scanf("%49s", nameBuff) == 1){
strncpy(buffer, nameBuff, length - 1);
buffer[length-1] = '\0'; // force zero termination just in case
return buffer;
} else {
return NULL;
}
}
The function still returns the name string even though this is not strictly necessary, this is done to allow a NULL return to signal error. It is called like this:
char name[40];
getNameWithCallerBuffer(name, sizeof(name));
printf("hello %s\n", name);
Choice #3 — use a static buffer
In this case there is a dedicated static buffer (allocated in the program’s data segment).
char *getName(){
static char nameBuff[50];
printf("please enter your name: ");
if(scanf("%49s", nameBuff) == 1){
return nameBuff;
} else {
return NULL;
}
}
This simple change, just adding the word static, seemingly solves everything. There is no need for the caller to provide a buffer, no need to worry about memory leaks. Sadly not so (there are no free lunches in C coding), this introduces some subtle issues.
First if getName were somehow to end up calling itself, then the fact that there is only a single buffer for all names found by getName would result in the second call overwriting the first. getName here is a classic example of a ‘non reentrant function’.
For a real-life example of this type of issue look at strtok(3) — Linux manual page (man7.org). You will see that that the original version has this issue and so a ‘reentrant’ version (strtok_r) was added later.
Secondly, there is only one buffer and so if I call it a second time the first name will be overwritten.
For this simple example it’s unlikely that these potential issues will be a problem, but in a larger codebase they certainly can be.
Other cases
For the struct product case you have 2 choices. The first one is to reserve a fixed size buffer in the struct
struct product {
char name[50];
int price;
};
Alternatively, you can strdup the name into the char* name field. And remember to free it when done.
For the linked list case (BTW, please do not use my newNode code as an example of how to build a linked list, it is a bit odd in how it builds the list) you must create new nodes on the heap
void newNode(struct node *head, int val){
struct node *n = malloc(sizeof(struct node));
n->value = val;
n->next = head->next;
head->next = n;
}
And of course, you must remember to free those nodes when you are finished with them.
Jolly Good Software 
Leave a comment