0

I tried to access a pointer but the program crashes. With memory access error. I receive the pointer from a stack. (pop- function). As a void*-pointer. Why an I getting this behavior?

int main()
{
    int MAX = 5;
    int field[MAX];
    int i; /* for the loop */
    int *pInt = NULL;

    initStack();

    for (i = 0; i < MAX; i++)
    {
        field[i] = i;
        push((field + i));  // HERE CRASH
    }

    for (i = 0; i < MAX; i++)
    {
        pInt = pop();   /* fetch next integer */

        printf("%d\n", *pInt); /* print value */
    }



    return EXIT_SUCCESS;
}

UPDATE:

I have tested my stack. And it works. But with for-loops it crashes.

My stack implementation. I get the error always I access to the pointer.

#include <stdio.h>
#include <stdlib.h>
#include <assert.h>

#include "stack.h"

/* 
   actual stack data structure
   This pointer will pointing at the actual field (of void * pointers) 
   that represents the stack.
 */
void **array;

/* the current capacity of the stack */
int max = 10;

/* counter variable for counting the elements of the stack. */
int counter = 0;

/* 
    offset address 
    points at the top element of the stack.
*/
int offset = -1;

void initStack(void)
{

    array = malloc(sizeof(void) * max);
    assert(array); /* tests whether pointer is assigned to memory. */
}

/*
    grow: increases the stack by 10 elements.
          This utility function isn't part of the public interface
*/
void grow(void)
{
    max += 10; /* increases the capacity */

    int i; // for the loop
    void **tmp = malloc(sizeof(void) * max);

    /* copies the elements from the origin array in the new one. */
    for (i = 0; i < max - 10; i++)
    {
        *(tmp + i) = *(array + i);
    }

    array = tmp; /* setups the new one as basis */
}

/* push: pushs the argument onto the stack */
void push(void *object)
{

    assert(object); /* tests whether pointer isn't null */

    if (offset < max)
    {

        offset++; /* increases the element-pointer */

        /* 
            moves pointer by the offset address 
            pushes the object onto stack 
         */
        *(array + offset) = object;
    }
    else /* stack is full */
    {

        /* TODO */
        grow();
        push(object); /* recursive call */
    }
}

/*
    pop: pops the top element of the stack from the stack.
*/
void *pop(void)
{
    printf("\tBEFORE\n"); //DEBUG
    void *top = *(array + offset);
    assert(top);
    assert(array + offset);
    printf("\tAFTER  void *top = *(array + offset);\n"); //DEBUG
    // int *pInt = top;
    // printf("\tpop: value= %d\n", *top); /* DEBUG */

    /* decreases the offset address for pointing of
        the new top element */
    offset--;

    return top;
}
Paul Floyd
  • 5,530
  • 5
  • 29
  • 43
  • 1
    _Questions seeking debugging help (why isn't this code working?) must include the desired behavior, a specific problem or error and the shortest code necessary to reproduce it in the question itself. Questions without a clear problem statement are not useful to other readers. See: How to create a [mcve]._ – Sourav Ghosh Dec 08 '17 at 15:40
  • 1
    `(int *)pop()` this address must be invalid. [mcve] would be nice. – Jean-François Fabre Dec 08 '17 at 15:41
  • @ChristianBender It is unclear how field is declared and defined. – Vlad from Moscow Dec 08 '17 at 15:45
  • @Jean-FrançoisFabre I have removed (int *) but still error. – ChristianBender Dec 08 '17 at 15:48
  • @VladfromMoscow sorry. I have updated my comment. – ChristianBender Dec 08 '17 at 15:50
  • @ChristianBender This statement array = malloc(sizeof(void) * max); does not make sense because the type void is incomplete and the sizeof( void ) is undefined or some compilers make it equal to 1. – Vlad from Moscow Dec 08 '17 at 15:51
  • so many malloc and never free, this is leaking lot of memory, realloc can simplify your code – Ôrel Dec 08 '17 at 15:51
  • sizeof(void) is never a good decision. – Justin Randall Dec 08 '17 at 15:54
  • `sizeof(void)` is not only not a good decision, it is a *constraint violation*: "The sizeof operator shall not be applied to an expression that has [...] an incomplete type" ([C2011, 6.5.3.4/1](http://port70.net/~nsz/c/c11/n1570.html#6.5.3.4p1)). As such, conforming compilers are obligated to emit a diagnostic about it. I find it a bit surprising that GCC (v4.8.5) relegates this particular diagnostic to those that it presents only when its `-pedantic` option is in effect, but in fact it does so. – John Bollinger Dec 08 '17 at 17:03

2 Answers2

3

There is an error in your stack implementation. In both initStack and grow, you do this:

malloc(sizeof(void) * max);

This is invalid, as void doesn't have a size, although some compilers will evaluate this to 1. So you aren't allocating enough space for an array of void *. As a result, you write past the end of allocated memory which invokes undefined behavior.

Change the type you're getting the size of to void * in both places.

malloc(sizeof(void *) * max);
dbush
  • 205,898
  • 23
  • 218
  • 273
  • It is perhaps worth noting that this sort of mixup can be avoided generically by referring directly to the size of the elements being allocated. For example, `array = malloc(sizeof(*array) * max)`. That even continues to work if the type to which `array` points is later changed. – John Bollinger Dec 08 '17 at 15:56
  • Whenever you are operating on pointer add Null check always and also push field +i will be invalid index when i value is max -1.for example id you have array [5] valid index will be 0 to 5 now if you are accesing array out of bound program will crash – Rohit Dec 13 '17 at 01:48
1

the issue is that kind of allocation:

void initStack()
{

    array = malloc(sizeof(void) * max);
}

sizeof(void) is illegal but some compilers consider it legal like gcc, which in that case returns 1, which isn't enough for your int pointer.

So you could fix those by passing the size of the element:

void initStack(int sz)
{
    array = malloc(sz * max);

call by

initStack(sizeof(int *));
Jean-François Fabre
  • 137,073
  • 23
  • 153
  • 219