[PATCH] D92657: Fix interaction between clang and some inline builtins from glibc under _FORTIFY_SOURCE

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

This code was originally added to solve https://llvm.org/pr9614, which has a fair amount of context for how we got here today.



================
Comment at: clang/test/CodeGen/memmove-always-inline-definition-used.c:17-19
+// CHECK-LABEL: define available_externally i8* @memmove
+// CHECK: call void @llvm.memcpy
+// CHECK: call i8* @memmove({{.*}}) #[[ATTR:[0-9]+]]
----------------
Even if this happens to work in practice, memmove is still mutually recursive, and some other IPO transform could break this. The code pattern seems fragile.

If we are serious about implementing glibc fortify, which, to be clear, is a new direction for us, we should think about what IR pattern we really want to see out of clang.

I think this IR better represents the intention of the original program, and is better for optimizations and other IR analysis consumers:
```
define void @usercode() {
...
  call void @__fortify_memmove(...)
...
}
define i8* @__fortify_memmove(...) {
...
  call i8* @llvm.memcpy(...)
...
  call i8* @memmove(...)
...
}
declare i8* @memmove(...)
```

If we're going to have to live with this glibc fortify code pattern, maybe clang should try to spot it and rewrite it into more helpful and analyzable LLVM IR.

---

If people agree that this is a reasonable direction, let's think about how to implement it.

One way to get there would be to implement asm labels by waiting until after IRGen to do some value replacement. So, clang would emit code for the inline definition of `@memmove`, and then later, it would rename `@memmove_alias` to `@memmove`. At that point, it would invent a new name for the original `@memmove`.  The least effort thing to do would be to rename it to `@memmove.1` using the normal value renaming. We would never see this symbol in the ELF symbol table, because these are usually marked always inline.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92657/new/

https://reviews.llvm.org/D92657