-1

Here is a basic stack implementation code. However, it generates signal abort.

int *arr;
int size = 2;
int top = 0;

int pop() {

    int i;
    if (top <= size / 4) {
        int *arr2 = (int*)malloc(sizeof(int) * size / 2);
        for ( i = 0; i < size; i++)
            arr2[i] = arr[i];
        free(arr);
        arr = arr2;
        size /= 2;
    }
    return arr[--top];
}

void push( int a) {

    int i;
    if (top >= size) {
        int *arr2 = (int*)malloc(sizeof(int)*size * 2);
        for ( i = 0; i < size; i++)
            arr2[i] = arr[i];
        free(arr);
        arr = arr2;
        size *= 2;
    }   
    arr[top++] = a;
}

Here is the output:

*** glibc detected *** a.out: free(): invalid pointer: 0x0804a030 ***

and the debug data shows aborted sig 6 The interesting thing is that it shows the line of free() but As 4386427 said, the problem is accessing out of bounds of memory while copying arr2 of size

enter image description here

0 0xffffe410 in __kernel_vsyscall ()

1 0xb7e8a7d0 in raise () from /lib/libc.so.6

2 0xb7e8bea3 in abort () from /lib/libc.so.6

3 0xb7ebff8b in __libc_message () from /lib/libc.so.6

4 0xb7ec5911 in malloc_printerr () from /lib/libc.so.6

5 0xb7ec6f84 in free () from /lib/libc.so.6

6 0x080484a0 in pop () at stacks_eng.c:14

7 0x0804867e in main () at stacks_eng.c:55 (gdb) f 6

6 0x080484a0 in pop () at stacks_eng.c:14 14 free(arr);

trincot
  • 317,000
  • 35
  • 244
  • 286
roll
  • 125
  • 13
  • How and where is `arr` initialized? – Ajay Brahmakshatriya May 21 '18 at 11:18
  • On what inputs does the program crash? Provide a [MCVE] – Ajay Brahmakshatriya May 21 '18 at 11:21
  • You return arr , which is already free'd. – Tsakiroglou Fotis May 21 '18 at 11:24
  • OT: You should consider putting `arr`, `top`, `size` into a struct and pass that struct to the `push`/`pop` functions. Global variables are bad.... – Support Ukraine May 21 '18 at 11:37
  • On a side-note, [don't cast malloc()](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858). – Tim Čas May 21 '18 at 11:51
  • Tim Čas why are you offering not to cast. It is necessary not to get a warning from GCC, isn't it? Moreover, it provides parsing 4 bytes between arr[i] and arr[i+1] – roll May 21 '18 at 13:48
  • Tsakiroglou Fotis, no I've already assigned a different address of the memory in which arr2 stores into variable arr, so it's not a freed address anymore. – roll May 21 '18 at 13:51
  • Please [edit] your question to show us what kind of debugging you've done. I expect you to have run your [mcve] within Valgrind or a similar checker, and to have investigated with a debugger such as GDB, for example. Ensure you've enabled a full set of compiler warnings, too. What did the tools tell you, and what information are they missing? And read Eric Lippert's [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). – Toby Speight May 21 '18 at 17:25
  • Your image of text [isn't very helpful](//meta.unix.stackexchange.com/q/4086). It can't be read aloud or copied into an editor, and it doesn't index very well, meaning that other users with the same problem are less likely to find the answer here. Please [edit] your post to incorporate the relevant text directly (preferably using copy+paste to avoid transcription errors). – Toby Speight May 22 '18 at 10:21

1 Answers1

5

There may be more issues but here is a starter:

int *arr2 = (int*)malloc(sizeof(int) * size / 2);

This makes the size of arr2 be half of size and then you do:

    for ( i = 0; i < size; i++)
        arr2[i] = arr[i];

So you are clearly writing out of bounds, i.e. undefined behavior.

Maybe you wanted:

size /= 2;

before the loop.

BTW: Check the realloc function. It seems to be what you need. It will perform better and you wont have to write the code to copy elements yourself.

Support Ukraine
  • 42,271
  • 4
  • 38
  • 63
  • Many thanks. It worked clearly. The reason that I missed the point is that I focused more on free () function where gdb points. And Since it could be a bit complicated, I couldn't analyze Valgrind's feedback. I wish I would have listened to him. – roll May 21 '18 at 13:07
  • Yes, definitely look into `realloc`. It will save you from having to copy the array every time as it will be able to simply shrink its footprint when resizing to be smaller, and when growing, will grow in place if possible. `realloc` aside, you can use `memcpy` to copy a contiguous set of memory. – Christian Gibbons May 21 '18 at 13:17