CODEWITHSUNDEEP
c++c
NJMR
NJMR

Reputation: 1926

Static data memory limit in C++ application

Someone has written function in our C++ application and is already in the production and I don't know why it's not crashing the application yet. Below is the code.

char *str_Modify()
{
    char buffer[70000] = { 0 };
    char targetString[70000] = { 0 };

    memset(targetString, '\0', sizeof(targetString));

    ...
    ...
    ...
    return targetString;
}

As you can see, the function is returning the address of a local variable and the allocated memory will be released once the function returns. My question

Upvotes: 4

Views: 1282

Answers (4)

Useless
Useless

Reputation: 67752

Wanted to know what is the static data memory limit?

Platform-specific. You haven't specified a platform (OS, compiler, version), so no-one can possibly tell you. It's probably fine though.

What can be the quick fix for this code?

The quick fix is indeed to make the buffer static.

The good fix is to rewrite the function as

char *modify(char *out, size_t outsz) {
    // ...
    return out;
}

(returning the input is just to simplify reusing the new function in existing code).

Is it a good practice to make the variable targetString static?

No. Sometimes it's the best you can do, but it has a number of problems:

  1. The buffer is always the same size, and always using ~68Kb of memory and/or address space for no good reason. You can't use a bigger one in some contexts, and a smaller one in others. If you really have to memset the whole thing, this incurs a speed penalty in situations where the buffer could be much smaller.

  2. Using static (or global) variables breaks re-entrancy. Simple example: code like

    printf("%s,%s\n", str_Modify(1), str_Modify(2));

    cannot work sanely, because the second invocation overwrites the first (compare strtok, which can't be used to interleave the tokenizing of two different strings, because it has persistent state).

  3. Since it isn't re-entrant, it also isn't thread-safe, in case you use multiple threads. It's a mess.

Upvotes: 1

msc
msc

Reputation: 34628

It is well defined behaviour in C and C++ because return an address of a static variable exist in memory after function call is over.

For example:

#include <stdio.h>

int *f()
{
    static int a[10]={0};
    return a;
}
int main() 
{
    f();
    return 0;
}

It is working fine on GCC compiler. [Live Demo]

But, if you remove static keyword then compiler generates warning:

prog.c: In function 'f':
prog.c:6:12: warning: function returns address of local variable [-Wreturn-local-addr]
     return a;
            ^

Also, see this question comments wrote by Ludin.

I believe you are confusing this with int* fun (void) { static int i = 10; return &i; } versus int* fun (void) { int i = 10; return &i; }, which is another story. The former is well-defined, the latter is undefined behavior.

Also, tutorialspoint say's :

Second point to remember is that C does not advocate to return the address of a local variable to outside of the function, so you would have to define the local variable as static variable.

Upvotes: 1

NickJH
NickJH

Reputation: 571

Returning targetString is indeed UB as other answers have said. But there's another supplemental reason why it might crash on some platforms (especially embedded ones): Stack size. The stack segment, where auto variables usually live, is often limited to a few kilobytes; 64K may be common. Two 70K arrays might not be safe to use.

Making targetString static fixes both problems and is an unalloyed improvement IMO; but might still be problematic if the code is used re-entrantly from multiple threads. In some circumstances it could also be considered an inefficent use of memory.

An alternative approach might be to allocate the return buffer dynamically, return the pointer, and have the calling code free it when no longer required.

As for why might it not crash: if the stack segment is large enough and no other function uses enough of it to overwrite buffer[] and that gets pushed first; then targetString[] might survive unscathed, hanging just below the used stack, effectively in a world of its own. Very unsafe though!

Upvotes: 1

Bathsheba
Bathsheba

Reputation: 234735

(Note that your call to memset has no effect, all the elements are zero-initialised prior to the call.)

It's not crashing the application since one manifestation of the undefined behaviour of your code (returning back a pointer to a now out-of-scope variable with automatic storage duration) is not crashing the application.

Yes, making it static does validate the pointer, but can create other issues centred around concurrent access.

And pick your language: In C++ there are other techniques.

Upvotes: 6

Related Questions