llvm dev - Mar 2014 - [LLVMdev] getElapsedWallTime unnecessary heap allocation and memory leak

In the file \lib\Support\Process.cpp on line 60, it seems as though an
unnecessary heap allocation and memory leak occurs.

This is the offending code:

static TimeValue getElapsedWallTime() {
  static TimeValue &StartTime = *new TimeValue(TimeValue::now());
  return TimeValue::now() - StartTime;
}

The issue is that the StartTime variable's value is allocated on the heap,
after which a *reference* to it is stored (not the pointer itself). This means
that the allocated memory is actually never properly de-allocated.
Granted, the memory leak is small but if anyone attempts to write memory
leak-free software while including LLVM code, this will be an issue (as it
currently is for our company).


Is there anyone who knows why exactly this was implemented the way it currently
is implemented? It seems rather unnecessary to be implemented with this
complexity when a simpler implementation would work as well:

static TimeValue getElapsedWallTime() {
  static TimeValue TimeValue(TimeValue::now());
  return TimeValue::now() - StartTime;
}

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140319/4b247c4c/attachment.html>

Probably because so many static allocations (in this and other small
functions) may lead to increase in the size of the executable, if there
were say even a 100 functions all having a static allocated variable,
it would need around 400 byte in the executable, a bigger object will
require even more.

That's the best explanation I can think of.

-- 
http://www.fastmail.fm - The professional email service

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140319/8a6fba0b/attachment.html>

Hi Bryan,

On Wed, Mar 19, 2014 at 5:17 PM, Bryan Keiren <
bryan.keiren at guerrilla-games.com> wrote:
>  In the file \lib\Support\Process.cpp on line 60, it seems as though an
> unnecessary heap allocation and memory leak occurs.
>
>
>
> This is the offending code:
>
>
>
> *static TimeValue getElapsedWallTime() {*
>
> *  static TimeValue &StartTime = *new TimeValue(TimeValue::now());*
>
> *  return TimeValue::now() - StartTime;*
>
> *}*
>
>
>
> The issue is that the *StartTime* variable's value is allocated on the
> heap, after which a **reference** to it is stored (not the pointer
> itself). This means that the allocated memory is actually never properly
> de-allocated.
>
What is the difference between storing a pointer and a reference here?

>  Granted, the memory leak is small but if anyone attempts to write memory
> leak-free software while including LLVM code, this will be an issue (as it
> currently is for our company).
>
>
>
>
>
> Is there anyone who knows why exactly this was implemented the way it
> currently is implemented? It seems rather unnecessary to be implemented
> with this complexity when a simpler implementation would work as well:
>
>
>
> *static TimeValue getElapsedWallTime() {*
>
> *  static TimeValue TimeValue(TimeValue::now());*
>
> *  return TimeValue::now() - StartTime;*
>
> *}*
>
Function-local static variables might be dangerous and lead to
destruction-order problems. For example, if a program calls
getElapsedWallTime() at destruction time (after main() exits), then static
variable TimeValue might already
be destroyed/deallocated. Memory pointed to by StartValue will most likely
be considered as "reachable" until the end of the program, and
shouldn't
cause error reports by leak detectors, such as LeakSanitizer, for instance (
http://clang.llvm.org/docs/LeakSanitizer.html)

>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>

-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140319/77e5df98/attachment.html>

Hi Alexey,

The reason why our code considers this a leak is because just before termination
of the application, our memory management code checks to see if there have been
any allocations that have not yet been matched with a de-allocation. Because we
have overloaded the new operator, the allocation of this variable passes through
our own memory allocation routines and therefore is considered by the system
when checking for leaks.

I can see how this would not need to be considered a leak, as the memory is
available to be used for the entire program execution. However, if you’re trying
to clean the entire heap before your application terminates (something we like
to think isn’t such a far-fetched goal), then this allocation is an issue.

I also understand your point regarding the dangers of having a function local
static as in the code I proposed earlier.
Would you say that in this case there is no way to ensure a fully cleaned heap
before main() exits without changing our definition of a memory leak?
Cheers,
Bryan

From: Alexey Samsonov [mailto:samsonov at google.com]
Sent: Wednesday, March 19, 2014 14:46
To: Bryan Keiren
Cc: llvmdev at cs.uiuc.edu
Subject: Re: [LLVMdev] getElapsedWallTime unnecessary heap allocation and memory
leak

Hi Bryan,

On Wed, Mar 19, 2014 at 5:17 PM, Bryan Keiren <bryan.keiren at
guerrilla-games.com<mailto:bryan.keiren at guerrilla-games.com>> wrote:
In the file \lib\Support\Process.cpp on line 60, it seems as though an
unnecessary heap allocation and memory leak occurs.

This is the offending code:

static TimeValue getElapsedWallTime() {
  static TimeValue &StartTime = *new TimeValue(TimeValue::now());
  return TimeValue::now() - StartTime;
}

The issue is that the StartTime variable's value is allocated on the heap,
after which a *reference* to it is stored (not the pointer itself). This means
that the allocated memory is actually never properly de-allocated.

What is the difference between storing a pointer and a reference here?

Granted, the memory leak is small but if anyone attempts to write memory
leak-free software while including LLVM code, this will be an issue (as it
currently is for our company).


Is there anyone who knows why exactly this was implemented the way it currently
is implemented? It seems rather unnecessary to be implemented with this
complexity when a simpler implementation would work as well:

static TimeValue getElapsedWallTime() {
  static TimeValue TimeValue(TimeValue::now());
  return TimeValue::now() - StartTime;
}

Function-local static variables might be dangerous and lead to destruction-order
problems. For example, if a program calls getElapsedWallTime() at destruction
time (after main() exits), then static variable TimeValue might already
be destroyed/deallocated. Memory pointed to by StartValue will most likely be
considered as "reachable" until the end of the program, and
shouldn't cause error reports by leak detectors, such as LeakSanitizer, for
instance (http://clang.llvm.org/docs/LeakSanitizer.html)



_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu<mailto:LLVMdev at cs.uiuc.edu>        
http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



--
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140319/98a594d1/attachment.html>