Closed Bug 1736552 Opened 7 months ago Closed 7 months ago

`ParserBase::setFunctionEndFromCurrentToken` and `TokenStreamSpecific<..>::advance` disagree about liveness

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
95 Branch
Tracking Status
firefox95 --- fixed

People

(Reporter: jseward, Assigned: arai)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached file Valgrind complainage

This is something picked up by Valgrind/Memcheck. It looks like an
inconsistency between the one-and-only assertion in
ParserBase::setFunctionEndFromCurrentToken and what's in
TokenStreamSpecific<..>::advance. The former has this:

  MOZ_ASSERT(anyChars.currentToken().type != TokenKind::Eof);

which, aside from the actual assertion, implies that "it's OK to look at
anyChars.currentToken().type right now. But V reports an uninitialised
value error on that line, and claims that the undefined value comes from
TokenStreamSpecific<..>::advance, specifically this:

  MOZ_MAKE_MEM_UNDEFINED(&cur->type, sizeof(cur->type));

That "paints" cur->type as containing undefined data. I'm assuming that
anyChars.currentToken().type overlaps the painted area (maybe exactly) and
that's why V subsequently complains.

In effect TokenStreamSpecific<..>::advance says "the type field is now
invalid" but the assertion in ParserBase::setFunctionEndFromCurrentToken
says "well, I don't care, I want to look at it anyway". Resolving the
inconsistency requires someone who understands the logic here, so I offer no
fix.

STR:

Configure/build on x86_64-linux with
CC="gcc" CXX="g++" ../src/configure --enable-debug --disable-optimize --enable-valgrind --disable-jemalloc

Then:

valgrind --track-origins=yes
./dist/bin/js --no-threads -f
/space/MOZ/MC-TALL/js/src/jit-test/lib/prologue.js
-e 'const platform='"'"'linux'"'"''
-e 'const libdir='"'"'/space/MOZ/MC-TALL/js/src/jit-test/lib/'"'"''
-e 'const scriptdir='"'"'/space/MOZ/MC-TALL/js/src/jit-test/tests/arrow-functions/'"'"''
--module-load-path /space/MOZ/MC-TALL/js/src/jit-test/modules/
-f
/space/MOZ/MC-TALL/js/src/jit-test/tests/arrow-functions/associativity-1.js

produces the attached complaint.

Thank you!

indeed, it shouldn't use type after skipping the inner lazy function, given that operation doesn't get any token,
but just jumps to the end of the function.

Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Severity: -- → S3
Priority: -- → P1
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/0379d2957094
Do not read Token.type field after TokenStreamSpecific::advance. r=tcampbell
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
You need to log in before you can comment on or make changes to this bug.