From dc3f8913d2ad33b1129ea488393e12cc88061aff Mon Sep 17 00:00:00 2001 From: Nigel Perks Date: Wed, 24 Jun 2020 12:20:29 -0700 Subject: [PATCH] Fix crash on XCore on unused inline in EmitTargetMetadata EmitTargetMetadata passed to emitTargetMD a null pointer as returned from GetGlobalValue, for an unused inline function which has been removed from the module at that point. A FIXME in CodeGenModule.cpp commented that the calling code in EmitTargetMetadata should be moved into the one target that needs it (XCore). A review comment agreed. So the calling loop has been moved into the XCore subclass. The check for null is done in that loop. Differential Revision: https://reviews.llvm.org/D77068 --- clang/lib/CodeGen/CodeGenModule.cpp | 17 +----------- clang/lib/CodeGen/CodeGenModule.h | 3 --- clang/lib/CodeGen/TargetInfo.cpp | 34 +++++++++++++++++++----- clang/lib/CodeGen/TargetInfo.h | 9 ++++--- clang/test/CodeGen/xcore-unused-inline.c | 9 +++++++ 5 files changed, 43 insertions(+), 29 deletions(-) create mode 100644 clang/test/CodeGen/xcore-unused-inline.c diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 5e902aa15bcf..71778ac31660 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -663,7 +663,7 @@ void CodeGenModule::Release() { if (!getCodeGenOpts().RecordCommandLine.empty()) EmitCommandLineMetadata(); - EmitTargetMetadata(); + getTargetCodeGenInfo().emitTargetMetadata(*this, MangledDeclNames); EmitBackendOptionsMetadata(getCodeGenOpts()); } @@ -5791,21 +5791,6 @@ void CodeGenModule::EmitCommandLineMetadata() { CommandLineMetadata->addOperand(llvm::MDNode::get(Ctx, CommandLineNode)); } -void CodeGenModule::EmitTargetMetadata() { - // Warning, new MangledDeclNames may be appended within this loop. - // We rely on MapVector insertions adding new elements to the end - // of the container. - // FIXME: Move this loop into the one target that needs it, and only - // loop over those declarations for which we couldn't emit the target - // metadata when we emitted the declaration. - for (unsigned I = 0; I != MangledDeclNames.size(); ++I) { - auto Val = *(MangledDeclNames.begin() + I); - const Decl *D = Val.first.getDecl()->getMostRecentDecl(); - llvm::GlobalValue *GV = GetGlobalValue(Val.second); - getTargetCodeGenInfo().emitTargetMD(D, GV, *this); - } -} - void CodeGenModule::EmitCoverageFile() { if (getCodeGenOpts().CoverageDataFile.empty() && getCodeGenOpts().CoverageNotesFile.empty()) diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 59b27fa32a1b..1ebc907bbf65 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -1532,9 +1532,6 @@ private: /// Emit the Clang commandline as llvm.commandline metadata. void EmitCommandLineMetadata(); - /// Emits target specific Metadata for global declarations. - void EmitTargetMetadata(); - /// Emit the module flag metadata used to pass options controlling the /// the backend to LLVM. void EmitBackendOptionsMetadata(const CodeGenOptions CodeGenOpts); diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp index d28326f9f2f8..24237c460687 100644 --- a/clang/lib/CodeGen/TargetInfo.cpp +++ b/clang/lib/CodeGen/TargetInfo.cpp @@ -9535,11 +9535,15 @@ public: class XCoreTargetCodeGenInfo : public TargetCodeGenInfo { mutable TypeStringCache TSC; + void emitTargetMD(const Decl *D, llvm::GlobalValue *GV, + const CodeGen::CodeGenModule &M) const; + public: XCoreTargetCodeGenInfo(CodeGenTypes &CGT) : TargetCodeGenInfo(std::make_unique(CGT)) {} - void emitTargetMD(const Decl *D, llvm::GlobalValue *GV, - CodeGen::CodeGenModule &M) const override; + void emitTargetMetadata(CodeGen::CodeGenModule &CGM, + const llvm::MapVector + &MangledDeclNames) const override; }; } // End anonymous namespace. @@ -9700,11 +9704,13 @@ StringRef TypeStringCache::lookupStr(const IdentifierInfo *ID) { /// The output is tested by test/CodeGen/xcore-stringtype.c. /// static bool getTypeString(SmallStringEnc &Enc, const Decl *D, - CodeGen::CodeGenModule &CGM, TypeStringCache &TSC); + const CodeGen::CodeGenModule &CGM, + TypeStringCache &TSC); /// XCore uses emitTargetMD to emit TypeString metadata for global symbols. -void XCoreTargetCodeGenInfo::emitTargetMD(const Decl *D, llvm::GlobalValue *GV, - CodeGen::CodeGenModule &CGM) const { +void XCoreTargetCodeGenInfo::emitTargetMD( + const Decl *D, llvm::GlobalValue *GV, + const CodeGen::CodeGenModule &CGM) const { SmallStringEnc Enc; if (getTypeString(Enc, D, CGM, TSC)) { llvm::LLVMContext &Ctx = CGM.getModule().getContext(); @@ -9716,6 +9722,21 @@ void XCoreTargetCodeGenInfo::emitTargetMD(const Decl *D, llvm::GlobalValue *GV, } } +void XCoreTargetCodeGenInfo::emitTargetMetadata( + CodeGen::CodeGenModule &CGM, + const llvm::MapVector &MangledDeclNames) const { + // Warning, new MangledDeclNames may be appended within this loop. + // We rely on MapVector insertions adding new elements to the end + // of the container. + for (unsigned I = 0; I != MangledDeclNames.size(); ++I) { + auto Val = *(MangledDeclNames.begin() + I); + llvm::GlobalValue *GV = CGM.GetGlobalValue(Val.second); + if (GV) { + const Decl *D = Val.first.getDecl()->getMostRecentDecl(); + emitTargetMD(D, GV, CGM); + } + } +} //===----------------------------------------------------------------------===// // SPIR ABI Implementation //===----------------------------------------------------------------------===// @@ -10049,7 +10070,8 @@ static bool appendType(SmallStringEnc &Enc, QualType QType, } static bool getTypeString(SmallStringEnc &Enc, const Decl *D, - CodeGen::CodeGenModule &CGM, TypeStringCache &TSC) { + const CodeGen::CodeGenModule &CGM, + TypeStringCache &TSC) { if (!D) return false; diff --git a/clang/lib/CodeGen/TargetInfo.h b/clang/lib/CodeGen/TargetInfo.h index 85565475eee7..250e6b81c7ce 100644 --- a/clang/lib/CodeGen/TargetInfo.h +++ b/clang/lib/CodeGen/TargetInfo.h @@ -57,10 +57,11 @@ public: virtual void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &M) const {} - /// emitTargetMD - Provides a convenient hook to handle extra - /// target-specific metadata for the given global. - virtual void emitTargetMD(const Decl *D, llvm::GlobalValue *GV, - CodeGen::CodeGenModule &M) const {} + /// emitTargetMetadata - Provides a convenient hook to handle extra + /// target-specific metadata for the given globals. + virtual void emitTargetMetadata( + CodeGen::CodeGenModule &CGM, + const llvm::MapVector &MangledDeclNames) const {} /// Determines the size of struct _Unwind_Exception on this platform, /// in 8-bit units. The Itanium ABI defines this as: diff --git a/clang/test/CodeGen/xcore-unused-inline.c b/clang/test/CodeGen/xcore-unused-inline.c new file mode 100644 index 000000000000..8b5bdbf2ed6d --- /dev/null +++ b/clang/test/CodeGen/xcore-unused-inline.c @@ -0,0 +1,9 @@ +// REQUIRES: xcore-registered-target +// RUN: %clang_cc1 -triple xcore-unknown-unknown -emit-llvm -o - %s + +// D77068 fixes a segmentation fault and assertion failure "Unexpected null +// Value" in the case of an unused inline function, when targeting xcore. This +// test verifies that clang does not crash and does not produce code for such a +// function. + +inline void dead_function(void) {}