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

Nico Weber thakis at chromium.org
Tue Aug 26 09:51:26 PDT 2014


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.

================
Comment at: lib/CodeGen/CGDeclCXX.cpp:430
@@ -424,4 +429,3 @@
+    Fn = CreateGlobalInitOrDestructFunction(*this, FTy, "_GLOBAL__sub_I__null_");
   }
 
----------------
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.

================
Comment at: unittests/CodeGen/BufferSourceTest.cpp:33
@@ +32,3 @@
+// if the source file is a memory buffer.
+const char *TestProgram =
+    "class EmitCXXGlobalInitFunc    "
----------------
const char TestProgram[] =


================
Comment at: unittests/CodeGen/BufferSourceTest.cpp:42
@@ +41,3 @@
+TEST(BufferSourceTest, EmitCXXGlobalInitFunc) {
+    CompilerInstance *compiler = new CompilerInstance;
+
----------------
Does anything own this? I think this should be stack-allocated. (?)

================
Comment at: unittests/CodeGen/CMakeLists.txt:16
@@ +15,3 @@
+  clangTooling
+  clangCodegen
+  )
----------------
This has to be clangCodeGen (note capital G). Maybe it should be CodeGenTests for ConsistenCy.

http://reviews.llvm.org/D5043






More information about the cfe-commits mailing list