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

72

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 ↗

(On Diff #347447)

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

clang/lib/Basic/Builtins.cpp

63

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 ↗

(On Diff #347481)

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

8

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/lib/Basic/Builtins.cpp

if (FuncName.equals(BuiltinInfo[i].Name))
       return strchr(BuiltinInfo[i].Attributes, 'f') != nullptr;
 
   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');
+      ((LangOpts.NoBuiltin || LangOpts.isNoBuiltinFunc(BuiltinInfo.Name)) &&
+           strchr(BuiltinInfo.Attributes, 'f') ||
+       // if we are not compiling with -fcoroutine-ts option then,
+       // disable the coroutine intrinsic.
+       (!LangOpts.Coroutines &&
+        strstr(BuiltinInfo.Name, "__builtin_coro") != 0));
   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);

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
+
+int main() {
+  void *co_h;
+
+#ifdef ERRORS
+// expected-error@+6{{use of undeclared identifier '__builtin_coro_done'}}
+// expected-error@+6{{use of undeclared identifier '__builtin_coro_id'}}
+// expected-error@+6{{use of undeclared identifier '__builtin_coro_alloc'}}
+#else
+// expected-no-diagnostics
+#endif
+  bool d = __builtin_coro_done(co_h);
+  __builtin_coro_id(32, 0, 0, 0);
+  __builtin_coro_alloc();
+}