[llvm] r176225 - Fix a problem in alias analysis. It is about the misinterpretation of "Object".

[Bill Wendling]

On Feb 27, 2013, at 4:24 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:

> Author: shuxin_yang
> Date: Wed Feb 27 18:24:45 2013
> New Revision: 176225
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=176225&view=rev
> Log:
> Fix a problem in alias analysis. It is about the misinterpretation of "Object".
> 
Even though you put the explanation in the code, you should also put the explanation of the problem in your commit message. Comments in code can be changed or deleted and aren't good for doing archeological research via the VCS.

-bw

> This problem is exposed by r171325 which is already reverted. It is rather
> hard to fabricate a testing case without it.
> 
> r171325 should *NOT* be resurrected as it has a potential problem although 
> this problem dosen't directly contribute to PR14988.
> 
> The bug is tracked by:
>  - rdar://13063553, and
>  - http://llvm.org/bugs/show_bug.cgi?id=14988
> 
> Thank Arnold for coming up a better solution to this problem. After
> comparing this solution and my original proposal, I decided to ditch mine.
> 
> Modified:
>    llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp
> 
> Modified: llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp?rev=176225&r1=176224&r2=176225&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp (original)
> +++ llvm/trunk/lib/Analysis/BasicAliasAnalysis.cpp Wed Feb 27 18:24:45 2013
> @@ -98,6 +98,35 @@ static uint64_t getObjectSize(const Valu
> static bool isObjectSmallerThan(const Value *V, uint64_t Size,
>                                 const DataLayout &TD,
>                                 const TargetLibraryInfo &TLI) {
> +  // Note that the meanings of the "object" are slightly different in the
> +  // following contexts:
> +  //    c1: llvm::getObjectSize()
> +  //    c2: llvm.objectsize() intrinsic
> +  //    c3: isObjectSmallerThan()
> +  // c1 and c2 share the same meaning; however, the meaning of "object" in c3
> +  // refers to the "entire object".
> +  //
> +  //  Consider this example:
> +  //     char *p = (char*)malloc(100)
> +  //     char *q = p+80;
> +  //
> +  //  In the context of c1 and c2, the "object" pointed by q refers to the
> +  // stretch of memory of q[0:19]. So, getObjectSize(q) should return 20.
> +  //
> +  //  However, in the context of c3, the "object" refers to the chunk of memory
> +  // being allocated. So, the "object" has 100 bytes, and q points to the middle
> +  // the "object". In case q is passed to isObjectSmallerThan() as the 1st
> +  // parameter, before the llvm::getObjectSize() is called to get the size of
> +  // entire object, we should:
> +  //    - either rewind the pointer q to the base-address of the object in
> +  //      question (in this case rewind to p), or
> +  //    - just give up. It is up to caller to make sure the pointer is pointing
> +  //      to the base address the object.
> +  // 
> +  // We go for 2nd option for simplicity.
> +  if (!isIdentifiedObject(V))
> +    return false;
> +
>   // This function needs to use the aligned object size because we allow
>   // reads a bit past the end given sufficient alignment.
>   uint64_t ObjectSize = getObjectSize(V, TD, TLI, /*RoundToAlign*/true);
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits