js1_8_5/extensions/reflect-parse.js is failing

RESOLVED FIXED in mozilla10

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

Other Branch
mozilla10
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
The location information is bogus. Valgrind finds uninitialized memory reads. I have a fix.
(Assignee)

Comment 1

6 years ago
Created attachment 568521 [details] [diff] [review]
v1
Assignee: general → jorendorff
Attachment #568521 - Flags: review?(luke)
(Assignee)

Comment 2

6 years ago
Here's what happened here.

struct TokenPos previously had no constructors declared. I added this:
    TokenPos() {}

But there is a struct Token that includes a TokenPos, and a class TokenStream
that has a member `Token tokens[N];'. TokenStream's constructor did this:
      : tokens()

This apparently had the effect of zeroing out all the members of each Token, though I have a hard time constructing a mental model of C++ in which that makes any sense... By adding the do-nothing constructor, I changed the meaning from "fill with zeroes" to "don't initialize".

We um... we probably shouldn't be depending on that, but for today let's just restore the status quo rather than charging forward unwarily.
See bug 588061. If only jsreflect.cpp needs zero-init, and only of the not-yet-got "first token", we should hack only that file, or find a better fix for bug 588061.

/be

Comment 4

6 years ago
(In reply to Jason Orendorff [:jorendorff] from comment #2)
> This apparently had the effect of zeroing out all the members of each Token,
> though I have a hard time constructing a mental model of C++ in which that
> makes any sense... By adding the do-nothing constructor, I changed the
> meaning from "fill with zeroes" to "don't initialize".

To wit, (8.5.10) "An object whose initializer is an empty set of parenthesis, i.e., (), shall be value-initialized."  8.5.7 then specifies value-initialization to be: if you don't have a user-provided constructor, your zero-initialize your members and, if you do, then you just call the user-provided constructor without zero-initialization.

Comment 5

6 years ago
Comment on attachment 568521 [details] [diff] [review]
v1

Oops, didn't see the r?.
Attachment #568521 - Flags: review?(luke) → review+
(Assignee)

Comment 6

6 years ago
Thanks, Luke.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9fa6410c1a3a

Comment 7

6 years ago
https://hg.mozilla.org/mozilla-central/rev/9fa6410c1a3a
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
(Assignee)

Updated

6 years ago
Duplicate of this bug: 696268
You need to log in before you can comment on or make changes to this bug.