Do not call the alloca() function inside loops

This bug was found in Pixie project. The error is detected by the following diagnostic: V505 The ‘alloca’ function is used inside the loop. This can quickly overflow stack.

inline  void  triangulatePolygon(....) {
  ...
  for (i=1;i<nloops;i++) {
    ...
    do {
      ...
      do {
        ...
        CTriVertex *snVertex =
          (CTriVertex *) alloca(2*sizeof(CTriVertex));
        ...
      } while(dVertex != loops[0]);
      ...
    } while(sVertex != loops[i]);
    ...
  }
  ...
}

Explanation

1401582288_2087221287

The alloca(size_t) function allocates memory by using the stack. Memory allocated by alloca() is freed when leaving the function.

There’s not much stack memory usually allocated for programs. When you create a project in Visual C++, you may see that the default setting is just 1 megabyte for the stack memory size, this is why the alloca() function can very quickly use up all the available stack memory if used inside a loop.

In the example above, there are 3 nested loops at once. Therefore, triangulating a large polygon will cause a stack overflow.

It is also unsafe to use such macros as A2W in loops as they also contain a call of the alloca() function.

As we have already said, by default, Windows-programs use a stack of 1 Megabyte. This value can be changed; in the project settings find and change the parameters ‘Stack Reserve Size’, and ‘Stack Commit Size’. Details: “/STACK (Stack Allocations)“. However, we should understand that making the stack size bigger isn’t the solution to the problem – you just postpone the moment when the program stack will overflow.

Recommendation

Do not call the alloca() function inside loops. If you have a loop and need to allocate a temporary buffer, use one of the following 3 methods to do so:

  1. Allocate memory in advance, and then use one buffer for all the operations. If you need buffers of different sizes every time, allocate memory for the biggest one. If that’s impossible (you don’t know exactly how much memory it will require), use method 2.
  2. Make the loop body a separate function. In this case, the buffer will be created and destroyed right off at each iteration. If that’s difficult too, there’s only method N3 left.
  3. Replace alloca() with the malloc() function or new operator, or use a class such as std::vector. Take into account that memory allocation will take more time in this case. In the case of using malloc/new you will have to think about freeing it. On the other hand, you won’t get a stack overflow when demonstrating the program on large data to the customer.

Written by Andrey Karpov.

This error was found with PVS-Studio static analysis tool.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s