Hi German, On Thu, May 19, 2022 at 02:30:23PM +0100, German Gomez via Elfutils-devel wrote: > On 28/04/2022 20:56, Mark Wielaard wrote: > > I haven't been able to look at the actual patches yet. And I am on > > vacation this week. But I'll review next week after I am back. > > Thanks a lot for looking.
And then it took a couple of months to actually review the patches, sorry. The first two patches look OK with one small issue where/how to define DW_AARCH64_RA_SIGN_STATE (lets put it in cfi.h instead of dwarf.h). I have some concerns about the last two patches because they define new public api that is aarch64 specific. > > A quick scan shows we need a aarch64 special public function, which > > would be slightly ugly imho. I had hoped it could be a variant of the > > func_addr_mask. But maybe this is too different to make more generic. > > I did consider func_addr_mask initially, but when I wrote the patch it > wasn't exposed as a perf-thread value. Currently PAC masks are constant > but might be different from thread to thread in the future. So I placed > it in the Thread struct. Aha. I assumed it was per process, but it makes sense if it is per-thread. > I agree the arch-specific naming is not pretty. I think I can certainly > rework it into a more generic feature. But I think I would need to make > sure that the masks can be supplied to the Thread struct before the > unwind. Are there any other architectures with a similar "mask"? Maybe we can make it a special dwfl_thread_state_registers call with e.g. firstreg -1 and nregs 1? That way we don't need a new function, just a "magic" negative register number. Also we need to extract the pauth mask somehow in libdwfl/linux-core-attach.c Cheers, Mark
