[PATCH] D112059: Fix inline builtin handling in case of redefinition

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a subscriber: nathanchance.
nickdesaulniers added a comment.

Yes; GCC does behave this way.  It does not consider a non-gnu-inline redefinition an error, and it does seem to prefer the non-gnu-inline redeclaration when both are present, AFAICT.  The test is verifying that behavior correctly.  This patch is fixing the test case, the reported reduced cases from @manojgupta link <https://reviews.llvm.org/D111009#3068566>, @nathanchance link <https://github.com/ClangBuiltLinux/linux/issues/1477#issuecomment-943650477>, and myself link <https://github.com/ClangBuiltLinux/linux/issues/1477#issuecomment-944811052>, and fixing the kernel builds link <https://github.com/ClangBuiltLinux/continuous-integration2/runs/3854281995?check_suite_focus=true>.

Further, I did a build+boot tests of:

1. mainline x86_64 + CONFIG_FORTIFY_SOURCE=y
2. mainline x86_64 + CONFIG_FORTIFY_SOURCE=y + CONFIG_LTO_CLANG_FULL=y
3. mainline x86_64 + CONFIG_FORTIFY_SOURCE=y + CONFIG_LTO_CLANG_THIN=y



================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1318
+    // external one. That's GCC behavior too.
+    else {
+      for (auto const *Redecl : FD->redecls()) {
----------------
do we really want to be iterating every redaclaration like this, even for non-inline builtin declarations? Is there a way to avoid the below loop for most functions? Perhaps we should be doing this when we hit a redeclaration instead? Or wrap all this work in a check that there's a builtin ID associated with the FD?


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1328
+          }
+        }
+      }
----------------
If there are multiple redeclarations, do we want to be erasing the clone each time, or can we `break` out of this loop?


================
Comment at: clang/test/CodeGen/strlen-inline-builtin-redecl.c:10-12
+extern inline __attribute__((always_inline))
+__attribute__((gnu_inline)) unsigned long
+strlen(const char *p) {
----------------
Has this example been formatted? Does rotating the attributes to the front help?


================
Comment at: clang/test/CodeGen/strlen-inline-builtin-redecl.c:42
+}
+extern typeof(strlen) strlen;
----------------
I think this line can be dropped without affecting the test. Or consider using any of the further reduced test cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112059