Compiler crashing when compiling coroutine intrinsics without the fcoroutines-ts option.

[zahiraam]

Summary

Instead of generating an error the compiler is crashing. This patch is to fix this issue.

https://bugs.llvm.org/show_bug.cgi?id=50406

[rnk]

clang/lib/Basic/Builtins.cpp

70

The existing convention is to mark the builtins with a flag that enables them, see this MS_LANG example. Would that be a reasonable alternative? Maybe one day we will have builtins that don't start with __builtin_coro, so this would be nicer.

[aaron.ballman]

clang/include/clang/Basic/Builtins.h

40

Can you add a comment as to what the builtin requires?

clang/lib/Basic/Builtins.cpp

64

I think this should be using & rather than ==, as with the other cases (don't forget to surround the bitwise AND expression with parens to silence warnings).

[aaron.ballman]

clang/include/clang/Basic/Builtins.h

40

Sorry, I didn't catch this earlier, but shouldn't this be 0x200? I suspect this will fix the CI failures.

[aaron.ballman]

LGTM, but please wait for a few days in case @rnk or others have a concern. I don't think the CI failures are related to your changes here, btw.

[erichkeane]

No real feedback, however I note that the bug link you have is one that was resolved in 2009, so I suspect it is completely unrelated.

[erichkeane]

clang/test/SemaCXX/coroutine-builtins.cpp

7

Another nit/idea to make this a little nicer: If you use the 'bookmark' mechanism for the VerifyDiagnosticConsumer these will be much more reliable.

Basically you put

// #SomeBookmarkName

at the end of the line, and then do:

// expected-error@#SomeBookMarkName {{...

[zahiraam]

In D102871#2779575, @erichkeane wrote:

No real feedback, however I note that the bug link you have is one that was resolved in 2009, so I suspect it is completely unrelated.

Thanks Erich. Yes wrong link. Fixed.

[zahiraam]

rGa4b61c82cf1a committed.

clang/include/clang/Basic/Builtins.h

enum LanguageID {
   C_LANG = 0x2,       // builtin for c only.
   CXX_LANG = 0x4,     // builtin for cplusplus only.
   OBJC_LANG = 0x8,    // builtin for objective-c and objective-c++
   MS_LANG = 0x10,     // builtin requires MS mode.
   OCLC20_LANG = 0x20, // builtin for OpenCL C 2.0 only.
   OCLC1X_LANG = 0x40, // builtin for OpenCL C 1.x only.
   OMP_LANG = 0x80,    // builtin requires OpenMP.
   CUDA_LANG = 0x100,  // builtin requires CUDA.
+  COR_LANG = 0x200,   // builtin requires use of 'fcoroutine-ts' option.
   ALL_LANGUAGES = C_LANG | CXX_LANG | OBJC_LANG, // builtin for all languages.
   ALL_GNU_LANGUAGES = ALL_LANGUAGES | GNU_LANG,  // builtin requires GNU mode.
   ALL_MS_LANGUAGES = ALL_LANGUAGES | MS_LANG,    // builtin requires MS mode.
   ALL_OCLC_LANGUAGES = OCLC1X_LANG | OCLC20_LANG // builtin for OCLC languages.
 };
 
 namespace Builtin {
 enum ID {

clang/include/clang/Basic/Builtins.def

 BUILTIN(__builtin___get_unsafe_stack_top, "v*", "Fn")
 BUILTIN(__builtin___get_unsafe_stack_ptr, "v*", "Fn")
 
 // Nontemporal loads/stores builtins
 BUILTIN(__builtin_nontemporal_store, "v.", "t")
 BUILTIN(__builtin_nontemporal_load, "v.", "t")
 
 // Coroutine intrinsics.
-BUILTIN(__builtin_coro_resume, "vv*", "")
-BUILTIN(__builtin_coro_destroy, "vv*", "")
-BUILTIN(__builtin_coro_done, "bv*", "n")
-BUILTIN(__builtin_coro_promise, "v*v*IiIb", "n")
-
-BUILTIN(__builtin_coro_size, "z", "n")
-BUILTIN(__builtin_coro_frame, "v*", "n")
-BUILTIN(__builtin_coro_noop, "v*", "n")
-BUILTIN(__builtin_coro_free, "v*v*", "n")
-
-BUILTIN(__builtin_coro_id, "v*Iiv*v*v*", "n")
-BUILTIN(__builtin_coro_alloc, "b", "n")
-BUILTIN(__builtin_coro_begin, "v*v*", "n")
-BUILTIN(__builtin_coro_end, "bv*Ib", "n")
-BUILTIN(__builtin_coro_suspend, "cIb", "n")
-BUILTIN(__builtin_coro_param, "bv*v*", "n")
+LANGBUILTIN(__builtin_coro_resume, "vv*", "", COR_LANG)
+LANGBUILTIN(__builtin_coro_destroy, "vv*", "", COR_LANG)
+LANGBUILTIN(__builtin_coro_done, "bv*", "n", COR_LANG)
+LANGBUILTIN(__builtin_coro_promise, "v*v*IiIb", "n", COR_LANG)
+
+LANGBUILTIN(__builtin_coro_size, "z", "n", COR_LANG)
+LANGBUILTIN(__builtin_coro_frame, "v*", "n", COR_LANG)
+LANGBUILTIN(__builtin_coro_noop, "v*", "n", COR_LANG)
+LANGBUILTIN(__builtin_coro_free, "v*v*", "n", COR_LANG)
+
+LANGBUILTIN(__builtin_coro_id, "v*Iiv*v*v*", "n", COR_LANG)
+LANGBUILTIN(__builtin_coro_alloc, "b", "n", COR_LANG)
+LANGBUILTIN(__builtin_coro_begin, "v*v*", "n", COR_LANG)
+LANGBUILTIN(__builtin_coro_end, "bv*Ib", "n", COR_LANG)
+LANGBUILTIN(__builtin_coro_suspend, "cIb", "n", COR_LANG)
+LANGBUILTIN(__builtin_coro_param, "bv*v*", "n", COR_LANG)
 
 // OpenCL v2.0 s6.13.16, s9.17.3.5 - Pipe functions.
 // We need the generic prototype, since the packet type could be anything.
 LANGBUILTIN(read_pipe, "i.", "tn", OCLC20_LANG)
 LANGBUILTIN(write_pipe, "i.", "tn", OCLC20_LANG)
 
 LANGBUILTIN(reserve_read_pipe, "i.", "tn", OCLC20_LANG)
 LANGBUILTIN(reserve_write_pipe, "i.", "tn", OCLC20_LANG)

clang/lib/Basic/Builtins.cpp

bool Builtin::Context::isBuiltinFunc(llvm::StringRef FuncName) {
   return false;
 }
 
 bool Builtin::Context::builtinIsSupported(const Builtin::Info &BuiltinInfo,
                                           const LangOptions &LangOpts) {
   bool BuiltinsUnsupported =
       (LangOpts.NoBuiltin || LangOpts.isNoBuiltinFunc(BuiltinInfo.Name)) &&
       strchr(BuiltinInfo.Attributes, 'f');
+  bool CorBuiltinsUnsupported =
+      !LangOpts.Coroutines && (BuiltinInfo.Langs & COR_LANG);
   bool MathBuiltinsUnsupported =
     LangOpts.NoMathBuiltin && BuiltinInfo.HeaderName &&
     llvm::StringRef(BuiltinInfo.HeaderName).equals("math.h");
   bool GnuModeUnsupported = !LangOpts.GNUMode && (BuiltinInfo.Langs & GNU_LANG);
   bool MSModeUnsupported =
       !LangOpts.MicrosoftExt && (BuiltinInfo.Langs & MS_LANG);
   bool ObjCUnsupported = !LangOpts.ObjC && BuiltinInfo.Langs == OBJC_LANG;
   bool OclC1Unsupported = (LangOpts.OpenCLVersion / 100) != 1 &&
                           (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES ) ==  OCLC1X_LANG;
   bool OclC2Unsupported =
       (LangOpts.OpenCLVersion != 200 && !LangOpts.OpenCLCPlusPlus) &&
       (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES) == OCLC20_LANG;
   bool OclCUnsupported = !LangOpts.OpenCL &&
                          (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES);
   bool OpenMPUnsupported = !LangOpts.OpenMP && BuiltinInfo.Langs == OMP_LANG;
   bool CUDAUnsupported = !LangOpts.CUDA && BuiltinInfo.Langs == CUDA_LANG;
   bool CPlusPlusUnsupported =
       !LangOpts.CPlusPlus && BuiltinInfo.Langs == CXX_LANG;
-  return !BuiltinsUnsupported && !MathBuiltinsUnsupported && !OclCUnsupported &&
-         !OclC1Unsupported && !OclC2Unsupported && !OpenMPUnsupported &&
-         !GnuModeUnsupported && !MSModeUnsupported && !ObjCUnsupported &&
-         !CPlusPlusUnsupported && !CUDAUnsupported;
+  return !BuiltinsUnsupported && !CorBuiltinsUnsupported &&
+         !MathBuiltinsUnsupported && !OclCUnsupported && !OclC1Unsupported &&
+         !OclC2Unsupported && !OpenMPUnsupported && !GnuModeUnsupported &&
+         !MSModeUnsupported && !ObjCUnsupported && !CPlusPlusUnsupported &&
+         !CUDAUnsupported;
 }
 
 /// initializeBuiltins - Mark the identifiers for all the builtins with their
 /// appropriate builtin ID # and mark any non-portable builtin identifiers as
 /// such.
 void Builtin::Context::initializeBuiltins(IdentifierTable &Table,
                                           const LangOptions& LangOpts) {
   // Step #1: mark all target-independent builtins with their ID's.

clang/test/SemaCXX/coroutine-builtins.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify -fcoroutines-ts %s
+// RUN: %clang_cc1 -fsyntax-only -verify -DERRORS %s
+
+// Check that we don't crash when using __builtin_coro_* without the fcoroutine-ts option
+
+#ifdef ERRORS
+// expected-error@+9{{use of undeclared identifier '__builtin_coro_done'}}
+// expected-error@+9{{use of undeclared identifier '__builtin_coro_id'}}
+// expected-error@+9{{use of undeclared identifier '__builtin_coro_alloc'}}
+#else
+// expected-no-diagnostics
+#endif
+
+int main() {
+  void *co_h;
+  bool d = __builtin_coro_done(co_h);
+  __builtin_coro_id(32, 0, 0, 0);
+  __builtin_coro_alloc();
+}