989613 - OOM: Check oom of SnapshotWriter::allocWriter_

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Nicolas B. Pierron [:nbp]]

Attachment: Add oom checks

Bug 962555 part 5 adds a new CompactBufferWriter, but the oom method of SnapshotWriter was not updated.

Comment 1

[Hannes Verschore [:h4writer]]

Comment on attachment 8398931 [details] [diff] [review]
Add oom checks

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

Good find! Sorry I overlooked this when reviewing part 5.

::: js/src/jit/Snapshots.h
@@ +340,5 @@
>      void endSnapshot();
>  
>      bool oom() const {
> +        return writer_.oom() || writer_.length() >= MAX_BUFFER_SIZE
> +            || allocWriter_.oom() || allocWriter_.length() >= MAX_BUFFER_SIZE;

Style nit: the || should be on the previous line.

Comment 2

[Nicolas B. Pierron [:nbp]]

The problem appears with changeset 69bc4df5ede5 which is only on nightly for the moment.

Comment 3

[Nicolas B. Pierron [:nbp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/59c24af38f64

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/59c24af38f64

Comment 5

[Matt Wobensmith [:mwobensmith][:matt:]]

Marking [qa-] due to lack of test or STR. If you would like verification, please feel free to provide something to test with. Thank you.