[Lldb-commits] [PATCH] D146263: [lldb] Move UnixSignals creation into Platform plugins

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D146263#4214336 <https://reviews.llvm.org/D146263#4214336>, @rupprecht wrote:

> This seems to cause a regression on Linux where we no longer get the signal details.
>
> Given an intentionally-crashy test binary, such as:
>
>   void crash() {
>     // Allocate valid but non-accessible memory and attempt to write to it,
>     // triggering a Segmentation Fault.
>     int *ptr = static_cast<int *>(
>         mmap(nullptr, sizeof(int), PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, 0, 0));
>     *ptr = 0x12345678;
>     munmap(ptr, sizeof(int));
>   }
>
> we used to see this:
>
>   $ bin/lldb crashy
>   (lldb) target create "crashy"
>   Current executable set to 'crashy' (x86_64).
>   (lldb) r
>   Process 2317185 launched: 'crashy' (x86_64)
>   Process 2317185 stopped
>   * thread #1, name = 'crashy', stop reason = signal SIGSEGV: invalid permissions for mapped object (fault address: 0x7ffff7fc5000)
>
> but now the stop reason is just: `stop reason = signal SIGSEGV` (no details about the fault address)

Yep, definitely a regression... I suspect we're not creating the correct UnixSignals object anymore in this case. It would be nice if we had more testing here... I'll try to fix this, thanks for pointing it out.

> Unrelated to the bug above: this also creates a circular dependency between gdb-server <-> gdb-remote packages (https://llvm.org/docs/CodingStandards.html#library-layering)

Ah, yeah that does happen too, doesn't it? Maybe the Signals should live in the Process plugins instead of the Platform plugins? I'm not married to this specific implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146263