Improve CodeStubAssembler assert functionality

[Reland]: Improve CodeStubAssembler assert functionality

Introduce CSA_ASSERT macro that outputs a message, file name and line number to
console before calling DebugBreak.

Committed: https://crrev.com/23836e9c14f3df9b675fe02e2c23bb11e728b83d
Committed: https://crrev.com/2f9526523854b112b70af576b98b413914f0d0e3
Cr-Original-Commit-Position: refs/heads/master@{#40307}
Cr-Commit-Position: refs/heads/master@{#40322}

[danno, 2016-10-14 09:07:55]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 09:08:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 09:13:50]

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

[commit-bot: I haz the power, 2016-10-14 09:13:51]

Dry run: Try jobs failed on following builders:
  v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/16116)

[danno, 2016-10-14 09:29:19]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 09:29:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danno, 2016-10-14 09:30:07]

danno@chromium.org changed reviewers:
+ epertoso@chromium.org, ishell@google.com

[danno, 2016-10-14 09:30:07]

PTAL

[epertoso, 2016-10-14 09:40:14]

https://codereview.chromium.org/2419433008/diff/20001/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2419433008/diff/20001/src/code-stub-assembler...
src/code-stub-assembler.cc:36: printf("%d\n", buffer.length());
Spurious printf.

https://codereview.chromium.org/2419433008/diff/20001/src/code-stub-assembler...
src/code-stub-assembler.cc:39: Comment(comment);
Just Coment("[ Assert: %s", message) should be sufficient.

https://codereview.chromium.org/2419433008/diff/20001/src/code-stub-assembler...
src/code-stub-assembler.cc:927: CSA_ASSERT(
Doesn't this always print out code-stub-assembler.cc:927?

[Igor Sheludko, 2016-10-14 09:48:59]

ishell@chromium.org changed reviewers:
+ ishell@chromium.org

[Igor Sheludko, 2016-10-14 09:48:59]

Nice! lgtm with suggestion:

https://codereview.chromium.org/2419433008/diff/20001/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2419433008/diff/20001/src/code-stub-assembler...
src/code-stub-assembler.cc:52: Runtime::kDebugPrint, SmiConstant(Smi::kZero),
You can probably call kGlobalPrint which will just print a string (but without
\n).

[commit-bot: I haz the power, 2016-10-14 09:58:56]

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

[commit-bot: I haz the power, 2016-10-14 09:58:56]

Dry run: This issue passed the CQ dry run.

[danno, 2016-10-14 10:51:22]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 10:51:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danno, 2016-10-14 10:53:25]

please take another look

https://codereview.chromium.org/2419433008/diff/20001/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2419433008/diff/20001/src/code-stub-assembler...
src/code-stub-assembler.cc:36: printf("%d\n", buffer.length());
On 2016/10/14 09:40:14, epertoso wrote:
> Spurious printf.

Done.

https://codereview.chromium.org/2419433008/diff/20001/src/code-stub-assembler...
src/code-stub-assembler.cc:39: Comment(comment);
On 2016/10/14 09:40:14, epertoso wrote:
> Just Coment("[ Assert: %s", message) should be sufficient.

Done.

https://codereview.chromium.org/2419433008/diff/20001/src/code-stub-assembler...
src/code-stub-assembler.cc:52: Runtime::kDebugPrint, SmiConstant(Smi::kZero),
On 2016/10/14 09:48:59, Igor Sheludko wrote:
> You can probably call kGlobalPrint which will just print a string (but without
> \n).

Done.

https://codereview.chromium.org/2419433008/diff/20001/src/code-stub-assembler...
src/code-stub-assembler.cc:927: CSA_ASSERT(
On 2016/10/14 09:40:14, epertoso wrote:
> Doesn't this always print out code-stub-assembler.cc:927?

Is that a problem? I think that's fine, it gives you a place to start.

[commit-bot: I haz the power, 2016-10-14 11:18:59]

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

[commit-bot: I haz the power, 2016-10-14 11:19:00]

Dry run: This issue passed the CQ dry run.

[danno, 2016-10-14 11:25:32]

The CQ bit was checked by danno@chromium.org

[danno, 2016-10-14 11:25:32]

The patchset sent to the CQ was uploaded after l-g-t-m from ishell@chromium.org
Link to the patchset: https://codereview.chromium.org/2419433008/#ps40001
(title: "Review feedback")

[commit-bot: I haz the power, 2016-10-14 11:25:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 11:28:20]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-14 11:28:56]

Description was changed from

==========
Improve CodeStubAssembler assert functionality

Introduce CSA_ASSERT macro that outputs a message, file name and line number to
console before calling DebugBreak.
==========

to

==========
Improve CodeStubAssembler assert functionality

Introduce CSA_ASSERT macro that outputs a message, file name and line number to
console before calling DebugBreak.

Committed: https://crrev.com/23836e9c14f3df9b675fe02e2c23bb11e728b83d
Cr-Commit-Position: refs/heads/master@{#40307}
==========

[commit-bot: I haz the power, 2016-10-14 11:28:56]

Patchset 3 (id:??) landed as
https://crrev.com/23836e9c14f3df9b675fe02e2c23bb11e728b83d
Cr-Commit-Position: refs/heads/master@{#40307}

[Michael Achenbach, 2016-10-14 12:32:03]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2414373002/ by machenbach@chromium.org.

The reason for reverting is: [Sheriff] Fails leak checker:
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20-%20debug/buil....

[jgruber, 2016-10-14 12:34:17]

jgruber@chromium.org changed reviewers:
+ jgruber@chromium.org

[jgruber, 2016-10-14 12:34:17]

https://codereview.chromium.org/2419433008/diff/40001/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2419433008/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:41: if (message != nullptr) {
Drive-by-comment: Allocate buffer in here to avoid allocation if message ==
nullptr? Should we use a ScopedVector to ensure it's freed? And I'm also a bit
surprised that there's no length argument for SNPrintF.

[danno, 2016-10-14 14:48:45]

Description was changed from

==========
Improve CodeStubAssembler assert functionality

Introduce CSA_ASSERT macro that outputs a message, file name and line number to
console before calling DebugBreak.

Committed: https://crrev.com/23836e9c14f3df9b675fe02e2c23bb11e728b83d
Cr-Commit-Position: refs/heads/master@{#40307}
==========

to

==========
[Reland]: Improve CodeStubAssembler assert functionality

Introduce CSA_ASSERT macro that outputs a message, file name and line number to
console before calling DebugBreak.

Committed: https://crrev.com/23836e9c14f3df9b675fe02e2c23bb11e728b83d
Cr-Commit-Position: refs/heads/master@{#40307}
==========

[danno, 2016-10-14 14:50:34]

Memory leak addressed

https://codereview.chromium.org/2419433008/diff/40001/src/code-stub-assembler.cc
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2419433008/diff/40001/src/code-stub-assembler...
src/code-stub-assembler.cc:41: if (message != nullptr) {
On 2016/10/14 12:34:17, jgruber wrote:
> Drive-by-comment: Allocate buffer in here to avoid allocation if message ==
> nullptr? Should we use a ScopedVector to ensure it's freed? And I'm also a bit
> surprised that there's no length argument for SNPrintF.

I misinterpreted the interface, problem is now solved. Vector carries around the
length, so SNPrintF asked the buffer how big it is.

[danno, 2016-10-14 14:50:58]

The CQ bit was checked by danno@chromium.org

[danno, 2016-10-14 14:50:59]

The patchset sent to the CQ was uploaded after l-g-t-m from ishell@chromium.org
Link to the patchset: https://codereview.chromium.org/2419433008/#ps60001
(title: "Fix memory leak")

[commit-bot: I haz the power, 2016-10-14 14:51:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 15:16:06]

Description was changed from

==========
[Reland]: Improve CodeStubAssembler assert functionality

Introduce CSA_ASSERT macro that outputs a message, file name and line number to
console before calling DebugBreak.

Committed: https://crrev.com/23836e9c14f3df9b675fe02e2c23bb11e728b83d
Cr-Commit-Position: refs/heads/master@{#40307}
==========

to

==========
[Reland]: Improve CodeStubAssembler assert functionality

Introduce CSA_ASSERT macro that outputs a message, file name and line number to
console before calling DebugBreak.

Committed: https://crrev.com/23836e9c14f3df9b675fe02e2c23bb11e728b83d
Cr-Commit-Position: refs/heads/master@{#40307}
==========

[commit-bot: I haz the power, 2016-10-14 15:16:07]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-14 15:16:24]

Description was changed from

==========
[Reland]: Improve CodeStubAssembler assert functionality

Introduce CSA_ASSERT macro that outputs a message, file name and line number to
console before calling DebugBreak.

Committed: https://crrev.com/23836e9c14f3df9b675fe02e2c23bb11e728b83d
Cr-Commit-Position: refs/heads/master@{#40307}
==========

to

==========
[Reland]: Improve CodeStubAssembler assert functionality

Introduce CSA_ASSERT macro that outputs a message, file name and line number to
console before calling DebugBreak.

Committed: https://crrev.com/23836e9c14f3df9b675fe02e2c23bb11e728b83d
Committed: https://crrev.com/2f9526523854b112b70af576b98b413914f0d0e3
Cr-Original-Commit-Position: refs/heads/master@{#40307}
Cr-Commit-Position: refs/heads/master@{#40322}
==========

[commit-bot: I haz the power, 2016-10-14 15:16:25]

Patchset 4 (id:??) landed as
https://crrev.com/2f9526523854b112b70af576b98b413914f0d0e3
Cr-Commit-Position: refs/heads/master@{#40322}