[LibFunc] "free" captures the pointer operand

[jdoerfert]

Summary

We used to mark the free operand as "nocapture", though, arguably, there
is no reason "free" could not put the pointer value in a "free-table",
thereby effectively "capturing" it.

[efriedma]

Is there some practical issue I'm missing?

there is no reason "free" could not put the pointer value in a "free-table"

Data structures which are internal to the malloc implementation are considered opaque; otherwise, we couldn't add any attribute markings to malloc.

[jdoerfert]

In D66439#1638284, @efriedma wrote:

Is there some practical issue I'm missing?

I should have added my motivation:

I want heap2stack transformation under different circumstances, one of which is:

• whenever all capturing uses of a malloc are in free calls, we can perform the heap2stack transformation (because no one else can know about the pointer).

Now this is a different approach then the one originally proposed by @hfinkel, which was more along the lines of:

• whenever all paths lead *must* lead a free of the pointer (so no unwinding, etc.) we can perform heap2stack

They should catch different cases but the first is easier (IMHO).

The problem now is that non-capturing uses can free the pointer as well.

there is no reason "free" could not put the pointer value in a "free-table"

Data structures which are internal to the malloc implementation are considered opaque; otherwise, we couldn't add any attribute markings to malloc.

Fair point. Though I'm unsure why we need the nocapture here anyway. I guess if it otherwise confuses the alias analysis somehow (which I'm not really sure of in reality) we should consider lifetime markers instead.

[efriedma]

I'd rather analyze whether the memory is valid separately from whether the pointer itself is captured, I think? Removing nocapture markings makes all the existing uses of capture analysis weaker.

[jdoerfert]

Thx @efriedma, We'll do it without this change

[jdoerfert]

We'll need "nofree" for call site arguments ;)

llvm/lib/Transforms/Utils/BuildLibCalls.cpp

case LibFunc_fopen:
     return Changed;
   case LibFunc_fdopen:
     Changed |= setDoesNotThrow(F);
     Changed |= setRetDoesNotAlias(F);
     Changed |= setDoesNotCapture(F, 1);
     Changed |= setOnlyReadsMemory(F, 1);
     return Changed;
   case LibFunc_feof:
-  case LibFunc_free:
   case LibFunc_fseek:
   case LibFunc_ftell:
   case LibFunc_fgetc:
   case LibFunc_fgetc_unlocked:
   case LibFunc_fseeko:
   case LibFunc_ftello:
   case LibFunc_fileno:
   case LibFunc_fflush:
   case LibFunc_fclose:
   case LibFunc_fsetpos:
   case LibFunc_flockfile:
   case LibFunc_funlockfile:
   case LibFunc_ftrylockfile:
     Changed |= setDoesNotThrow(F);
     Changed |= setDoesNotCapture(F, 0);
     return Changed;
+  case LibFunc_free:
+    // Free is nothrow but does capture the argument.
+    Changed |= setDoesNotThrow(F);
+    return Changed;
   case LibFunc_ferror:
     Changed |= setDoesNotThrow(F);
     Changed |= setDoesNotCapture(F, 0);
     Changed |= setOnlyReadsMemory(F);
     return Changed;
   case LibFunc_fputc:
   case LibFunc_fputc_unlocked:
   case LibFunc_fstat:

llvm/test/Transforms/InferFunctionAttrs/annotate.ll

 declare i32 @fputc(i32, %opaque*)
 
 ; CHECK: declare i32 @fputs(i8* nocapture readonly, %opaque* nocapture) [[G1]]
 declare i32 @fputs(i8*, %opaque*)
 
 ; CHECK: declare i64 @fread(i8* nocapture, i64, i64, %opaque* nocapture) [[G1]]
 declare i64 @fread(i8*, i64, i64, %opaque*)
 
-; CHECK: declare void @free(i8* nocapture) [[G3:#[0-9]+]]
+; Make sure there is *no* nocapture here.
+; CHECK: declare void @free(i8*) [[G3:#[0-9]+]]
 declare void @free(i8*)
 
 ; CHECK: declare double @frexp(double, i32* nocapture) [[G1]]
 declare double @frexp(double, i32*)
 
 ; CHECK: declare float @frexpf(float, i32* nocapture) [[G1]]
 declare float @frexpf(float, i32*)