[libcxx][nfc] Fix the ASAN bots: update expected.pass.cpp.

[zoecarver]

Summary

Ensures that get_return_object's return type is the same as the return type for the function calling co_return. Otherwise, we try to construct an object, then free it, then return it.

[zoecarver]

It would be great if someone who knows more about coroutines than I do looked over this (which is basically anyone because I know next to nothing). Basically it seems like we get the use-after-free error any time get_return_object returns something that is not the parent object and the parent object is not empty.

[ldionne]

I have an intuition that this is related to https://bugs.llvm.org/show_bug.cgi?id=50369, which might not be fixed in reality. WDYT?

[zoecarver]

In D103196#2782994, @ldionne wrote:

I have an intuition that this is related to https://bugs.llvm.org/show_bug.cgi?id=50369, which might not be fixed in reality. WDYT?

@ldionne I think it's unlikely that this has anything to do with shared_ptr. Below is a minimal reproducer that causes the same problem with the sanitizers. As you can see, there's no reference to shared_ptr, or any pointer. I think the new/delete that the bots are referencing is the new/delete that is used to create and deallocate the coroutine state (or maybe it's the promise... that might actually explain why it's trying to use the same memory).

// ADDITIONAL_COMPILE_FLAGS: -g -fsanitize=address

#include <experimental/coroutine>
#include <cassert>
#include <memory>

#include "test_macros.h"

using namespace std::experimental;

int x;

struct tag { char data[8]; }; // `tag` can be any type. It could be empty, or an int, or anything. 

struct expected {
  char data; // No issues if this member isn't here.

  expected(tag) : data() {}

  struct promise_type {
    tag get_return_object() { return {}; } // No issues if we return an `expected` instead. 
    suspend_never initial_suspend() { return {}; }
    suspend_never final_suspend() noexcept { return {}; }
    tag return_value(tag) { return tag{}; }
    void unhandled_exception() {}
  };
};

expected f1() {
  co_return {};
}

int main(int, char**) {
  auto c1 = f1();
  (void)c1;

  return 0;
}

[zoecarver]

I have an intuition that this is related to https://bugs.llvm.org/show_bug.cgi?id=50369, which might not be fixed in reality. WDYT?

Also, that bug is fixed: https://godbolt.org/z/Ko138bn8a

They're either using too old a standard or too old of a compiler. Looks like someone already commented that, but I can investigate further and close the bug.

[ldionne]

Thanks a lot for the reduction Zoe! Here's an ever so slightly more reduced Godbolt version: https://godbolt.org/z/oPK1PcPjT. Can you file a bug against Clang? Let's ship this to fix the CI.

libcxx/test/std/experimental/language.support/support.coroutines/end.to.end/expected.pass.cpp

struct expected {
   std::shared_ptr<Data> data;
 
   expected(T val) : data(std::make_shared<Data>(val, Error())) {}
   expected(error_tag, Error error) : data(std::make_shared<Data>(T(), error)) {}
   expected(std::shared_ptr<Data> p) : data(p) {}
 
   struct promise_type {
     std::shared_ptr<Data> data;
-    std::shared_ptr<Data> get_return_object() { data = std::make_shared<Data>(); return data; }
+    expected get_return_object() { data = std::make_shared<Data>(); return {data}; }
     suspend_never initial_suspend() { return {}; }
     suspend_never final_suspend() noexcept { return {}; }
     void return_value(T v) { data->val = v; data->error = {}; }
     void unhandled_exception() {}
   };
 
   bool await_ready() { return !data->error; }
   T await_resume() { return data->val; }