Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asmparser doesn't detect errors soon enough #1274

Closed
lattner opened this issue Sep 8, 2006 · 4 comments
Closed

asmparser doesn't detect errors soon enough #1274

lattner opened this issue Sep 8, 2006 · 4 comments

Comments

@lattner
Copy link
Collaborator

lattner commented Sep 8, 2006

Bugzilla Link 902
Resolution FIXED
Resolved on Feb 22, 2010 12:42
Version 1.0
OS All

Extended Description

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

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 28, 2006

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(($2).get()))
GEN_ERROR(
"Arithmetic operator requires integer, FP, or packed operands!");
if (isa((
$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.

@lattner
Copy link
Collaborator Author

lattner commented Sep 28, 2006

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

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 28, 2006

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.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 28, 2006

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.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants