This is an archive of the discontinued LLVM Phabricator instance.

Fix for PR14269: Clang crashes when a bit field is used as inline assembler input / output with memory constraint
ClosedPublic

Authored by andreybokhanko on May 28 2015, 5:18 AM.

Details

Summary

Fix for PR14269 (https://llvm.org/bugs/show_bug.cgi?id=14269) -- clang simply crashes when a bit field is used as inline assembler input / output with memory constraint.

One generally can't get address of a bit field, so the general solution is to error on such cases. GCC does the same.

Diff Detail

Repository
rL LLVM

Event Timeline

andreybokhanko retitled this revision from to Fix for PR14269: Clang crashes when a bit field is used as inline assembler input / output with memory constraint.
andreybokhanko updated this object.
andreybokhanko edited the test plan for this revision. (Show Details)
andreybokhanko added a subscriber: Unknown Object (MLST).
mcrosier edited edge metadata.Jun 4 2015, 6:42 AM

I think it's acceptable to reference a PR# in the test case, but in general we don't include these in the source code unless there's an associated FIXME. Please remove the PR#s from SemaStmtAsm.cpp. Otherwise, seems reasonable to me.

andreybokhanko edited edge metadata.

Chad, thanks for the review. I removed mentioning of PRs; patch updated.

echristo accepted this revision.Jun 4 2015, 5:21 PM
echristo edited edge metadata.

One inline comment and then LGTM.

Thanks!

-eric

lib/Sema/SemaStmtAsm.cpp
241 ↗(On Diff #27125)

"Bit field" -> "Bitfield" please.

This revision is now accepted and ready to land.Jun 4 2015, 5:21 PM
andreybokhanko added a reviewer: amusman.

Eric, thanks!

Patch updated: "bit field" changed to "bitfield" in comments; "bit field" changed to "bit-field" in messages, as "bit-field" seems to be the prevailing spelling in other messages.

I don't have commit access; Alexander [Musman], could you, please, commit the patch?

Andrey

This revision was automatically updated to reflect the committed changes.
rsmith added a subscriber: rsmith.Jun 8 2015, 3:13 PM
rsmith added inline comments.
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
158

What about other weird kinds of lvalues, like the result of __real / __imag, vector indexing, and global register variables? Those have the same problem; CGStmt.cpp blindly calls LValue::getAddress without checking for those cases.

Testcase:

typedef __attribute__((vector_size(16))) int vi4;
void test(vi4 v) {
  __asm__("" : "=rm"(v[2]));
}

Richard, I will take a look.

Yours,
Andrey