Page MenuHomePhabricator

Fix assertion in inline assembler IR gen

Authored by amusman on Sep 18 2015, 2:03 AM.



Several inputs may not refer to one output constraint in inline assembler insertions, clang fails on assertion on such test case here:

llvm/lib/IR/InlineAsm.cpp:46: llvm::InlineAsm::InlineAsm(llvm::FunctionType*, const string&, const string&, bool, bool, llvm::InlineAsm::AsmDialect): Assertion `Verify(getFunctionType(), constraints) && "Function type not legal for constraints!"' failed.

I suggest to check this case in Sema.


Diff Detail

Event Timeline

amusman updated this revision to Diff 35068.Sep 18 2015, 2:03 AM
amusman retitled this revision from to Fix assertion in inline assembler IR gen.
amusman updated this object.
amusman added reviewers: rsmith, aaron.ballman, ABataev.
amusman added a subscriber: cfe-commits.
aaron.ballman edited edge metadata.Sep 18 2015, 2:19 PM

Generally LGTM, with one question about guarding against overflow.


Is it possible for InputConstraintsInfos.size() to be greater than OutputConstrainInfos.size()? Basically, I'm worried about buffer overflow here with TiedTo.

amusman added inline comments.Sep 21 2015, 3:38 AM

The check above (which emits error err_asm_invalid_input_constraint) seems to capture this case so I always get correct TiedTo value here.
What if I add an assertion that TiedTo>=0 && TiedTo <InputMatchedToOutput.size()?

amusman updated this revision to Diff 35222.Sep 21 2015, 3:42 AM
amusman edited edge metadata.

Added an assertion for TiedTo value.

aaron.ballman accepted this revision.Sep 21 2015, 5:53 AM
aaron.ballman edited edge metadata.

With one minor change, LGTM! Thank you for working on this.



No need for the TiedTo >= 0 since it's an unsigned value anyway. Our usual way of writing this sort of assert is:

assert(TiedTo < InputMatchedToOutput.size() && "TiedTo value out of range");
This revision is now accepted and ready to land.Sep 21 2015, 5:53 AM
amusman closed this revision.Sep 22 2015, 12:38 AM

Thanks, I've commited the changes in revision 248158 (but forgot to add link in svn commit message).