16

I'm using a server with 128GB memory to do some computation. I need to malloc() a 2D float array of size 56120 * 56120. An example code is as follows:

int main(int argc, char const *argv[])
{
    float *ls;
    int num = 56120,i,j;
    ls = (float *)malloc((num * num)*sizeof(float));
    if(ls == NULL){
        cout << "malloc failed !!!" << endl;
        while(1);
    }
    cout << "malloc succeeded ~~~" << endl;
    return 0;
}

The code compiles successfully but when I run it, it says "malloc failed !!!". As I calculated, it only takes about 11GB of memory to hold the whole array. Before I started the code, I checked the server and there was 110GB of free memory available. Why does the error happen?

I also found that if I reduce num to, say 40000, then the malloc will succeed.

Does this mean that there is a limit on the maximum memory that can be allocated by malloc()?

Moreover, if I change the way of allocation, directly declaring a 2D float array of such size, as follows:

int main(int argc, char const *argv[])
{
    int num = 56120,i,j;
    float ls[3149454400];
    if(ls == NULL){
        cout << "malloc failed !!!" << endl;
        while(1);
    }
    cout << "malloc succeeded ~~~" << endl;
    for(i = num - 10 ; i < num; i ++){
        for( j = num - 10; j < num ; j++){
            ls[i*num + j] = 1;
        }
    }
    for(i = num - 11 ; i < num; i ++){
        for( j = num - 11; j < num ; j++){
            cout << ls[i*num + j] << endl;
        }
    }
    return 0;
}

then I compile and run it. I get a "Segmentation fault".

How can I solve this?

Adrian McCarthy
  • 45,555
  • 16
  • 123
  • 175
pfc
  • 1,831
  • 4
  • 27
  • 50
  • 2
    3149454400 floats is a **bit** too much for a stack-allocated array... – Eugene Sh. Jan 11 '17 at 14:49
  • 5
    One reason might be that the allocated memory has to be contiguous. If there is no such large block available then the allocation will fail. – Some programmer dude Jan 11 '17 at 14:49
  • As for the problem with the array, remember that most compilers puts their local variables on the stack, and the stack is quite limited (the default process stack size on Linux is 8MB). – Some programmer dude Jan 11 '17 at 14:50
  • 1
    What is your platform? linux 64-bit? – Ctx Jan 11 '17 at 14:50
  • Also, regarding the array, it's not an allocation in the same way that `malloc` does it. It is the compiler that handles it, and it sets the size at the time of compilation (C and C++ can differ here!). An array can *never* be `NULL`. – Some programmer dude Jan 11 '17 at 14:52
  • One thing I noticed is you're using `int i` in your calculation. Try casting those like this maybe `(size_t)i * i * sizeof float` – Zan Lynx Jan 11 '17 at 14:58
  • I meant `num` instead of `i` – Zan Lynx Jan 11 '17 at 14:58
  • 2
    You really should remove the C tag, seeing as how you use `std::cout` and `std::endl`. This then of course bears the question why you use `malloc` in the first place, and why `NULL` instead of `nullptr`, and why you declare variables before using them. – Christian Hackl Jan 11 '17 at 15:36
  • @ZanLynx Casting is not needed here as in `(size_t)i * i * sizeof float`. [Reversing](http://stackoverflow.com/a/41595104/2410359) the order accomplishes the needed widening. `sizeof (float) * i * i `. At least that stops the UB of `int` overflow. – chux - Reinstate Monica Jan 11 '17 at 17:01
  • @plugwash: It's the same problem but with a different number of bits. – Zan Lynx Jan 11 '17 at 18:05

5 Answers5

23

The problem is, that your calculation

(num * num) * sizeof(float)

is done as 32-bit signed integer calculation and the result for num=56120 is

-4582051584

Which is then interpreted for size_t with a very huge value

18446744069127500032

You do not have so much memory ;) This is the reason why malloc() fails.

Cast num to size_t in the calculation of malloc, then it should work as expected.

Ctx
  • 18,090
  • 24
  • 36
  • 51
  • 10
    num should be `size_t` not `long` – Stargateur Jan 11 '17 at 14:57
  • 2
    @Stargateur Why should it be? `num` is not a size itself, so `size_t` seems inappropriate – Ctx Jan 11 '17 at 14:58
  • Even after using `long`, it fails : http://code.geeksforgeeks.org/gkj53M – Saurav Sahu Jan 11 '17 at 15:01
  • 1
    @Ctx That's what malloc is defined like. `size_t` will correspond to one of the unsigned integer types of the platform and is defined as the type of the result of the `sizeof` operator. The beauty is that code written that way is platform independent. `size_t` may be defined as `unsigned int` or `unsigned long` or even `unsigned long long` depending on the `int` and address size. Of course you can use the native type, but the code may fail (as we see) on other platforms. – Peter - Reinstate Monica Jan 11 '17 at 15:01
  • 7
    @Ctx `malloc()` take a `size_t`, use a `size_t`. – Stargateur Jan 11 '17 at 15:01
  • @SauravSahu The site there probably has imposed some limits on how much memory a program may use there – Ctx Jan 11 '17 at 15:01
  • @Stargateur I still disagree, you could cast the result of `(num * num) * sizeof(float)` to `size_t` inside the malloc, but this is unnecessary. – Ctx Jan 11 '17 at 15:03
  • Do you know any website where there is no limit imposed? Please let me know. – Saurav Sahu Jan 11 '17 at 15:03
  • 1
    @SauravSahu I would be surprised if there was one... Just try it locally on a machine with sufficient RAM. – Ctx Jan 11 '17 at 15:05
  • 1
    @Ctx If you cast the result it will fail, you must cast `num` not the result. use cast only when you can't do something else. And when you do you have to **verify** that you can. Here `num` must be positive and inferior to `SIZE_MAX`. – Stargateur Jan 11 '17 at 15:06
  • 2
    @Stargateur I meant "leave num as long and cast the result". num is semantically _not_ a `size_t` in this program, so declaring it as one is outright misleading and wrong in my eyes. – Ctx Jan 11 '17 at 15:07
  • @PeterA.Schneider what are you trying to say? I know this command. ulimit for data segment is unlimited by default on linux, btw. – Ctx Jan 11 '17 at 15:08
  • @Ctx I thought you were not aware of it since you said " I would be surprised if there was one [a limit on memory usage]... Just try it locally on a machine with sufficient RAM." On a properly managed server with more than one user I'd actually be amazed *if there were no limits.* – Peter - Reinstate Monica Jan 11 '17 at 15:10
  • @PeterA.Schneider Yes, on a shared server with many users there should be such a limit, I agree. But it's not default on a vanilla linux installation. – Ctx Jan 11 '17 at 15:12
  • Oh yes. This is the right solution. I change the type of `num` to `long` and everything becomes correct. Thank you so much! – pfc Jan 11 '17 at 15:41
  • 5
    @Ctx *Use the data type `long` for `num`* and *you could cast the result of `(num * num) * sizeof(float)` to `size_t`* Try that on Windows, where `long` is still a 32-bit *signed* value even for 64-bit applications. Still nasty. – Andrew Henle Jan 11 '17 at 16:02
  • 1
    @AndrewHenle Well, you will always find a combination of "value of num", "datatype" and "platform" which will fail in this context. – Ctx Jan 11 '17 at 16:06
  • 1
    @Ctx Which is *why* such things as `size_t` exist. You still should check for overflow, though. – Andrew Henle Jan 11 '17 at 16:09
  • @AndrewHenle It is definitively the wrong way to declare all variables as size_t which could somehow be part of a calculation for a malloc size. Imagine using a formula based on argc. `int main (size_t argc, char **argv)`? – Ctx Jan 11 '17 at 16:11
  • 1
    @Ctx *It is definitively the wrong way to declare all variables as `size_t` which could somehow be part of a calculation for a malloc size. Imagine using a formula based on argc. `int main (size_t argc, char **argv)`?* That's an irrelevant [strawman](https://en.wikipedia.org/wiki/Straw_man). – Andrew Henle Jan 11 '17 at 16:13
  • @AndrewHenle I am sorry, but the definition of "Strawman" you linked is not appropriate for this situation at all. Please do not try to cover your lack of reasoning with such unfair methods. Thank you. – Ctx Jan 11 '17 at 16:48
  • 1
    @Ctx I think Andrew's point is that, when facing the need to allocate memory, the type `size_t` is explicitly defined to avoid these sorts of issues. You could do a platform-specific approach, true. Alternatively, you could use `size_t` and get the correct result on all platforms, including future ones! Even if you don't declare the variables as `size_t`, you should be doing your math in that type. – Cort Ammon Jan 11 '17 at 17:25
  • Time to move to chat? I won't edit the answer to `size_t` due to the arguing, but it would improve it. – Kahler Jan 11 '17 at 17:27
  • 1
    Using "long" in C is generally the wrong answer, if you want something of a fixed size use one of the C99 fixed size types. If you want something that represents the maximum size of a single allocation use size_t. – plugwash Jan 11 '17 at 17:29
  • @CortAmmon Yes, casting it to size_t in the calculation itself would also be an appropriate option. If this is what we all can live with, I will edit the answer to this. – Ctx Jan 11 '17 at 17:36
  • 1
    @Ctx: `num` is semantically unsigned. It is meaningless to make it negative. Making it a `size_t` would make a lot of sense. – Zan Lynx Jan 11 '17 at 18:10
10

As other have pointed out, 56120*56120 overflows int math on OP's platform. That is undefined behavior (UB).

malloc(size_t x) takes a size_t argument and the values passed to it is best calculated using at least size_t math. By reversing the multiplication order, this is accomplished. sizeof(float) * num cause num to be widened to at least size_t before the multiplication.

int num = 56120,i,j;
// ls = (float *)malloc((num * num)*sizeof(float));
ls = (float *) malloc(sizeof(float) * num * num);

Even though this prevents UB, This does not prevent overflow as mathematically sizeof(float)*56120*56120 may still exceed SIZE_MAX.

Code could detect potential overflow beforehand.

if (num < 0 || SIZE_MAX/sizeof(float)/num < num) Handle_Error();

No need to cast the result of malloc().
Using the size of the referenced variable is easier to code and maintain than sizing to the type.
When num == 0, malloc(0) == NULL is not necessarily an out-of-memory.
All together:

int num = 56120;
if (num < 0 || ((num > 0) && SIZE_MAX/(sizeof *ls)/num < num)) {
  Handle_Error();
}
ls = malloc(sizeof *ls * num * num);
if (ls == NULL && num != 0) {
  Handle_OOM();
}
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
7
int num = 56120,i,j;
ls = (float *)malloc((num * num)*sizeof(float));

num * num is 56120*56120 which is 3149454400 which overflows a signed int which causes undefined behavoir.

The reason 40000 works is that 40000*40000 is representable as an int.

Change the type of num to long long (or even unsigned int)

Mike Vine
  • 9,468
  • 25
  • 44
  • 5
    num should be `size_t` not `long long` or even `unsigned int`. – Stargateur Jan 11 '17 at 14:58
  • 2
    @Stargateur actually, any type that is large enough to hold the number will do just fine. A long, or even unsigned int which can hold a number that size will work. – UKMonkey Jan 11 '17 at 15:01
  • 5
    @UKMonkey `malloc()` take a `size_t`, use a `size_t`. Don't mix type or you will have problem like this question... – Stargateur Jan 11 '17 at 15:02
  • 1
    @Stargateur That depends on the use case, we don't really know what `num` means in this context. Imagine it means something like 'the number of dimensions I'm doing my calculation in' then that's not a good reason to make it a `size_t`. Yes, it is used in an expression which will eventually lead to a `size_t` as memory for those dimension is allocated, but in general you dont propagate types backwards like that and make `num` a `size_t`. – Mike Vine Jan 11 '17 at 15:04
  • 2
    @MikeVine If `num` represent the size of the array, yes it should be `size_t`. Or the OP need to cast it properly. Here you say "use a long long" but what if the value don't fit in a `long long` next time? You propose to change the type of `num` why don't use the proper type? – Stargateur Jan 11 '17 at 15:11
  • 1
    @Stargateur Semantically, `num` should be whatever it properly represents. What if we also had a later line which went `float angle = (1/num) * 360.0f;`. This has a very similar problem. Would you also fix this by saying 'num should be a float'? In which case it needs to be a float AND a size_t... The only way round this is by properly casting to the right type _at the point of use_ and having its local type be semantically whatever makes sense no matter what its use case is later. – Mike Vine Jan 11 '17 at 15:19
  • 1
    @Stargateur you almost certainly mix types all the time without knowing it. unsigned int x = 5 for example mixes types. – UKMonkey Jan 11 '17 at 15:31
  • 1
    @UKMonkey "any type that is large enough to hold the number will do just fine." is true yet considering `long` or `unsigned int` for a memory size calculation is not using the best type available, which is `size_t` as that is the type `malloc()` receives. [Code can check memory requirements](http://stackoverflow.com/a/41595104/2410359) using just `size_t` and the type of `num`. No need to introduce another type. – chux - Reinstate Monica Jan 11 '17 at 17:10
6

This is in contrast to what others have written, but for me, changing the variable num to size_t from int allows allocation. It could be that num*num overflows the int for malloc. Doing malloc with 56120 * 56120 instead of num*num should throw an overflow error.

Kostas
  • 127
  • 5
2

float ls[3149454400]; is an array with automatic storage type, which is usually allocated on the process stack. A process stack is limited by default by a value much smaller than 12GB you are attempting to push there. So the segmentation fault you are observing is caused by the stack overflow, rather than by the malloc.

Eugene Sh.
  • 17,802
  • 8
  • 40
  • 61
  • @Stargateur Of course. It would move it from the automatic storage into the static storage. Still not a very good idea though... The startup code will have to set the whole array to zeros, which might be a bit time-consuming. – Eugene Sh. Jan 11 '17 at 15:03
  • 1
    Going slightly off topic here, but most mainstream OS's would _probably_ actually manage this being a very large array in static storage without being very time consuming - they either share a 'zero page' or have a supply of 'already zero'd pages' or only allocate a zero page when accessed. Still probably not a great idea though - large allocations should always be on the heap. – Mike Vine Jan 11 '17 at 15:26
  • It's also not what the OP is doing - 3x10^9 is the result of the multiplication, but the overflow is resulting in a very different request – UKMonkey Jan 11 '17 at 15:33