nickdesaulniers added a comment.
hmm...this seems to cause a bunch of ICEs building the kernel:
Global is external, but doesn't have external or weak linkage!
i64 (i8*)* @strlen
fatal error: error in backend: Broken module found, compilation aborted!
Global is external, but doesn't have external or weak linkage!
i8* (i8*, i8*, i64)* @strncpy
Global is external, but doesn't have external or weak linkage!
i8* (i8*, i8*)* @strcpy
Global is external, but doesn't have external or weak linkage!
i64 (i8*)* @strlen
Global is external, but doesn't have external or weak linkage!
i64 (i8*, i8*, i64)* @strlcpy
Global is external, but doesn't have external or weak linkage!
i8* (i8*, i32, i64)* @memchr
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:4895
+
+ // Replaceable builtin provide their own implementation of a builtin. If we
+ // are in an inline builtin implementation, avoid trivial infinite
----------------
s/Replaceable builtin/Replaceable builtins/
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:4898
// recursion.
+ StringRef FDInlineName = (FD->getName() + ".inline").str();
if (!FD->isInlineBuiltinDeclaration() ||
----------------
When building this patch (with Clang, via cmake vars `-DCMAKE_C_COMPILER=` and
`-DCMAKE_CXX_COMPILER=`), I observe the following warning:
```
[99/110] Building CXX object
tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/CGExpr.cpp.o
/android0/llvm-project/clang/lib/CodeGen/CGExpr.cpp:4898:30: warning: object
backing the pointer will be destroyed at the end of the full-expression
[-Wdangling-gsl]
StringRef FDInlineName = (FD->getName() + ".inline").str();
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
This leads to crashes when using the resulting image built with assertions
enabled.
```
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index afed918c5c7f..cb6469d58b4c 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -4895,9 +4895,9 @@ static CGCallee EmitDirectCallee(CodeGenFunction &CGF,
GlobalDecl GD) {
// Replaceable builtin provide their own implementation of a builtin. If we
// are in an inline builtin implementation, avoid trivial infinite
// recursion.
- StringRef FDInlineName = (FD->getName() + ".inline").str();
+ Twine FDInlineName = FD->getName() + ".inline";
if (!FD->isInlineBuiltinDeclaration() ||
- CGF.CurFn->getName() == FDInlineName) {
+ CGF.CurFn->getName() == FDInlineName.str()) {
return CGCallee::forBuiltin(builtinID, FD);
}
// When directing calling an inline builtin, call it through it's mangled
@@ -4906,7 +4906,7 @@ static CGCallee EmitDirectCallee(CodeGenFunction &CGF,
GlobalDecl GD) {
llvm::Constant *CalleePtr = EmitFunctionDeclPointer(CGF.CGM, GD);
llvm::Function *Fn = llvm::cast<llvm::Function>(CalleePtr);
llvm::Module &M = CGF.CGM.getModule();
- llvm::Function *Clone = M.getFunction(FDInlineName);
+ llvm::Function *Clone = M.getFunction(FDInlineName.str());
if (!Clone) {
Clone = llvm::Function::Create(Fn->getFunctionType(), Fn->getLinkage(),
Fn->getAddressSpace(),
```
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:4901
+ CGF.CurFn->getName() == FDInlineName) {
return CGCallee::forBuiltin(builtinID, FD);
+ }
----------------
I think if you flip the order of the branches by inverting the logic in the
conditional, then you could avoid needing braces for the single statement
branch. ie.
```
if (FD->isInlineBuiltinDeclaration() && CGF.CurFn->getName() != FDInlineName) {
llvm::Constant *CalleePtr = EmitFunctionDeclPointer(CGF.CGM, GD);
...
} else
return CGCallee::forBuiltin(builtinID, FD);
```
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:4908
+ llvm::Function *Fn = llvm::cast<llvm::Function>(CalleePtr);
+ llvm::Module &M = CGF.CGM.getModule();
+ llvm::Function *Clone = M.getFunction(FDInlineName);
----------------
If you have a handle to a `Function`, you can get a pointer to it's module via
`Function::getParent()`.
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:4913
+ Fn->getAddressSpace(),
+ FD->getName() + ".inline", &M);
+ Clone->addFnAttr(llvm::Attribute::AlwaysInline);
----------------
Can we reuse `FDInlineName` here for the 4th param?
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1302
+ if (FD->isInlineBuiltinDeclaration() && Fn) {
+ llvm::Module &M = CGM.getModule();
+ llvm::Function *Clone = M.getFunction((Fn->getName() + ".inline").str());
----------------
could use `Fn->getParent()` here, too.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1307
+ Fn->getAddressSpace(),
+ (Fn->getName() + ".inline").str(), &M);
+ Clone->addFnAttr(llvm::Attribute::AlwaysInline);
----------------
`Fn->getName() + ".inline"` appears twice. Want to save it in a local variable
then reuse it, similar to what you do above in clang/lib/CodeGen/CGExpr.cpp?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111009/new/
https://reviews.llvm.org/D111009
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
