This is an archive of the discontinued LLVM Phabricator instance.

[IPO/LowerTypesTest] Skip blockaddress when replacing uses
ClosedPublic

Authored by davide on Nov 6 2017, 1:30 PM.

Details

Summary

Blockaddresses refer to the function itself, therefore replacing them would cause an assertion in doRAUW.
Fixes https://bugs.llvm.org/show_bug.cgi?id=35201
This was found when trying CFI on a proprietary kernel by Dmitry Mikulin.

Diff Detail

Repository
rL LLVM

Event Timeline

davide created this revision.Nov 6 2017, 1:30 PM
filcab edited edge metadata.Nov 6 2017, 2:38 PM

Looks good. I’ll ask you to wait a bit and see if @pcc has any objection, though.

Thank you,

Filipe

pcc added inline comments.Nov 6 2017, 3:06 PM
lib/IR/Value.cpp
464 ↗(On Diff #121777)

Can this line just be
if (isa<BlockAddress>(U.getUser()))
?

test/Transforms/LowerTypeTests/blockaddress.ll
16 ↗(On Diff #121777)

I think this test case is more complicated than it needs to be. This seems to be enough to trigger the bug:

target triple = "x86_64-unknown-linux"

define void @f1() {
entry:
  %0 = call i1 @llvm.type.test(i8* bitcast (i8* ()* @f2 to i8*), metadata !"_ZTSFvP3bioE")
  ret void
}

declare i1 @llvm.type.test(i8*, metadata)

define i8* @f2() !type !5 {
  br label %b

b:
  ret i8* blockaddress(@f2, %b)
}

!5 = !{i64 0, !"_ZTSFvP3bioE"}

The only important part of the output is the body of f2, so I would only have CHECK lines for that.

davide updated this revision to Diff 121808.Nov 6 2017, 3:57 PM

Address Peter's comments.

lib/IR/Value.cpp
464 ↗(On Diff #121777)

yes.

test/Transforms/LowerTypeTests/blockaddress.ll
16 ↗(On Diff #121777)

yes, thanks for reducing further.

pcc accepted this revision.Nov 6 2017, 4:07 PM

LGTM

This revision is now accepted and ready to land.Nov 6 2017, 4:07 PM
This revision was automatically updated to reflect the committed changes.