0

I have a program that takes as input an integer n followed by (n+1) lists. The integers in these lists are used to perform an arbitrary mathematical operation. Input will look like this for example:

5
2: [2,-2]
3: [1,-1,1]
4: [-5,3,-5,-1]
5: [2,-1,-1,-5,-2]
6: [-5,4,-3,3,4,-3]
10: [-23,9,-15,-22,5,6,-21,-15,-22,4]

The desired output is:

25: [-2300,9870,-23594,42570,-55674,58618,-44668,17698,7396,-18046,4346,25258,-50964,54140,-34960,3790,14120,-19116,16540,-9380,4494,1702,-1824,-64,48]

The way I read these lists is that I perform the operation as I read. In other words, I read a line and then perform an operation and then read the next line instead of reading every line and then performing the operation all in one go.

h_init is the array I use to "accumulate" my intermediate results in. h_next is used to store the next list that is read. temp is to store the calculated results within each iteration of the for loop. At the end of each iteration I will copy temp into h_init.

After the mathematical operation is performed, the length of the result increases therefore I use realloc to extend the length of h_init at the end of every iteration.

When I run my code with input I get the following error:

malloc: Incorrect checksum for freed object 0x7fae81505280: probably modified after being freed.
Corrupt value: 0xffffffea0000000c

So I understand that this error is saying that my error is most likely trying to access an array after I've freed it but I don't understand how and where this is happening. In my code you can see that I instantiate h_next and temp at the very start of the for loop and then I free them both just before the current iteration ends.

So my question is: where am I supposed to free temp and h_next then to avoid this error?

(See code below)

void *safeMalloc(int sz) {
  void *p = calloc(sz, 1);
  if (p == NULL) {
    fprintf(stderr, "Fatal error: safeMalloc(%d) failed.\n", sz);
    exit(EXIT_FAILURE);
  }
  return p;
}

int *makeIntArray(int n) {
  /* allocates dynamic int array of size/length n */
  return safeMalloc(n*sizeof(int));
}

void printIntArray(int length, int *arr) {
  printf("[");
  if (length > 0) {
    printf("%d", arr[0]);
    for (int i=1; i<length; i++) {
      printf(",%d", arr[i]);
    }
  }
  printf("]\n");
}

int *readSignal(int *len) {
  int *x;
  char c;
  scanf("%d:", len);
  x = calloc(*len, sizeof(int));
  do c = getchar(); while (c != '[');
  if (*len > 0) {
    scanf("%d", &x[0]);
    for (int i=1; i < *len; i++) scanf(",%d", &x[i]);
  }
  do c = getchar(); while (c != ']');
  return x;
}

void printSignal(int len, int *x) {
  printf("%d: [", len);
  if (len > 0) {
    printf("%d", x[0]);
    for (int i=1; i < len; i++) printf(",%d", x[i]);
  }
  printf("]\n");
}

int main(int argc, char *argv[]) {
  int n;
  scanf("%d\n", &n);

  int len_h_init;
  int *h_init;

  h_init = readSignal(&len_h_init);

  for (int i = 0; i < n; i++){
    int len_next;
    int *h_next;

    int len_temp;
    int  *temp;

    h_next=readSignal(&len_next);

    int temp_indent = 0;
    len_temp = (len_h_init-1) + len_next;
    temp = makeIntArray(len_temp);
  
    for(int i=0; i<len_h_init; i++){
      for(int j=0; j<len_next; j++){
        temp[temp_indent+j] = temp[temp_indent+j] + h_init[i]*h_next[j];
      }
      temp_indent = temp_indent + 1;
    }

    h_init = (int *) realloc(h_init, len_temp);
    len_h_init = len_temp;

    for(int z = 0; z < len_h_init; z++) {
      h_init[z] = temp[z];
    }
    free(temp);
    free(h_next);
  }

  printSignal(len_h_init, h_init); 

  return 0;
}
  • This is a good opportunity to learn to use a tool like valgrind, AddressSanitizer, etc. But to get help here, it would be a lot easier if people could test your code, which means you need to share the input that reproduces the problem. – Nate Eldredge Dec 05 '20 at 23:46
  • Possibly unrelated, but the return value of `getchar()` needs to be assigned to an `int` variable, not a `char`, and checked for EOF. See https://stackoverflow.com/questions/18013167/why-must-the-variable-used-to-hold-getchars-return-value-be-declared-as-int – Nate Eldredge Dec 05 '20 at 23:48
  • 3
    In `temp = makeIntArray(len_temp);`, the variable `len_temp` is uninitialized. Compiler warnings should catch that (e.g. `gcc -O -Wall`). – Nate Eldredge Dec 05 '20 at 23:51
  • @NateEldredge Hi Nate thanks for helping. I fixed the initialisation error with ```len_temp``` and its still giving me the same error. I've also edited the post to include the expected output for the input that I've listed so that you can test my code. I have never used AddressSanitiser before but I will give it a try now. – steven.wang Dec 06 '20 at 09:40
  • @NateEldredge Oh and w.r.t. ```getchar()``` the ```readSignal()``` function was provided by my professor and he says we are not to make changes to it. – steven.wang Dec 06 '20 at 09:47

1 Answers1

0

AddressSanitizer showed me that I was getting a heap buffer overflow error. I tried to fix it by freeing h_init before using realloc and that fixed the problem.

  • I don't see how that could possibly have fixed a buffer overflow. I'm afraid you may have only masked it. – Nate Eldredge Dec 06 '20 at 20:30
  • @NateEldredge I'm very inexperienced with C, could you give me an explanation why so that I can have an understanding? – steven.wang Dec 07 '20 at 16:03
  • Well, a buffer overflow normally means that you are reading or writing to array elements outside the range you have allocated, such as `int *p = malloc(4 * sizeof(int)); p[4] = 7;` (since only `p[0], p[1], p[2], p[3]` are valid). You can't fix that by freeing - you have to either correct the erroneous read/write, or change the size of the allocation. It's not clear to me exactly what you changed, but from your description it doesn't sound correct. – Nate Eldredge Dec 07 '20 at 16:29
  • As a general principle, when programming in C you want to avoid the temptation to change things at random until the problem goes away. By the nature of the language, bugs will not always manifest themselves predictably, and code that "seems to work" may still have serious, even dangerous, problems lurking, You really want to get in the habit of trying to completely understand what the problem was and why your fix truly addresses it. – Nate Eldredge Dec 07 '20 at 16:32