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

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

420

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

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).

lib/CodeGen/CGDeclCXX.cpp

       AddGlobalCtor(Fn, Priority);
     }
   }
-  
-  // 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()));
-  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 = nullptr;
+  SourceManager &SM = Context.getSourceManager();
+  if (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.
+    SmallString<128> FileName(llvm::sys::path::filename(
+        SM.getFileEntryForID(SM.getMainFileID())->getName()));
+    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] = '_';
+    }
+    Fn = CreateGlobalInitOrDestructFunction(
+        *this, FTy, llvm::Twine("_GLOBAL__sub_I_", FileName));
+  } else {
+    Fn = CreateGlobalInitOrDestructFunction(*this, FTy, "_GLOBAL__I_a");
   }
-  llvm::Function *Fn = CreateGlobalInitOrDestructFunction(
-      *this, FTy, llvm::Twine("_GLOBAL__sub_I_", FileName));
 
   CodeGenFunction(*this).GenerateCXXGlobalInitFunc(Fn, CXXGlobalInits);
   AddGlobalCtor(Fn);