Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2483053005:
[wasm] Check data segments for zero-sized memory. (Closed)
Description[wasm] Check data segments for zero-sized memory.
According to the spec data segments are allowed even if the memory size
is zero. However, if one of the data segments has a length greater than
0, then module instantiation should fail.
I also changed the exception type in LoadDataSegments to TypeError,
because that's the exception type for all exceptions which can happen
during instantiation.
R=titzer@chromium.org, rossberg@chromium.org
TEST=cctest/test-run-wasm-module/EmptyMemoryEmptyDataSegment, cctest/test-run-wasm-module/EmptyMemoryNonEmptyDataSegment
Committed: https://crrev.com/e3c7324a2e0bf7317fd12302a02ed4bddc856a8c
Cr-Commit-Position: refs/heads/master@{#40922}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Comments addressed #Patch Set 3 : Rebase #Patch Set 4 : remove redundant '(' ')' #Patch Set 5 : Use a different formatter string #Messages
Total messages: 30 (23 generated)
The CQ bit was checked by ahaas@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2483053005/diff/1/src/wasm/wasm-module.cc File src/wasm/wasm-module.cc (right): https://codereview.chromium.org/2483053005/diff/1/src/wasm/wasm-module.cc#new... src/wasm/wasm-module.cc:1152: } else if (!CheckDataSegmentsAreEmpty()) { Why not just call LoadDataSegments(nullptr, 0)?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by ahaas@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2483053005/diff/1/src/wasm/wasm-module.cc File src/wasm/wasm-module.cc (right): https://codereview.chromium.org/2483053005/diff/1/src/wasm/wasm-module.cc#new... src/wasm/wasm-module.cc:1152: } else if (!CheckDataSegmentsAreEmpty()) { On 2016/11/09 at 17:20:31, titzer wrote: > Why not just call LoadDataSegments(nullptr, 0)? Done. I also fixed another issue with non-empty memory an out-of-bounds empty segments.
On 2016/11/10 10:26:25, ahaas wrote: > https://codereview.chromium.org/2483053005/diff/1/src/wasm/wasm-module.cc > File src/wasm/wasm-module.cc (right): > > https://codereview.chromium.org/2483053005/diff/1/src/wasm/wasm-module.cc#new... > src/wasm/wasm-module.cc:1152: } else if (!CheckDataSegmentsAreEmpty()) { > On 2016/11/09 at 17:20:31, titzer wrote: > > Why not just call LoadDataSegments(nullptr, 0)? > > Done. I also fixed another issue with non-empty memory an out-of-bounds empty > segments. lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...) v8_win_nosnap_shared_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
The CQ bit was checked by ahaas@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...) v8_win_nosnap_shared_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
The CQ bit was checked by ahaas@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...) v8_win_nosnap_shared_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
The CQ bit was checked by ahaas@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by ahaas@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org Link to the patchset: https://codereview.chromium.org/2483053005/#ps80001 (title: "Use a different formatter string")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== [wasm] Check data segments for zero-sized memory. According to the spec data segments are allowed even if the memory size is zero. However, if one of the data segments has a length greater than 0, then module instantiation should fail. I also changed the exception type in LoadDataSegments to TypeError, because that's the exception type for all exceptions which can happen during instantiation. R=titzer@chromium.org, rossberg@chromium.org TEST=cctest/test-run-wasm-module/EmptyMemoryEmptyDataSegment, cctest/test-run-wasm-module/EmptyMemoryNonEmptyDataSegment ========== to ========== [wasm] Check data segments for zero-sized memory. According to the spec data segments are allowed even if the memory size is zero. However, if one of the data segments has a length greater than 0, then module instantiation should fail. I also changed the exception type in LoadDataSegments to TypeError, because that's the exception type for all exceptions which can happen during instantiation. R=titzer@chromium.org, rossberg@chromium.org TEST=cctest/test-run-wasm-module/EmptyMemoryEmptyDataSegment, cctest/test-run-wasm-module/EmptyMemoryNonEmptyDataSegment Committed: https://crrev.com/e3c7324a2e0bf7317fd12302a02ed4bddc856a8c Cr-Commit-Position: refs/heads/master@{#40922} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/e3c7324a2e0bf7317fd12302a02ed4bddc856a8c Cr-Commit-Position: refs/heads/master@{#40922} |
