GCC warning: function may return address of local variable

[m4r35n357]

Yes I've already looked on SO! None of the answers contain the word "may". My story is that I am passing in references to global variables, modifying them, and returning them in array literals. I suppose it must complaining about the array literals (c99) rather than the variables themselves, but how should I re-write it ([EDIT] the code runs and gives the correct answer BTW)?

Here is the function:

double *t_sin_cos (double *S, double *C, double *U, int k) {
    assert((S != C) && (S != U) && (C != U));
    assert(k >= 0);
    if (k == 0) {
        sincos(U[0], S, C);
        return (double[2]) {S[0] / k, - C[0] / k};
    } else {
        S[k] = 0.0;
        C[k] = 0.0;
        for (int j = 0; j < k; j++) {
            S[k] += (k - j) * U[k - j] * C[j];
            C[k] += (k - j) * U[k - j] * S[j];
        }
        return (double[2]) {S[k] / k, - C[k] / k};
    }
}

and here is the error message:

taylor-ode.c: In function ‘t_sin_cos’:
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
taylor-ode.c:70:28: note: declared here
         return (double[2]) {S[0] / k, - C[0] / k};
                            ^
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
taylor-ode.c:78:28: note: declared here
         return (double[2]) {S[k] / k, - C[k] / k};
                            ^

 

[jedishrfu]

You could create copies of the variables and return that. The warning is that you may be returning an address on the stack that will only be valid as long as you don't call more functions which call functions... and obliterate the area containing the local variable.

It's only a warning and your code works but the danger here is that some future modification may cause it not to work and so you should heed it. If I got this warning I would definitely heed it because debugging a C/C++ program for a runtime stack based error is no fun.

 

[wle]

I suppose it must complaining about the array literals (c99) rather than the variables themselves

It's warning you that you are returning the address of a local array. Your second return for instance is equivalent to

    double result[] = {S[k] / k, - C[k] / k};
    return result;

 

[m4r35n357]

It's warning you that you are returning the address of a local array. Your second return for instance is equivalent to

    double result[] = {S[k] / k, - C[k] / k};
    return result;

OK thanks, so my first thought is to create a global (within the same file) struct or array to hold the result, and assign it within the method before returning. Is this right, or am I creating another anti-pattern?

 

[wle]

OK thanks, so my first thought is to create a global (within the same file) struct or array to hold the result, and assign it within the method before returning.

You could do that, but you should avoid using global variables when you don't need them.

Instead you can do one of two things: 1) allocate the array before calling the function and pass a pointer to it to your function (like how strcpy works) or 2) return the result in a two-element structure, e.g. like this:

typedef struct { double x, y; } pair;

pair t_sin_cos (double *S, double *C, double *U, int k) {
    /* code */
        return (pair) {S[k] / k, - C[k] / k};
    }
}

This actually returns a copy of the structure so it's safe.

 

[m4r35n357]

typedef struct { double x, y; } pair;

pair t_sin_cos (double *S, double *C, double *U, int k) {
    /* code */
        return (pair) {S[k] / k, - C[k] / k};
    }
}

This actually returns a copy of the structure so it's safe.

I like your option 2, as it seems more in the spirit of my attempt. As I currently understand it my mistake was not to declare the array outside the function. So if I have to do that then a struct would be much nicer anyway. Thanks for the tip!

 

[Vanadium 50]

return (double[2]) {S[k] / k, - C[k] / k};

Returns a pointer to an array that didn't exist when the function was called. The compiler is correct to complain. When the function terminates, that space is freed, and the pointer points at some free space that at one time held the correct answer.

Also, does the asset mean the input arrays don't overlap or that they don't have the same first element.

 

[m4r35n357]

return (double[2]) {S[k] / k, - C[k] / k};

Returns a pointer to an array that didn't exist when the function was called. The compiler is correct to complain. When the function terminates, that space is freed, and the pointer points at some free space that at one time held the correct answer.

Also, does the asset mean the input arrays don't overlap or that they don't have the same first element.

I'm still working on the first part, it's not urgent as I already have working arbitrary precision code. I just wanted to play a bit doing a double version with functions returning values (the existing code uses pointer parameters). Also I have probably been influenced by my Python version which returns tuple values in some cases, but the fine details are of course hidden ;)

As to your second question, the asserts are just to make sure that all the appropriate parameters are unique (I re-use a few temporary variables), I have not considered overlapping as I believe all arrays are dimensioned and allocated correctly (I don't manipulate addresses myself), and nothing is copied or changes size. Is that a good enough argument?

 

[Mark44]

Fixed it for ya:

Also, does the assert mean the input arrays don't overlap or that they don't have the same first element.

have not considered overlapping as I believe all arrays are dimensioned and allocated correctly

As V50 already said, returning a pointer to memory that's local to the function is very bad practice. You really should be allocating space for the array in the calling function, not in the called function.

 

[Vanadium 50]

Is that a good enough argument?

Well, if you are arguing you don't need to test more than you do because you know you set them up right, why do the assert (or...um... "asset"...oops) at all?

 

[m4r35n357]

Well, if you are arguing you don't need to test more than you do because you know you set them up right, why do the assert (or...um... "asset"...oops) at all?

OK guys steady on, I am not going to return the address of a local variable, really. I'm either going to fix the code as per #5 or abandon it as the MPFR version is the one I use and it doesn't do that anywhere. BTW I'm not a software dev or intending to be one, and I'm not trying to sell anything. The problem domain is far more interesting to me than the implementation to be honest. These programs are simple tiny things, however I have worked as a software tester so do I understand what you are saying really ;)

Oh, the asserts are just there as additional (on top of the compiler) tripwires in case I make a refactoring boo-boo, they are not meant to be exhaustive.

Having said that, thanks for all your help, my question is well and truly answered, hope it is useful to others learning c!

 

[m4r35n357]

OK I think this is pertinent to the OP.

I have since looked into a function that I wrote to create arrays of MPFR variables. It uses malloc() and returns the address of that memory, and yet it works without raising errors. Apparently this does not count as a local variable for the purposes of the error message!

My mistake was to refactor something like this to return either an array or struct defined within the function. Apparently this does count as a local variable for the purposes of the error message.

My conclusion? Always use manual memory allocation for factory functions. Is that reasonable?

 

[Vanadium 50]

Apparently this does not count as a local variable for the purposes of the error message!

Because the memory is allocated, and not freed until free is called.

When exactly is free called? How do you ensure it is only called once (i.e. malloc/free are always in pairs)? I have a feeling that you have reinvented something called the "memory leak". It is not good.

 

[m4r35n357]

Because the memory is allocated, and not freed until free is called.

When exactly is free called? How do you ensure it is only called once (i.e. malloc/free are always in pairs)? I have a feeling that you have reinvented something called the "memory leak". It is not good.

You probably won't like this, but the program relies on the OS to free memory at exit. The program does not run continuously, it does its job then terminates. As I understand how OS and programs work, this is OK,but I'd be interested to know otherwise.

 

[m4r35n357]

BTW I found this question on Stack Overflow which seems to cover arguments both ways.

 

[Vanadium 50]

You probably won't like this

Don't much care. You've been told the right way to do this. If writing one extra line is too much trouble, and the code happens not at this time to trigger anything bad, it's not my problem if you have problems down the road.

 

[m4r35n357]

Don't much care. You've been told the right way to do this. If writing one extra line is too much trouble, and the code happens not at this time to trigger anything bad, it's not my problem if you have problems down the road.

You are assuming too much here. There are no memory leaks. Never mind.