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)) {
----------------
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.
================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1825
+ } else if (Info.isKind(InlineAsmIdentifierInfo::IK_EnumVal)) {
+ return Error(Start, "offset operator cannot yet handle constants");
+ }
----------------
epastor wrote:
> rnk wrote:
> > Please add a test for this corner case, I'm curious to see if the error
> > reporting really works.
> This error reporting doesn't work, in fact. We instead get "cannot take the
> address of an rvalue of type 'int'", with bad source location. Will
> investigate why we end up there.
Turns out Sema wasn't successfully recognizing inline-assembly references to
enum values, so we were processing them through as labels, and only recognizing
the problem late in processing. Fixed now, with accurate error reporting; test
added.
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
