0

I am incrementing pointer variable for allocation but its giving me segfault message. Nothing goes wrong with the program since my program get executed until the end except return statement. So basically I am thinking

when we increment pointer like pointer+1 we are moving forward on contiguous memory and inside function -- may be in reality -- addresses of variables and function calls could be located in dispersed manner -- correct me if I am wrong. And when we increment pointer to move into next contiguous memory address and that memory could be anything (some other function/program) and my allocation is done on some area that serves memory for something else and may be something outside the memory region of abc function. this is my code

    #include <stdio.h>

    struct abc{ char *c;};

    void abc(struct abc **t1);

    int main() {
        struct abc *k;
        abc(&k);
        printf("%s\n",k->c);
     
       // printf("%s\n",k->c);
        printf("%s\n",(k+1)->c);
       // printf("%s\n",(k+2)->c);

       return 1;
    }

    void abc(struct abc **t1){


        *t1=(struct abc *)malloc(sizeof(struct abc ));
        (*t1)->c="hello";
        *(t1+1)=(struct abc *) malloc((sizeof(struct abc )));
        (*t1+1)->c="cool";
        *(t1+2)=(struct abc *) malloc((sizeof (struct abc )));
        //(*t1+2)->c="Super cool";
    }

also another question is why it seems like I am having segment fault at return of main function. since main memory area is different from abc memory area. I need some explanation on this and question on is this way of allocation is wrong. if its not wrong the how to correct to make it work without seg fault

Update

    #include <stdio.h>

    struct abc{ char *c;};

    void abc(struct abc **t1);
    void abc2(struct abc *t1);
    int main() {
        struct abc *k;
        abc(&k);
        //printf("%s\n",k->c);
      // k->c="hi"; Seg Fault causes because ->c is immutable from assigning string literal
       // printf("%s\n",k->c);
        printf("%s\n",(k)->c);
        printf("%s\n",(k+1)->c);
        printf("%s\n",(k+2)->c);
    //   / printf("%s\n",(k+2)->c);

       return 1;
    }

    void abc(struct abc **t1){


        *t1=(struct abc *)malloc(sizeof(struct abc)*3);
        abc2((*t1));
           abc2((*t1+1));
        abc2((*t1+2));
     
    }

    void abc2(struct abc *t1){
        (t1)->c="cool";
    }

the above code works without segFault but its different from the answers so what's wrong with the above code

As below also works

        #include <stdio.h>

        struct abc{ char *c;};

        void abc(struct abc **t1);
        void abc2(struct abc *t1);
        int main() {
            struct abc *k;
            abc(&k);
            //printf("%s\n",k->c);
          // k->c="hi"; Seg Fault causes because ->c is immutable from assigning string literal
           // printf("%s\n",k->c);
            printf("%s\n",(k)->c);
            printf("%s\n",(k+1)->c);
            printf("%s\n",(k+2)->c);
        //   / printf("%s\n",(k+2)->c);

           return 1;
        }

        void abc(struct abc **t1){


            *t1=(struct abc *)malloc(sizeof(struct abc)*3);
            (*t1)->c="Cool";
            (*t1+1)->c="more cool";
            (*t1+2)->c="super cool";
            //below also works
            abc2((*t1));
             abc2((*t1+1));
         abc2((*t1+2));
         /*   (*t1)->c="hello";
            //*(t1+1)=(struct abc *) malloc((sizeof(struct abc)));
            (*t1+1)->c="cool";
            //*(t1+2)=(struct abc *) malloc((sizeof (struct abc)));
            (*t1+2)->c="Super cool";*/
        }

        void abc2(struct abc *t1){
            (t1)->c="cool";
        }
user786
  • 3,902
  • 4
  • 40
  • 72
  • @TomKarzes but this works `*t1=(struct abc *)malloc(sizeof(struct abc )*3);` – user786 Mar 14 '21 at 06:31
  • @TomKarzes is the above line make sense or there is something wrong – user786 Mar 14 '21 at 06:36
  • `malloc(sizeof(struct abc)*3)` allocate an array of 3 structs. 3x `malloc(sizeof(struct abc)` doesn't. – n. m. could be an AI Mar 14 '21 at 06:57
  • But note, you should have some way to communicate from your function back to the caller how many you allocated. Perhaps changing the return type to return that number or passing an additional pointer parameter and updating the value at that memory location with the number allocated will both do. – David C. Rankin Mar 14 '21 at 07:29
  • Actually, you're assuming `t1` is an array, so you would need to pass an array of three pointers for this to work. – Tom Karzes Mar 14 '21 at 08:57
  • @DavidC.Rankin `Perhaps changing the return type to return that number or passing an additional pointer parameter` that's what I did in larger program – user786 Mar 14 '21 at 14:26

2 Answers2

0

The value you're passing to abc() is pointing to uninitialized memory. you must malloc space for the three structs you wish to store. Otherwise, dereferencing it is undefined behavior. It seems to me that your goal is to have k be an array of 3 pointers to structs. If this is the case, you should declare it like this:

struct abc **k = (struct abc **)malloc(sizeof(struct abc *) * 3);

This should allocate 12 bytes (4 for each pointer) on a 32-bit system, or 24 bytes (8 for each pointer) on a 64-bit system.

Since k is now a pointer-to-a-pointer instead of just a pointer, you no longer need to get its address when calling abc(), so the next line becomes:

abc(k);

Inside the abc function, you should not be manually incrementing the values to pointers. That is the purpose of the [] operator. You should also manually cast your strings to char*s. After making those changes, your abc function should look like this:

void abc(struct abc **t1){
    t1[0]=(struct abc *)malloc(sizeof(struct abc));
    t1[0]->c=(char *) "hello";
    
    t1[1]=(struct abc *)malloc(sizeof(struct abc));
    t1[1]->c=(char *) "cool";
    
    t1[2]=(struct abc *)malloc(sizeof(struct abc));
    t1[2]->c=(char *) "Super cool";
}

Notice how t1[0] and *t1 do the same thing. The latter should always be used with arrays to make it clear that that t1 is, in fact, an array.

You should make similar changes in your printf() calls, so that they look like this:

printf("%s\n",k[0]->c);
printf("%s\n",k[1]->c);
printf("%s\n",k[2]->c);

Finally, you should always free your malloc'd memory! Begin by freeing each pointer in the array:

free(k[0]);
free(k[1]);
free(k[2]);

And then free the array itself:

free(k);

Getting in the habit of always freeing your data will be very helpful in preventing painful memory errors in larger programs.

After all that, your final program should look something like this:

#include <stdio.h>

struct abc {
    char *c;
};

void abc(struct abc **t1);

int main() {
    struct abc **k = (struct abc **)malloc(sizeof(struct abc *) * 3);
    abc(k);
    
    printf("%s\n",k[0]->c);
    printf("%s\n",k[1]->c);
    printf("%s\n",k[2]->c);
    
    free(k[0]);
    free(k[1]);
    free(k[2]);
    
    free(k);

    return 1;
}

void abc(struct abc **t1){
    t1[0]=(struct abc *)malloc(sizeof(struct abc));
    t1[0]->c=(char *) "hello";
    t1[1]=(struct abc *)malloc(sizeof(struct abc));
    t1[1]->c=(char *) "cool";
    t1[2]=(struct abc *)malloc(sizeof(struct abc));
    t1[2]->c=(char *) "Super cool";
}

To answer your final question, the behavior and timing of segfaults can be a very tricky and unpredictable thing. They usually result from undefined behavior which might not even be the same between different executions of the same program. What you should know is that the location in the program where the segfault occurs has no relation to the location in memory where it occurs.

Digit
  • 68
  • 5
  • The first sentence of the answer is incorrect. The value passed to `abc` is an initialized pointer, but it points to an uninitialized variable. – Ian Abbott Mar 14 '21 at 09:47
  • Your description of pointer arithmetic is wrong. Adding 1 to a pointer increments the address it contains by sizeof. – stark Mar 14 '21 at 12:18
  • @stark sizeof char pointer is 8 bytes so does this mean size of struct pointer `abc *` is also 8 bytes. And how much `alignment/padding` would be for `struct abc *` – user786 Mar 14 '21 at 14:39
0

The allocation approach is correct, but, you had forgotten to initialize the k, so k is a dangling pointer. This is why you have a segmentation fault. I applied a simple "dirty" fix by adding a variable that the compiler will initialize, just to prove that this is the problem.

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

struct abc{ char *c;};

void abc(struct abc **t1);

int main() {
    struct abc _abc[2], *k;
        k = &_abc[0];
    abc(&k);
    printf("%s\n",k->c);

    printf("%s\n",k->c);
    printf("%s\n",(k+1)->c);
    //printf("%s\n",(k+2)->c);

    return 1;
}

void abc(struct abc **t1){


    *t1=(struct abc *)malloc(sizeof(struct abc ));
    (*t1)->c=malloc(50);
    sprintf((*t1)->c, "hello");
    *(t1+1)=(struct abc *) malloc((sizeof(struct abc )));
    (*t1+1)->c=malloc(50);
    sprintf((*t1+1)->c, "cool");
    *(t1+2)=(struct abc *) malloc((sizeof (struct abc )));
    //(*t1+2)->c="Super cool";
}

after compiling

~$ gcc memtest.c -o memtest
~$ ./memtest
hello
hello
cool

To understand why you have the segmentation faiult in main and not in abc, you need to convert the example in assembler gcc test.c -S -ggdb3. In my case, on ARM64, the k is optimized out. Before a call to abc() the compiler inserts a __stack_chk_guard. Here is the segment in assembly (ARM64)

    .arch armv8-a
    .file   "memtest_bug.c"
    .text
.Ltext0:
    .align  2
    .global main
    .type   main, %function
main:
.LFB6:
    .file 1 "memtest_bug.c"
    .loc 1 8 16
    .cfi_startproc
    stp x29, x30, [sp, -32]!
    .cfi_def_cfa_offset 32
    .cfi_offset 29, -32
    .cfi_offset 30, -24
    mov x29, sp
    .loc 1 8 16
    adrp    x0, :got:__stack_chk_guard
    ldr x0, [x0, #:got_lo12:__stack_chk_guard]
    ldr x1, [x0]
    str x1, [sp, 24]
    mov x1,0
    .loc 1 10 9
    add x0, sp, 16
    bl  abc

When abc() is called k points to a memory region belonging to the process, so it's still a valid segment and there will be no segmentation fault generated. When the main finishes, the compiler inserts a code to check the stack guard and if check fails, it calls __stack_chk_fail.

    mov x3, 0
    beq .L3
    bl  __stack_chk_fail
.L3:
    mov w0, w1
    ldp x29, x30, [sp], 32
    .cfi_restore 30
    .cfi_restore 29
    .cfi_def_cfa_offset 0
    ret
    .cfi_endproc

This code detects the fault of the memory guards and generates segmentation fault. This is the stack when the fault is generated (on ARM64)

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x0000fffff7e7cd68 in __GI_abort () at abort.c:79
#2  0x0000fffff7eca29c in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0xfffff7f89f58 "*** %s ***: terminated\n") at ../sysdeps/posix/libc_fatal.c:155
#3  0x0000fffff7f3c600 in __GI___fortify_fail (msg=msg@entry=0xfffff7f89f40 "stack smashing detected") at fortify_fail.c:26
#4  0x0000fffff7f3c5d4 in __stack_chk_fail () at stack_chk_fail.c:24
#5  0x0000aaaaaaaaa8e4 in main () at memtest_bug.c:18
jordanvrtanoski
  • 5,104
  • 1
  • 20
  • 29
  • 1
    `but, you had forgotten to initialize the k, so k is a dangling pointer in your code, this is why you had segmentation fault. I applied a simple "dirty" fix by... ` please check the updated code in my question and explain is it still good idea to initialize the pointer. is it `good thing to initialize pointer the way u did` for real world code like `struct abc _abc[2], *k;
    k = &_abc[0];` this way I believe we fooled the compiler so it will never seg-Fault. But my updated code is also not causing seg-fault. Or I am wrong if my updated code may throw segFault
    – user786 Mar 14 '21 at 14:58
  • @Alex as I said, it's dirty fix. I don't know how you will use this code, so I applied a fix to demonstrate what is wrong. If you intend to have dynamic size of the array to which `k` is pointing, then for sure my `dirty fix` will not work. In such case I would use dynamic allocation and reallocate the array once I need to grow it for additional `x` places as described [in SO here](https://stackoverflow.com/questions/29140656/changing-size-of-array-in-c-using-realloc) – jordanvrtanoski Mar 14 '21 at 15:08
  • and it seems you updated your code while I was writing the answer, so yes, your updated code is also working fine since you will allocate a memory for the array. – jordanvrtanoski Mar 14 '21 at 15:09