Page MenuHomePhabricator

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

Authored by zoecarver on May 26 2021, 12:02 PM.

Details

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.

Diff Detail

Event Timeline

zoecarver created this revision.May 26 2021, 12:02 PM
zoecarver requested review of this revision.May 26 2021, 12:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2021, 12:02 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

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.

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?

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;
}

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 accepted this revision.May 26 2021, 1:51 PM

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.

This revision is now accepted and ready to land.May 26 2021, 1:51 PM