[libcxx-commits] [PATCH] D128998: [libunwind] Handle G in personality string

[Fangrui Song via Phabricator via libcxx-commits]

MaskRay added inline comments.


================
Comment at: libunwind/src/libunwind.cpp:186
+// This resets MTE tags of tagged frames to zero.
+extern "C" _LIBUNWIND_HIDDEN int __unw_step_untag(unw_cursor_t *cursor) {
+  _LIBUNWIND_TRACE_API("__unw_step_untag(cursor=%p)",
----------------
fmayer wrote:
> danielkiss wrote:
> > compnerd wrote:
> > > fmayer wrote:
> > > > fmayer wrote:
> > > > > fmayer wrote:
> > > > > > danielkiss wrote:
> > > > > > > `__unw_step` is used in phase 1(search) and phase 2(apply).
> > > > > > > maybe would be better to differentiate as __unw_step_search and __unw_step_apply (or similar) to make architecture agonistic.
> > > > > > > For PAC we need to calculate the return value always but with MTE we could just untag memory when actually doing the unwinding.
> > > > > > > same for `step(bool untag = false);` could be `step(bool search_phase = false);`
> > > > > > I assume for `step(bool search_phase = false)`  you meant `step(bool apply_phase = false)` (or `step(bool search_phase = true)`).
> > > > > > 
> > > > > > I'll hold off on further renames until we can get some consensus here.
> > > > > > For PAC we need to calculate the return value always but with MTE we could just untag memory when actually doing the unwinding.
> > > > > 
> > > > > Also, we *must not* untag the frame in the search phase, because then we can cause tag mismatches in what you call the apply phase.
> > > > Another naming option would be `__unw_step_stage2`
> > > I like `__unw_step_stage2` better.
> > `__unw_step_stage2` works for me too.
> > `step(bool stage2 = false)` would be it's pair IMHO.
> @MaskRay do you also agree with this name?
`__unw_step_stage2` works for me, too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128998/new/

https://reviews.llvm.org/D128998