This is an archive of the discontinued LLVM Phabricator instance.

[Format] Fix crash when hitting eof while lexing JS template string
ClosedPublic

Authored by sammccall on Oct 6 2022, 4:12 AM.

Details

Summary

Different loop termination conditions resulted in confusion of whether
*Offset was intended to be inside or outside the token.
This ultimately led to constructing an out-of-range SourceLocation.

Fix by making Offset consistently point *after* the token.

Diff Detail

Event Timeline

sammccall created this revision.Oct 6 2022, 4:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 6 2022, 4:12 AM
sammccall requested review of this revision.Oct 6 2022, 4:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 6 2022, 4:12 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
kadircet accepted this revision.Oct 6 2022, 4:25 AM

thanks!

clang/lib/Format/FormatTokenLexer.cpp
767

it's a separate concern, but feels like this could also trigger some crashes (e.g. an input like "`\")

772

nit: Offset += 2; instead?

This revision is now accepted and ready to land.Oct 6 2022, 4:25 AM
sammccall marked an inline comment as done.Oct 6 2022, 8:00 AM
sammccall added inline comments.
clang/lib/Format/FormatTokenLexer.cpp
767

Good catch. I think you're right, but:

  • I'm not sure
  • that example doesn't crash under asan, and I didn't find a crashing one
  • that pattern occurs a bunch more times in this file
  • I really want to have an isolated minimal fix + test as this bug is causing an outage
  • I don't really have spare time now to dive deeper into this
This revision was landed with ongoing or failed builds.Oct 6 2022, 8:01 AM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Oct 6 2022, 8:14 AM

This breaks tests: http://45.33.8.238/linux/88341/step_7.txt

Please take a look and revert for now if it takes a while to fix.

gulfem added a subscriber: gulfem.Oct 6 2022, 9:36 AM

We are seeing the same error reported above:

FormatTests: clang/lib/Lex/Lexer.cpp:1151: SourceLocation clang::Lexer::getSourceLocation(const char *, unsigned int) const: Assertion `Loc >= BufferStart && Loc <= BufferEnd && "Location out of range for this buffer!"' failed.

https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8801040768684259873/overview

Fixed by 4b53c00173163774d32125fbcae283a46a9a4b19 I think.

(@kadircet suggested a possible second crash, I couldn't get it to crash so included the testcase with this patch. Turns out it does crash...)

It fixed the test error that we are seeing, thanks!

owenpan added a project: Restricted Project.Mar 29 2023, 5:35 PM