LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 902 - asmparser doesn't detect errors soon enough
Summary: asmparser doesn't detect errors soon enough
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: LLVM assembly language parser (show other bugs)
Version: 1.0
Hardware: All All
: P normal
Assignee: Unassigned LLVM Bugs
URL:
Keywords: crash-on-invalid, regression
Depends on:
Blocks:
 
Reported: 2006-09-08 01:41 PDT by Chris Lattner
Modified: 2010-02-22 12:42 PST (History)
2 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Lattner 2006-09-08 01:41:49 PDT
This input crashes the asmparser. This is a regression from the EH removal patches.

void %test() {
        add int 1, 2.0
        ret void
}

Perhaps we need a sj/lj solution for the asmparser?

-Chris
Comment 1 Reid Spencer 2006-09-28 01:50:07 PDT
This seems to be a logic error in the AsmParser. The affected code is this
production:

InstVal : ArithmeticOps Types ValueRef ',' ValueRef {
    if (!(*$2)->isInteger() && !(*$2)->isFloatingPoint() &&
        !isa<PackedType>((*$2).get()))
      GEN_ERROR(
        "Arithmetic operator requires integer, FP, or packed operands!");
    if (isa<PackedType>((*$2).get()) && $1 == Instruction::Rem)
      GEN_ERROR("Rem not supported on packed types!");
***    $$ = BinaryOperator::create($1, getVal(*$2, $3), getVal(*$2, $5));
    if ($$ == 0)
      GEN_ERROR("binary operator returned null!");
    delete $2;
    CHECK_FOR_ERROR
  }

For some reason the second getVal call is ending up not liking the IntTy first
argument when it hits this assertion:

assert(Ty == Type::DoubleTy)

llvm::ConstantFP::get(const llvm::Type*, double)

In this case, Ty == Type::IntTy.

That's all the analysis I have time for right now.
Comment 2 Chris Lattner 2006-09-28 12:46:37 PDT
The reason this crashes is as follows:

This line:
BinaryOperator::create($1, getVal(*$2, $3), getVal(*$2, $5))

calls:  getVal("int", "1")
which is fine.

Then it calls:  getVal("int", "2.0")

Which calls GenerateError on line 319:

    if (!ConstantFP::isValueValidForType(Ty, D.ConstPoolFP))
      GenerateError("FP constant invalid for type!!");

However, GenerateError returns, which then falls into the next line:
    return ConstantFP::get(Ty, D.ConstPoolFP);
... which aborts.


GenerateError should not return.

-Chris
Comment 3 Reid Spencer 2006-09-28 13:17:15 PDT
Yeah, I think I see the problem. YYERROR can't be used inside getVal because it
is basically a goto statement which can only be used in the body of a
production. Consequently GenerateError is called directly instead of using the
GEN_ERROR macro. But, nothing checks for the error after this. Should be fairly
easy to fix. I'll try to take a look at this today and ensure the error gets
triggered in this and other similar cases.
Comment 4 Reid Spencer 2006-09-28 14:31:59 PDT
Fixed with this patch:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060925/038179.html

Errors are generated with the YYERROR macro which can only be called from
a production (inside yyparse) because of the goto statement in the macro.
This lead to several situations where GEN_ERROR was not called but
GenerateError was used instead (because it doesn't use YYERROR). However,
in such situations, catching the error much later (e.g. at the end of
the production) is not sufficient because LLVM can assert on invalid data
before the end of the production is reached. The solution is to ensure that
the CHECK_FOR_ERROR macro (which invokes YYERROR if there's an error) is
used as soon as possible after a call to GenerateError has been made.