This is an archive of the discontinued LLVM Phabricator instance.

[lldb][Process/FreeBSD] Add support for address masks on aarch64
Needs ReviewPublic

Authored by andrew on Feb 24 2022, 7:13 AM.

Details

Reviewers
mgorny
Summary

Read the mask using ptrace with PT_GETREGSET with NT_ARM_ADDR_MASK
for live processes, or from the NT_ARM_ADDR_MASK note in a core
file.

As NT_ARM_ADDR_MASK is on FreeBSD is the same value as NT_ARM_PAC_MASK
and it contains a structure with the same layout and a superset of the
same data use this when reading a core file.

Diff Detail

Event Timeline

andrew created this revision.Feb 24 2022, 7:13 AM
andrew requested review of this revision.Feb 24 2022, 7:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2022, 7:13 AM
emaste added inline comments.Feb 24 2022, 9:04 AM
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
127

This #define is coming from our headers? We need to provide this constant within LLVM so this will be available on non-FreeBSD hosts.

We need to make sure a test covers this as well, perhaps just enabling lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py?

andrew added inline comments.Mar 1 2022, 7:44 AM
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
127

My understanding is this file is used on FreeBSD as it's used when debugging a currently running process.

emaste added inline comments.Mar 23 2022, 9:25 AM
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
127

Oh yes of course.

Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2022, 9:25 AM

We need to make sure a test covers this as well, perhaps just enabling lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py?

I have a local change to use the test, however it depends on forcing isAArch64PAuth in lldb/packages/Python/lldbsuite/test/lldbtest.py to return true. We'll need a way to detect which CPU features are supported. On Linux it reads /proc/cpuinfo, however there isn't a similar simple method on FreeBSD.

labath added a subscriber: labath.Mar 23 2022, 12:14 PM

Can you detect it from inside the debugee? You could have it exit with some predefined error code if the feature is not available (and then skip the test based on that)...

It's easy to detect in the debugee as it can check which hardware capabilities are passed to it from the kernel.

Another option could be to have a build tool that prints these capabilities and have the test decide based on it as they are identical on FreeBSD and Linux.

I did something along these lines for lldb-server memory tagging tests in lldb/test/API/tools/lldb-server/memory-tagging/main.c. In that case you've got 3 points where you we could skip the test, and you could check the auxv but that was overkill for this example.

Does FreeBSD have an equivalent to the auxv feature bits? (we could always sprinkle some #defines around if not).

andrew updated this revision to Diff 420205.Apr 4 2022, 9:17 AM

Add support for the PAC test

Thanks for enabling the test, great to see support for this outside Linux.

lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
817

I was going to suggest deduping this and the Linux function but then I found a problem with the Linux one that needs fixing. https://reviews.llvm.org/D122411#change-cwOvKZ9JBiEU

We could probably move everything within the if thread_sp into a function to reuse it but I can do that after I get the Linux one correct.

lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
104

Remove the extra newline here.

lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
37–38

Might as well delete these lines then. Perhaps a comment before backtrace = "# Between Linux and FreeBSD the last frames differ". (first frames? I get them mixed up sometimes)

40

You could do:

if frame.GetFunctionName().startswith(""___lldb_unnamed_symbol"):

If I understand correctly, on FreeBSD this unamed symbol marks the end of the backtrace basically? If so please comment before the if to say so.

lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c
19

Do you have any worry that HWCAP_PACA might not be defined or has it been around long enough at this point to be ok? I don't know what toolchains you usually build with.

For Linux we've sometimes done #ifndef something #define it to what we know the constant should be, but other times we just use it, with mixed results.

22

Add a comment like:

// We expect Linux to check for PAC up front using /proc/cpuinfo.
andrew updated this revision to Diff 426993.May 4 2022, 6:57 AM
andrew marked 5 inline comments as done and an inline comment as not done.

Cleanup based on feedback

Looks good to me but final decision for @mgorny .

lldb/test/API/python_api/sbvalue_const_addrof/main.cpp
2

Stray change.

andrew updated this revision to Diff 427010.May 4 2022, 7:56 AM

Remove a stray change