avogelsgesang added inline comments.
================ Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:246-248 + DataExtractor data(&promise_addr, sizeof(promise_addr), + process_sp->GetByteOrder(), + process_sp->GetAddressByteSize()); ---------------- labath wrote: > Have you checked there won't be a use-after-free problem here, given that > this data extractor will refer to the stack object? > > To create persistent data, you need to use the DataBufferSP constructor, but > I'm wondering if we couldn't fix this by creating the (non-pointer) object > using the `CreateValueObjectFromAddress` function, as above, but then > actually use valobj->AddressOf as the synthetic child. > > I am also somewhat surprised that we need to use the GetAddressOf trick here, > as this seems to indicate that the coroutine contains (in the proper C > "subobject" kind of way) the promise object. That's not necessarily wrong, > but it makes me think we may be "breaking the cycle" at the wrong place. Thanks for taking a look! > To create persistent data, you need to use the DataBufferSP constructor good point, I will keep this in mind as a fallback in case we don't decide to follow any of the other directions you hinted at. > wondering if we couldn't fix this by creating the (non-pointer) object using > the CreateValueObjectFromAddress function, as above, but then actually use > valobj->AddressOf as the synthetic child Good idea! I will give it a try > [...] as this seems to indicate that the coroutine contains (in the proper C > "subobject" kind of way) the promise object. That's not necessarily wrong, > but it makes me think we may be "breaking the cycle" at the wrong place. The physical layout of this is: ``` // in the standard library template<typename promise_type> struct exception_handle<promise_type> { __coro_frame<promise_type>* __hdl; // <--- this is the pointer we read with `GetCoroFramePtrFromHandle` }; // compiler-generated coroutine frame. Generated ad-hoc per coroutine struct __coro_frame<promise_type> { // The ABI guaranteees that hose two pointers are always the first two pointers in the struct. void (*resume)(void*); // function pointer for type erasure void (*destroy)(void*); // function pointer for type erasure // Next comes our promise type. This is under the control of the program author promise_type promise; // Next comes any compiler-generated, internal state which gets persisted across suspension points. // The functions pointed to by `resume`/`destroy` know how to interpret this part of the coroutine frame. int __suspension_point_id; double __some_internal_state; std::string __some_other_internal_state; .... }; ``` The programmer (i.e., most likely the user of this pretty-printer), wrote only the "promise" explicitly in his code. Everything else is compiler-generated. As such, the lldb-user will usually look for the "promise" first, and I would like to make it easy to find it, by exposing it as a top-level children of the `exception_handle` instead of hiding it inside a sub-child. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132815/new/ https://reviews.llvm.org/D132815 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits