labath 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());
----------------
avogelsgesang wrote:
> 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.
> 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.
That makes sense. And I think that's a good argument for automatically
"dereferencing" that object (i.e., status quo). That said, it's not fully clear
to me how do we end up looping here. I take it the promise object contains a
(compiler-generated ?) pointer to another __coro_frame object? What would
happen if we turned *that* into a pointer?
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
