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

Reply via email to