[clang] 8849831 - [Coroutines] Warning if return type of coroutine_handle::address is not void*

[Jun Ma via cfe-commits]

Author: Chuanqi Xu
Date: 2020-07-06T13:46:01+08:00
New Revision: 8849831d55a203eca1069a0e11877ab7e7e0ac57

URL: https://github.com/llvm/llvm-project/commit/8849831d55a203eca1069a0e11877ab7e7e0ac57
DIFF: https://github.com/llvm/llvm-project/commit/8849831d55a203eca1069a0e11877ab7e7e0ac57.diff

LOG: [Coroutines] Warning if return type of coroutine_handle::address is not void*

User can own a version of coroutine_handle::address() whose return type is not
void* by using template specialization for coroutine_handle<> for some
promise_type.

In this case, the codes may violate the capability with existing async C APIs
that accepted a void* data parameter which was then passed back to the
user-provided callback.

Patch by ChuanqiXu

Differential Revision: https://reviews.llvm.org/D82442

Added: 
    clang/test/SemaCXX/coroutine_handle-addres-return-type.cpp

Modified: 
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/CodeGen/CodeGenFunction.h
    clang/lib/Sema/SemaCoroutine.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f0921337f312..5b94aa8c4325 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10527,6 +10527,9 @@ def err_await_suspend_invalid_return_type : Error<
 def note_await_ready_no_bool_conversion : Note<
   "return type of 'await_ready' is required to be contextually convertible to 'bool'"
 >;
+def warn_coroutine_handle_address_invalid_return_type : Warning <
+  "return type of 'coroutine_handle<>::address should be 'void*' (have %0) in order to get capability with existing async C API.">,
+  InGroup<Coroutine>;
 def err_coroutine_promise_final_suspend_requires_nothrow : Error<
   "the expression 'co_await __promise.final_suspend()' is required to be non-throwing"
 >;

diff  --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 6b2538a677e5..b1841d646643 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1751,6 +1751,7 @@ class CodeGenFunction : public CodeGenTypeCache {
       ~InlinedRegionBodyRAII() { CGF.AllocaInsertPt = OldAllocaIP; }
     };
   };
+
 private:
   /// CXXThisDecl - When generating code for a C++ member function,
   /// this will hold the implicit 'this' declaration.

diff  --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 70b8fd282056..992cccac6405 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -391,7 +391,13 @@ static Expr *maybeTailCall(Sema &S, QualType RetType, Expr *E,
     return nullptr;
 
   Expr *JustAddress = AddressExpr.get();
-  // FIXME: Check that the type of AddressExpr is void*
+
+  // Check that the type of AddressExpr is void*
+  if (!JustAddress->getType().getTypePtr()->isVoidPointerType())
+    S.Diag(cast<CallExpr>(JustAddress)->getCalleeDecl()->getLocation(),
+           diag::warn_coroutine_handle_address_invalid_return_type)
+        << JustAddress->getType();
+
   return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
                           JustAddress);
 }

diff  --git a/clang/test/SemaCXX/coroutine_handle-addres-return-type.cpp b/clang/test/SemaCXX/coroutine_handle-addres-return-type.cpp
new file mode 100644
index 000000000000..a95138365234
--- /dev/null
+++ b/clang/test/SemaCXX/coroutine_handle-addres-return-type.cpp
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -verify %s -stdlib=libc++ -std=c++1z -fcoroutines-ts -fsyntax-only
+
+namespace std::experimental {
+template <class Promise = void>
+struct coroutine_handle;
+
+template <>
+struct coroutine_handle<void> {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept;
+  void *address() const;
+};
+
+template <class Promise>
+struct coroutine_handle : public coroutine_handle<> {
+};
+
+template <class... Args>
+struct void_t_imp {
+  using type = void;
+};
+template <class... Args>
+using void_t = typename void_t_imp<Args...>::type;
+
+template <class T, class = void>
+struct traits_sfinae_base {};
+
+template <class T>
+struct traits_sfinae_base<T, void_t<typename T::promise_type>> {
+  using promise_type = typename T::promise_type;
+};
+
+template <class Ret, class... Args>
+struct coroutine_traits : public traits_sfinae_base<Ret> {};
+} // namespace std::experimental
+
+struct suspend_never {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+struct task {
+  struct promise_type {
+    auto initial_suspend() { return suspend_never{}; }
+    auto final_suspend() noexcept { return suspend_never{}; }
+    auto get_return_object() { return task{}; }
+    static void unhandled_exception() {}
+    void return_void() {}
+  };
+};
+
+namespace std::experimental {
+template <>
+struct coroutine_handle<task::promise_type> : public coroutine_handle<> {
+  coroutine_handle<task::promise_type> *address() const; // expected-warning {{return type of 'coroutine_handle<>::address should be 'void*'}}
+};
+} // namespace std::experimental
+
+struct awaitable {
+  bool await_ready();
+
+  std::experimental::coroutine_handle<task::promise_type>
+  await_suspend(std::experimental::coroutine_handle<> handle);
+  void await_resume();
+} a;
+
+task f() {
+  co_await a;
+}
+
+int main() {
+  f();
+  return 0;
+}