1081374 - |BaselineCompiler::compile| can leak |baselineScript|

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Eric Rahm [:erahm]]

|BaselineCompiler::compile| [1] can leak |baselineScript| [2] when it early returns [3] if |globalTable->addEntry(entry)| fails.

[1] http://hg.mozilla.org/mozilla-central/annotate/e4cfacb76830/js/src/jit/BaselineCompiler.cpp#l73
[2] http://hg.mozilla.org/mozilla-central/annotate/e4cfacb76830/js/src/jit/BaselineCompiler.cpp#l182
[3] http://hg.mozilla.org/mozilla-central/annotate/e4cfacb76830/js/src/jit/BaselineCompiler.cpp#l258

Comment 1

[Jan de Mooij [:jandem]]

Regression from bug 1004831. Maybe we can use mozilla::UniquePtr with a policy that calls BaselineScript::Destroy...

Comment 2

[Kannan Vijayan [:djvj]]

Attachment: bug-1081374.patch

Test patch.  Passes jit-tests with --ion-eager.

Try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=a9918300e631

Comment 3

[Kannan Vijayan [:djvj]]

Attachment: bug-1081374.patch

Rebased patch.  Running in try.

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8521519 [details] [diff] [review]
bug-1081374.patch

Review of attachment 8521519 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/BaselineCompiler.cpp
@@ +4,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #include "jit/BaselineCompiler.h"
> +#include "mozilla/UniquePtr.h"

Nit: add a blank line before this one.

@@ +180,5 @@
>  
>      // Note: There is an extra entry in the bytecode type map for the search hint, see below.
>      size_t bytecodeTypeMapEntries = script->nTypeSets() + 1;
>  
> +    mozilla::UniquePtr<BaselineScript, JS::FreePolicy> baselineScript(

This should be DeletePolicy: BaselineScript has a LifoAlloc for IC stubs and we should run its destructor.

Comment 5

[Kannan Vijayan [:djvj]]

 https://hg.mozilla.org/integration/mozilla-inbound/rev/d146f4b9606d

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d146f4b9606d