epastor added inline comments.
================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1821
+ getParser().parsePrimaryExpr(Val, End))
+ return Error(Start, "unexpected token!");
+ } else if (ParseIntelInlineAsmIdentifier(Val, ID, Info, false, End, true)) {
----------------
rnk wrote:
> epastor wrote:
> > epastor wrote:
> > > rnk wrote:
> > > > Please test this corner case, I imagine it looks like:
> > > > mov eax, offset 3
> > > Interesting. This corner case didn't trigger in that scenario; we get an
> > > "expected identifier" error message with good source location, followed
> > > by another error "use of undeclared label '3'" in debug builds... and in
> > > release builds, we instead get a crash. On tracing the crash, it's a
> > > AsmStrRewrite applying to a SMLoc not coming from the same string...
> > >
> > > As near as I can tell, the issue is that we end up trying to parse "3" as
> > > a not-yet-declared label, as such expand it to
> > > `__MSASMLABEL_.${:uid}__3`, and then end up in a bad state because the
> > > operand rewrite is applying to the expanded symbol... which isn't in the
> > > same AsmString. If you use an actual undeclared label, you hit the same
> > > crash in release builds.
> > >
> > > This is going to take some work; I'll get back to it in a day or two.
> > Fixed; we now get the same errors in this scenario as we do in the current
> > LLVM trunk, reporting "expected identifier" and "use of undeclared label
> > '3'".
> >
> > On the other hand, I'm still working on finding a scenario that DOES
> > trigger this corner case.
> I see. Well, it sounds like it was a useful test. :)
Very useful indeed!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71436/new/
https://reviews.llvm.org/D71436
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
