Skip to content

Conversation

@ofrobots
Copy link
Contributor

This reverts commit 6fff47f as it is causing
issues in upstream: https://codereview.chromium.org/1390923004/

Original PR: #3165.

R=@bnoordhuis, @targos

@targos
Copy link
Member

LGTM.

The Chromium bug report is not accessible?

@targostargos added the v8 engine Issues and PRs related to the V8 dependency. label Oct 7, 2015
@ofrobots
Copy link
ContributorAuthor

Basically it has been causing crash reports on chrome canary. Access is probably restricted because of the existence of the crash report data. Here's the relevant comment:

The sequence of calls to NameBuffer in CodeEventLogger::CodeCreateEvent is:

  • AppendString(String::cast(source))
  • AppendByte(':')
  • AppendInt(line)

After the above patch, the utf16buffer used within AppendString() is large enough that copying its contents over to the utf8buffer can fill up the latter. AppendByte() properly checks for that and returns. AppendInt() calls SNPrintF() with a zero-length buffer (because there are zero remaining bytes in utf8buffer), which makes vsnprintf_s() rather unhappy: it calls the InvalidParameter handler which triggers a crash.

This reverts commit 6fff47f as it is causing issues in upstream: https://codereview.chromium.org/1390923004/ PR-URL: nodejs#3237 Reviewed-By: targos - Michaël Zasso <[email protected]>
ofrobots added a commit that referenced this pull request Oct 7, 2015
This reverts commit 6fff47f as it is causing issues in upstream: https://codereview.chromium.org/1390923004/ PR-URL: #3237 Reviewed-By: targos - Michaël Zasso <[email protected]>
@ofrobots
Copy link
ContributorAuthor

@bnoordhuis
Copy link
Member

LGTM

@targostargos closed this Oct 7, 2015
@targos
Copy link
Member

landed in a334ddc

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@ofrobots@targos@bnoordhuis@rvagg