This is an archive of the discontinued LLVM Phabricator instance.

Use deref facts derived from minimum object size of allocations
ClosedPublic

Authored by reames on Oct 28 2020, 2:02 PM.

Details

Summary

This change should be fairly straight forward. If we've reached a call, check to see if we can tell the result is dereferenceable from information about the minimum object size returned by the call.

To control compile time impact, I'm only adding the call for base facts in the routine. getObjectSize can also do recursive reasoning, and we don't want that general capability here.

As a follow up patch (without separate review), I will plumb through the missing TLI parameter. That will have the effect of extending this to known libcalls - malloc, new, and the like - whereas currently this only covers calls with the explicit allocsize attribute.

Diff Detail

Event Timeline

reames created this revision.Oct 28 2020, 2:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 28 2020, 2:02 PM
reames requested review of this revision.Oct 28 2020, 2:02 PM
reames planned changes to this revision.Oct 28 2020, 2:43 PM

Realized this is not correct. I failed to account for the case where the object has been conditionally deallocated along the path leading to the location of interest. Here's a counter example:

o = new T[];
if (c) delete o;
loop {

if (!c) o[i];

}

It's not legal to hoist o[i] above the !c test.

Use the nofree argument/function attribute to exclude this case.

reames updated this revision to Diff 304912.Nov 12 2020, 11:37 AM

Update to be conservative about whether allocation is a point in time fact or a globally established fact. Technically, the attribute we need to clarify is actually allocsize itself, but deref is the one we have demonstrated progress on, so describe the problem in terms of that.

nikic added a comment.Nov 23 2020, 3:23 AM

The logic here looks sound to me. Structurally, I would suggest integrating this with the getPointerDereferenceableBytes() check above, as you are currently duplicating the logic around it. Basically extract the current getPointerDereferenceableBytes() call into a static function/lambda, and add the handling for the allocated object size in there, the remaining logic (including the non-null check and the alignment check) will be shared.

The logic here looks sound to me. Structurally, I would suggest integrating this with the getPointerDereferenceableBytes() check above, as you are currently duplicating the logic around it. Basically extract the current getPointerDereferenceableBytes() call into a static function/lambda, and add the handling for the allocated object size in there, the remaining logic (including the non-null check and the alignment check) will be shared.

Doing it this way will greatly increase the scope of the analysis done for object size. I would expect it to be compile time prohibitive. The restriction to only calls was tactical and intentional.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 3 2020, 3:01 PM
This revision was automatically updated to reflect the committed changes.