Dismiss Notice
Join Physics Forums Today!
The friendliest, high quality science and math community on the planet! Everyone who loves science is here!

C/++/# GCC warning: function may return address of local variable

  1. Sep 24, 2018 #1
    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:
    Code (C):
    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:
    Code (Text):
    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};
                                ^
     
     
    Last edited: Sep 24, 2018
  2. jcsd
  3. Sep 24, 2018 #2

    jedishrfu

    Staff: Mentor

    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.
     
  4. Sep 24, 2018 #3

    wle

    User Avatar

    It's warning you that you are returning the address of a local array. Your second return for instance is equivalent to
    Code (C):
        double result[] = {S[k] / k, - C[k] / k};
        return result;
     
  5. Sep 24, 2018 #4
    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?
     
  6. Sep 24, 2018 #5

    wle

    User Avatar

    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:
    Code (C):
    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.
     
  7. Sep 24, 2018 #6
    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!
     
  8. Sep 24, 2018 #7

    Vanadium 50

    User Avatar
    Staff Emeritus
    Science Advisor
    Education Advisor
    2017 Award

    Code (Text):

    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.
     
  9. Sep 24, 2018 #8
    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?
     
  10. Sep 24, 2018 #9

    Mark44

    Staff: Mentor

    Fixed it for ya:
    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.
     
  11. Sep 24, 2018 #10

    Vanadium 50

    User Avatar
    Staff Emeritus
    Science Advisor
    Education Advisor
    2017 Award

    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?
     
  12. Sep 25, 2018 #11
    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!
     
  13. Oct 13, 2018 #12
    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?
     
  14. Oct 13, 2018 #13

    Vanadium 50

    User Avatar
    Staff Emeritus
    Science Advisor
    Education Advisor
    2017 Award

    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.
     
  15. Oct 13, 2018 #14
    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.
     
    Last edited: Oct 13, 2018
  16. Oct 13, 2018 #15
    BTW I found this question on Stack Overflow which seems to cover arguments both ways.
     
  17. Oct 13, 2018 #16

    Vanadium 50

    User Avatar
    Staff Emeritus
    Science Advisor
    Education Advisor
    2017 Award

    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.
     
  18. Oct 14, 2018 #17
    You are assuming too much here. There are no memory leaks. Never mind.
     
Share this great discussion with others via Reddit, Google+, Twitter, or Facebook

Have something to add?
Draft saved Draft deleted