1

My function is as follows: 

void Insert_ldb(int t){
    struct node_ldb *temp_ldb1,*lastnode_ldb;
    temp_ldb1=root_ldb[t];
    while(temp_ldb1->next!=NULL)
        temp_ldb1=temp_ldb1->next;
    if(temp_ldb1->next==NULL){
         lastnode_ldb=malloc(sizeof(*lastnode_ldb));//error appears at this line
         temp_ldb1->next=lastnode_ldb;
    }
}

and the struct node_ldb is defined as:

struct node_ldb{
    int sno;
    int *lvar;
    int *object;
    struct node_ldb *next;
};

On compiling no error appears, but on executing it terminates with the message:

malloc.c:3096: sYSMALLOc: Assertion (old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask) == 0) failed. Aborted

The weird part is that the same function executes successfully many times prior to termination. So is it possible that the error happened somewhere else? Because even valgrind does not show any error for the same. What could be the problem?

Jens
  • 69,818
  • 15
  • 125
  • 179
ceedee
  • 227
  • 1
  • 4
  • 13
  • 2
    `temp_ldb1=root_ldb[t];` Is there any guarantee that this pointer will never be NULL (or invalid) ? – wildplasser Sep 07 '13 at 13:22
  • @wildplasser Thanks for the correction. I thought `*lastnode_ldb` causes the dereferencing which is not the case. – Mahesh Sep 07 '13 at 13:25
  • No, sizeof is _mostly_ a compile-time operator. It gets different for VLAs. BTW (to the OP) is there a VLA involved? Please add the definition for the struct to the question. – wildplasser Sep 07 '13 at 13:29
  • Did the test runs under Valgrind led to the assertion as shown in your question. – alk Sep 07 '13 at 13:36
  • 2
    Side comment: spaces don't cost anything and they make code easier to read. – lurker Sep 07 '13 at 13:40
  • Ok. No VLA involved. Just a spurious overwrite somewhere else in your program (or the `temp_ldb1=root_ldb[t]` thing) – wildplasser Sep 07 '13 at 13:41
  • @wildplasser root_ldb[t] is initialized at the start. So it will not be NULL. – ceedee Sep 07 '13 at 13:41
  • 2
    Looks like the malloc arena got modified/trashed by some undefined behavior happening before your Insert_ldb() is called. – Jens Sep 07 '13 at 13:42
  • And `t` is guaranteed to be within bounds (and not negative) ? – wildplasser Sep 07 '13 at 13:42
  • @wildplasser Its within bound. I tried running the code and checked the value of t just before the malloc. – ceedee Sep 07 '13 at 13:46
  • What does *`root_ldb[t]` is initialized at the start* mean? I take it `root_ldb` is a global variable, or static in the module? How is it defined? Also (another side comment), the check `if (temp_ldb1->next == NULL)` following a `while (temp_ldb1->next != NULL)` is probably superfluous. – lurker Sep 07 '13 at 13:46
  • @mbratch : it indeed is highly suspicious. (I would use a pointer to pointer-construct in this case. And a for() loop instead of the ugly while) – wildplasser Sep 07 '13 at 13:48
  • it is initialised using the function void Initialize_LDB(int t){ root_ldb[t]=malloc(sizeof(struct node_ldb)); root_ldb[t]->sno=0; root_ldb[t]->object=0; root_ldb[t]->lvar=0; root_ldb[t]->next=NULL; } – ceedee Sep 07 '13 at 13:52
  • Do you initialise the newly allocced struct in/after this function, too? (you should **at least** set the ->next member to NULL) BTW: the test `if(temp_ldb1->next==NULL){` is not necessary. After the loop it will **always** be NULL. – wildplasser Sep 07 '13 at 14:06
  • Possible duplicate of old and highly rated question http://stackoverflow.com/questions/2987207/why-do-i-get-a-c-malloc-assertion-failure However, the answer is pretty much the same. – Ted Sep 04 '14 at 16:20

3 Answers3

3

This intensly smells like a memory management corruption happened prior to this call to malloc().

The corrupted memory management data then made this call to malloc() fail.

I strongly recommend to run the program using a memory checker like for example Valgrind until the malfunction had been reproduced.

alk
  • 69,737
  • 10
  • 105
  • 255
  • The problem was actually with the way i was allocating memory earlier. I was actually assigning the memory of size size(struct) to a pointer(to the struct), whereas i should have assigned size(struct *). Thanks.. – ceedee Sep 11 '13 at 20:52
1

Memory is getting corrupted. You may want to try out few simple things.

1) Put a counter in Insert_ldb. Hopefully the program fails at the same counter value. If it does, it may make it easier to debug.

2) Add few bytes of padding to malloc, e.g. starting with 8 bytes.

3) It's generally a good idea to initialize the contents after getting memory with malloc.

#define PAD_BYTES       8

void Insert_ldv(int t)
{
    static int counter;
    struct node_ldb *temp_ldb1, *lastnode_ldb;

    counter++;
    printf("counter = %d\n", counter);

    temp_ldb1 = root_ldb[t];   
    while (temp_ldb1->next != NULL) {
        temp_ldb1 = temp_ldb1->next;
    }                               

    if (temp_ldb1->next == NULL){
        lastnode_ldb = malloc(sizeof(*lastnode_ldb) + PAD_BYTES);
        memset(lastnode_ldb, 0, sizeof(*lastnode_ldb));
        temp_ldb1->next = lastnode_ldb;
   }                                 
}

When the program doesn't fail with padding, then you have a workaround for the short term. Increase PAD_BYTES by multiples of 4 until it doesn't fail. When working fine, PAD_BYTES is the number of bytes that the memory is overflowing.

How do lvar and object get set? I am guessing that object has probably more data items than memory allocated for it and as a consequence it is overwriting and corrupting the heap.

Also, when does the memory get freed?

Arun Taylor
  • 1,574
  • 8
  • 5
1

This most likely indicates heap corruption.

There are some weird things about that function. For example, the next filed of the new node is left uninitialized. Why? (Not even mentioning the rest of the fields.)

Also the temp_ldb1->next==NULL check in the if looks excessive, since the preceding while cycle ensures already that it is null at that point.

P.S. The authors of that sYSMALLOc used a rather bad programming practice of writing ultra-complex assertion conditions. Now we can't figure out which specific sub-condition failed.

AnT stands with Russia
  • 312,472
  • 42
  • 525
  • 765