rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm, thanks!
================
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)) {
----------------
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. :)
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
