This is an archive of the discontinued LLVM Phabricator instance.

[Attributor] Check violation of returned position nonnull and noundef attribute in AAUndefinedBehavior
ClosedPublic

Authored by okura on Aug 3 2020, 8:10 PM.

Details

Summary

This patch is a follow up of D84733.
If a function has noundef attribute in returned position, instructions that return undef or poison value cause UB.

Diff Detail

Event Timeline

okura created this revision.Aug 3 2020, 8:10 PM
Herald added a project: Restricted Project. · View Herald Transcript
okura requested review of this revision.Aug 3 2020, 8:10 PM

Cool :)

There is a typo in your subject line [Attributor].

llvm/lib/Transforms/IPO/AttributorAttributes.cpp
2052

I don't think we need to simplify the value here. If it is not simplified, it should happen as part of AAReturned. I guess we can just not do it for now?

2057

Reading this makes me question if it should ever happen. I hope AAReturnedValues will not look at dead returns so they should not be part of the RetInsts set and if that set is empty this should never be called.

2074–2079
2082

Note: When the other patch lands we should check if we want to ask the AANoUndef here.

okura retitled this revision from [Attributor} Check violation of returned position nonnull and noundef attribute in AAUndefinedBehavior to [Attributor] Check violation of returned position nonnull and noundef attribute in AAUndefinedBehavior.Aug 6 2020, 9:30 AM
okura marked an inline comment as done.Aug 6 2020, 9:34 AM
okura added inline comments.
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
2057

I overlooked the fact that a dead return instruction is ignored in checkForAllReturnedValuesAndReturnInsts. I'll fix this comment.

okura updated this revision to Diff 283646.Aug 6 2020, 9:43 AM
  • fix comment
  • change the callback so that it does not simplify the value.
okura updated this revision to Diff 283786.Aug 6 2020, 6:24 PM
  • clarify that dead returned values are ignored in checkForReturnedValuesAndReturnedInsts in a comment.
This revision is now accepted and ready to land.Aug 6 2020, 7:20 PM