Don't segfault in EmitCXXGlobalInitFunc when main file is a membuf

[loladiro]

Summary

When the main file is created from a membuffer, there is no file entry that can be retrieved. This uses "__GLOBAL_I_a" in that case which is what was always used before r208128.

[thakis]

Is it possible to test this?

I think if the input comes from stdin, sys::path::filename() returns <stdin>, which then gets turned into _stdin_ by the isPreprocessingNumberBody() loop – maybe this should be <null> / _null_ instead of the old _a spelling? (I don't care too much about this part.)

[rsmith]

lib/CodeGen/CGDeclCXX.cpp

415 ↗

(On Diff #12888)

if (FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID())) {

420 ↗

(On Diff #12888)

MainFile->getName()));

[loladiro]

Fixed minor nits, changed GLOBAL_I_a to _GLOBALsub_I__null_ and added a test. I'd appreciate if somebody who uses CMake to run the unittests could test this. I tried to modify the CMakeLists by pattern matching, but haven't actually tested this.

[thakis]

I patched this in and tried it with CMake, it mostly works (with the capital G mentioned below). I also checked that it runs as part of ninja check-clang. lgtm with comments addressed.

lib/CodeGen/CGDeclCXX.cpp

430 ↗

(On Diff #12909)

Nit: I'd just do

- SmallString<128> FileName(llvm::sys::path::filename(
-      SM.getFileEntryForID(SM.getMainFileID())->getName()));
+  SmallString<128> FileName;
+  if (const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID()))
+    FileName = llvm::sys::path::filename(MainFile->getName());
+  else
+    FileName = "<null>";

and nothing else, then you don't duplicate the CreateGlobalInitOrDestructFunction call.

unittests/CodeGen/BufferSourceTest.cpp

33 ↗

(On Diff #12909)

const char TestProgram[] =

42 ↗

(On Diff #12909)

Does anything own this? I think this should be stack-allocated. (?)

unittests/CodeGen/CMakeLists.txt

16 ↗

(On Diff #12909)

This has to be clangCodeGen (note capital G). Maybe it should be CodeGenTests for ConsistenCy.

[thakis]

(I left a comment on this on phab a while ago, but I haven't received an
email for that yet. Not sure if phab's mailer is slow. The comment does
show up for me on the web interface.)

[loladiro]

Addressed review comments.

[thakis]

lgtm

Closed by commit rL216495 (authored by kfischer).

cfe/trunk/lib/CodeGen/CGDeclCXX.cpp

for (SmallVectorImpl<GlobalInitData >::iterator
       
       for (; I < PrioE; ++I)
         LocalCXXGlobalInits.push_back(I->second);
 
       CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, LocalCXXGlobalInits);
       AddGlobalCtor(Fn, Priority);
     }
   }
-  
+  SmallString<128> FileName;
+  SourceManager &SM = Context.getSourceManager();
+  if (const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID())) {
     // Include the filename in the symbol name. Including "sub_" matches gcc and
     // makes sure these symbols appear lexicographically behind the symbols with
     // priority emitted above.
-  SourceManager &SM = Context.getSourceManager();
-  SmallString<128> FileName(llvm::sys::path::filename(
-      SM.getFileEntryForID(SM.getMainFileID())->getName()));
+    FileName = llvm::sys::path::filename(MainFile->getName());
+  } else {
+    FileName = SmallString<128>("<null>");
+  }
+
   for (size_t i = 0; i < FileName.size(); ++i) {
     // Replace everything that's not [a-zA-Z0-9._] with a _. This set happens
     // to be the set of C preprocessing numbers.
     if (!isPreprocessingNumberBody(FileName[i]))
       FileName[i] = '_';
   }
+
   llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
       *this, FTy, llvm::Twine("_GLOBAL__sub_I_", FileName));
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);
 
   CXXGlobalInits.clear();
   PrioritizedCXXGlobalInits.clear();

cfe/trunk/unittests/CMakeLists.txt

 if(CLANG_ENABLE_STATIC_ANALYZER)
   add_subdirectory(Frontend)
 endif()
 add_subdirectory(ASTMatchers)
 add_subdirectory(AST)
 add_subdirectory(Tooling)
 add_subdirectory(Format)
 add_subdirectory(Sema)
+add_subdirectory(CodeGen)
 # FIXME: Why are the libclang unit tests disabled on Windows?
 if(NOT WIN32) 
   add_subdirectory(libclang)
 endif()

cfe/trunk/unittests/CodeGen/BufferSourceTest.cpp

+//===- unittests/CodeGen/BufferSourceTest.cpp - MemoryBuffer source tests -===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Parse/ParseAST.h"
+#include "clang/Basic/TargetInfo.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/IR/LLVMContext.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+
+// Emitting constructors for global objects involves looking
+// at the source file name. This makes sure that we don't crash
+// if the source file is a memory buffer.
+const char TestProgram[] =
+    "class EmitCXXGlobalInitFunc    "
+    "{                              "
+    "public:                        "
+    "   EmitCXXGlobalInitFunc() {}  "
+    "};                             "
+    "EmitCXXGlobalInitFunc test;    ";
+
+TEST(BufferSourceTest, EmitCXXGlobalInitFunc) {
+    CompilerInstance compiler;
+
+    compiler.createDiagnostics();
+    compiler.getLangOpts().CPlusPlus = 1;
+    compiler.getLangOpts().CPlusPlus11 = 1;
+
+    compiler.getTargetOpts().Triple = llvm::Triple::normalize(
+        llvm::sys::getProcessTriple());
+    compiler.setTarget(clang::TargetInfo::CreateTargetInfo(
+      compiler.getDiagnostics(),
+      std::make_shared<clang::TargetOptions>(
+        compiler.getTargetOpts())));
+
+    compiler.createFileManager();
+    compiler.createSourceManager(compiler.getFileManager());
+    compiler.createPreprocessor(clang::TU_Prefix);
+
+    compiler.createASTContext();
+
+    compiler.setASTConsumer(std::unique_ptr<ASTConsumer>(
+        CreateLLVMCodeGen(
+            compiler.getDiagnostics(),
+            "EmitCXXGlobalInitFuncTest",
+            compiler.getCodeGenOpts(),
+            compiler.getTargetOpts(),
+            llvm::getGlobalContext())));
+
+    compiler.createSema(clang::TU_Prefix,NULL);
+
+    clang::SourceManager &sm = compiler.getSourceManager();
+    sm.setMainFileID(sm.createFileID(
+        llvm::MemoryBuffer::getMemBuffer(TestProgram), clang::SrcMgr::C_User));
+
+    clang::ParseAST(compiler.getSema(), false, false);
+}
+
+}

cfe/trunk/unittests/CodeGen/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_clang_unittest(CodeGenTests
+  BufferSourceTest.cpp
+  )
+
+target_link_libraries(CodeGenTests
+  clangAST
+  clangBasic
+  clangFrontend
+  clangParse
+  clangSema
+  clangTooling
+  clangCodeGen
+  )

cfe/trunk/unittests/CodeGen/Makefile

+##===- unittests/CodeGen/Makefile --------------------------*- Makefile -*-===##
+#
+#                     The LLVM Compiler Infrastructure
+#
+# This file is distributed under the University of Illinois Open Source
+# License. See LICENSE.TXT for details.
+#
+##===----------------------------------------------------------------------===##
+
+CLANG_LEVEL = ../..
+TESTNAME = CodeGen
+include $(CLANG_LEVEL)/../../Makefile.config
+LINK_COMPONENTS := $(TARGETS_TO_BUILD) asmparser bitreader support mc option
+USEDLIBS = clangTooling.a clangFrontend.a clangSerialization.a clangDriver.a \
+           clangRewrite.a clangRewriteFrontend.a \
+           clangParse.a clangSema.a clangAnalysis.a \
+           clangEdit.a clangAST.a clangASTMatchers.a clangLex.a clangBasic.a \
+           clangCodeGen.a
+
+include $(CLANG_LEVEL)/unittests/Makefile

cfe/trunk/unittests/Makefile

 ##===- unittests/Makefile ----------------------------------*- Makefile -*-===##
 #
 #                     The LLVM Compiler Infrastructure
 #
 # This file is distributed under the University of Illinois Open Source
 # License. See LICENSE.TXT for details.
 #
 ##===----------------------------------------------------------------------===##
 
 # If CLANG_LEVEL is not set, then we are the top-level Makefile. Otherwise, we
 # are being included from a subdirectory makefile.
 
 ifndef CLANG_LEVEL
 
 IS_UNITTEST_LEVEL := 1
 CLANG_LEVEL := ..
-PARALLEL_DIRS = Basic Lex Driver libclang Format ASTMatchers AST Tooling Sema
+PARALLEL_DIRS = CodeGen Basic Lex Driver libclang Format ASTMatchers AST Tooling \
+          Sema
 
 include $(CLANG_LEVEL)/../..//Makefile.config
 
 ifeq ($(ENABLE_CLANG_ARCMT),1)
 PARALLEL_DIRS += Frontend
 endif
 
 endif  # CLANG_LEVEL