[wasm] Add test for detailed stack trace

[wasm] Add test for detailed stack trace

This cctest triggers a detailed stack trace containing WASM frames.

R=jfb@chromium.org, mstarzinger@chromium.org, titzer@chromium.org
BUG=

Committed: https://crrev.com/5c1d28770b64d45826175e77e06d0e57377822c2
Cr-Commit-Position: refs/heads/master@{#35624}

[Michael Starzinger, 2016-04-11 14:46:44]

LGTM.

https://codereview.chromium.org/1875083002/diff/40001/test/cctest/wasm/test-w...
File test/cctest/wasm/test-wasm-stack.cc (right):

https://codereview.chromium.org/1875083002/diff/40001/test/cctest/wasm/test-w...
test/cctest/wasm/test-wasm-stack.cc:1: // Copyright 2015 the V8 project authors.
All rights reserved.
nit: s/2015/2016/ here.

[Clemens Hammacher, 2016-04-11 15:03:16]

https://codereview.chromium.org/1875083002/diff/40001/test/cctest/wasm/test-w...
File test/cctest/wasm/test-wasm-stack.cc (right):

https://codereview.chromium.org/1875083002/diff/40001/test/cctest/wasm/test-w...
test/cctest/wasm/test-wasm-stack.cc:1: // Copyright 2015 the V8 project authors.
All rights reserved.
On 2016/04/11 at 14:46:44, Michael Starzinger wrote:
> nit: s/2015/2016/ here.

Done.

[JF, 2016-04-11 18:12:00]

https://codereview.chromium.org/1875083002/diff/100001/test/cctest/wasm/test-...
File test/cctest/wasm/test-wasm-stack.cc (right):

https://codereview.chromium.org/1875083002/diff/100001/test/cctest/wasm/test-...
test/cctest/wasm/test-wasm-stack.cc:20: namespace {  // anonymous
The rest of the codebecase doesn't comment on anonymous namespace.

https://codereview.chromium.org/1875083002/diff/100001/test/cctest/wasm/test-...
test/cctest/wasm/test-wasm-stack.cc:29: } while (0)
This is better because it prevents re-evaluating the macro arguments:

#define CHECK_CSTREQ(exp, found)                                           \
  do {                                                                     \
    const char *exp_ = (exp);                                              \
    const char *found_ = (found);                                          \
    if (V8_UNLIKELY(strcmp(exp_, found_) != 0)) {                          \
      V8_Fatal(__FILE__, __LINE__,                                         \
               "Check failed: (%s) != (%s) ('%s' vs '%s').", #exp, #found, \
               exp_, found_);                                              \
    }                                                                      \
  } while (0)

https://codereview.chromium.org/1875083002/diff/100001/test/cctest/wasm/test-...
test/cctest/wasm/test-wasm-stack.cc:35: USE(frame);
This isn't necessary? frame is used below.

https://codereview.chromium.org/1875083002/diff/100001/test/cctest/wasm/test-...
test/cctest/wasm/test-wasm-stack.cc:48: int (&lineNumbers)[N]) {
Passing in std::vector would be cleaner than templating on size. You could
create a struct { const char *funname; int lineno; } and pass that in.

[Clemens Hammacher, 2016-04-12 08:30:48]

https://codereview.chromium.org/1875083002/diff/100001/test/cctest/wasm/test-...
File test/cctest/wasm/test-wasm-stack.cc (right):

https://codereview.chromium.org/1875083002/diff/100001/test/cctest/wasm/test-...
test/cctest/wasm/test-wasm-stack.cc:20: namespace {  // anonymous
On 2016/04/11 at 18:12:00, JF wrote:
> The rest of the codebecase doesn't comment on anonymous namespace.

Done.

https://codereview.chromium.org/1875083002/diff/100001/test/cctest/wasm/test-...
test/cctest/wasm/test-wasm-stack.cc:29: } while (0)
On 2016/04/11 at 18:12:00, JF wrote:
> This is better because it prevents re-evaluating the macro arguments:
> 
> #define CHECK_CSTREQ(exp, found)                                           \
>   do {                                                                     \
>     const char *exp_ = (exp);                                              \
>     const char *found_ = (found);                                          \
>     if (V8_UNLIKELY(strcmp(exp_, found_) != 0)) {                          \
>       V8_Fatal(__FILE__, __LINE__,                                         \
>                "Check failed: (%s) != (%s) ('%s' vs '%s').", #exp, #found, \
>                exp_, found_);                                              \
>     }                                                                      \
>   } while (0)

Done.

https://codereview.chromium.org/1875083002/diff/100001/test/cctest/wasm/test-...
test/cctest/wasm/test-wasm-stack.cc:35: USE(frame);
On 2016/04/11 at 18:12:00, JF wrote:
> This isn't necessary? frame is used below.

Right, I removed it.

https://codereview.chromium.org/1875083002/diff/100001/test/cctest/wasm/test-...
test/cctest/wasm/test-wasm-stack.cc:48: int (&lineNumbers)[N]) {
On 2016/04/11 at 18:12:00, JF wrote:
> Passing in std::vector would be cleaner than templating on size. You could
create a struct { const char *funname; int lineno; } and pass that in.

Not all buildbots can create std::vector from initializer list, so using
std::vector would blow up the code on each caller. Same problem for Vector.
(edit: I just see that initializer lists are banned anyway:
https://chromium-cpp.appspot.com/)

I updated the patch to use a struct tough. That's indeed much nicer.
Alternatively, I could add a new Vector constructor from an array reference,
like that:

  template <int N>
  Vector(T (&arr)[N]) : start_(arr), length_(N) {}

I would like that, but I did not want to touch such a central data structure ;)

[JF, 2016-04-12 18:18:52]

lgtm

[titzer, 2016-04-18 16:23:55]

On 2016/04/12 18:18:52, JF wrote:
> lgtm

lgtm

[Clemens Hammacher, 2016-04-19 08:40:11]

The CQ bit was checked by clemensh@chromium.org

[Clemens Hammacher, 2016-04-19 08:40:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/1875083002/#ps120001 (title: "address
JF's comments")

[commit-bot: I haz the power, 2016-04-19 08:40:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-19 08:40:20]

This CL has an open dependency (Issue 1878573003 Patch 20001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[Clemens Hammacher, 2016-04-19 12:55:13]

The CQ bit was checked by clemensh@chromium.org

[commit-bot: I haz the power, 2016-04-19 12:55:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1875083002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1875083002/120001

[commit-bot: I haz the power, 2016-04-19 14:20:05]

Committed patchset #7 (id:120001)

[nodir1, 2016-04-22 18:42:54]

Description was changed from

==========
[wasm] Add test for detailed stack trace

This cctest triggers a detailed stack trace containing WASM frames.

R=jfb@chromium.org, mstarzinger@chromium.org, titzer@chromium.org
BUG=
==========

to

==========
[wasm] Add test for detailed stack trace

This cctest triggers a detailed stack trace containing WASM frames.

R=jfb@chromium.org, mstarzinger@chromium.org, titzer@chromium.org
BUG=

Committed: https://crrev.com/5c1d28770b64d45826175e77e06d0e57377822c2
Cr-Commit-Position: refs/heads/master@{#35624}
==========

[nodir1, 2016-04-22 18:42:55]

Patchset 7 (id:??) landed as
https://crrev.com/5c1d28770b64d45826175e77e06d0e57377822c2
Cr-Commit-Position: refs/heads/master@{#35624}