[llvm-commits] [llvm] r128140 - in /llvm/trunk: lib/Analysis/BasicAliasAnalysis.cpp test/Analysis/BasicAA/non-escaping-local-object.ll

[Frits van Bommel]

On Wed, Mar 23, 2011 at 3:19 AM, Anders Carlsson <andersca at mac.com> wrote:
> Log:
> A global variable with internal linkage where all uses are in one function and whose address is never taken is a non-escaping local object and can't alias anything else.

>  /// isNonEscapingLocalObject - Return true if the pointer is to a function-local
>  /// object that never escapes from the function.
>  static bool isNonEscapingLocalObject(const Value *V) {
> @@ -79,6 +102,16 @@
>         return true;
>       return !PointerMayBeCaptured(V, false, /*StoreCaptures=*/true);
>     }
> +
> +  // If this is an internal global variable that's only used in this function,
> +  // check if it escapes the function.
> +  if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(V)) {
> +    if (GV->hasInternalLinkage() && areAllUsesInOneFunction(GV)) {
> +      return !PointerMayBeCaptured(V, /*ReturnCaptures=*/true,
> +                                   /*StoreCaptures=*/true);

Looking at the places where this function is used, I see it being used
for things like checking whether a function call might modify this
object. Note that for internal globals used in one function, it isn't
safe to assume that simply because they don't escape they can't be
modified by calls -- a call might recurse, allowing another instance
of this function to modify it.

For instance:

int foo(int x) {
  static int counter = 0;
  counter++;
  if (bar(x))
    foo(x - 1);
  return counter;
}

Even though "counter" is an internal global used only in foo(), calls
can still modify it without it needing to escape first -- it
essentially comes pre-escaped, so isNonEscapingLocalObject() shouldn't
return true for it.
Note that the recursion could be through an arbitrarily long call
chain (or e.g. indirectly through function pointers) so this case
isn't always easy to detect.

It might be better to either:
a) remove this code here and create a new function that returns true
if the argument is a non-escaping local object or an internal global
used in a single function, then inspect all uses of
isNonEscapingLocalObject() to see whether they should be calling that
new function instead, or
b) add a parameter to isNonEscapingLocalObject() to flag whether this
is a safe option. (Preferably defaulting to false), or
c) simply revert this patch.