[Lldb-commits] [PATCH] D66638: Unwind: Add a stack scanning mechanism to support win32 unwinding

Tue Sep 24 01:51:06 PDT 2019

labath added a comment.

In D66638#1679440 <https://reviews.llvm.org/D66638#1679440>, @clayborg wrote:

> In D66638#1679195 <https://reviews.llvm.org/D66638#1679195>, @labath wrote:
>
> > Thanks for the review and sorry for the delay (I was OOO). The idea to use `Process::GetLoadAddressPermissions` makes sense, both from consistency and correctness perspectives. Unfortunately it does not work "out of the box" because minidump core files (my primary use case for this) do not currently provide enough information through the memory region info api. It tries pretty hard to achieve that, but in the case of regular (not "full memory dumps") windows minidump, all we have is the "MemoryList" stream, which only contains a couple of memory regions, and it does not include the areas covered by loaded modules (which is where PCs should point). If this is to work, we'd need to extend this memory parsing code to also take into account the module memory ranges (from the ModuleList stream).
> >
> > @clayborg, you're the one who wrote the memory region parsing code IIRC. Does that sound OK to you ?
>
>
> I am not sure if we can infer anything about permissions from the module ranges. I believe this range contains everything (.text, .data, .bss, etc), so it has a mixture of permissions?
>
> If we can find a way to use the module base of image, we should only parse the module ranges if the /proc/<pid>/maps is not in a minidump file as that is the best source for mapped ranges.

If we're able to find the actual module that got mapped at this address, then we could use the sections of that module to reconstruct the permissions fairly accurately. If we can't find that module (which will unfortunately be the most common case for me), then we can't determine the actual permissions, but we can still differentiate between a piece of memory being definitely(*) not executable (if there is no module loaded at that address) and it being _potentially_ executable (if there is a module there). Given the data we have, I think that is the best we can do.

The MemoryRegion class already kind of supports this, because it stores the permissions in a ternary enum ("yes", "no", "don't know"), but the `Process::GetLoadAddressPermissions` interprets the permissions strictly, and treats "don't know" as "no". So, one way to handle that would be to return "don't know" for these memory ranges and then change the `GetLoadAddressPermissions` interface so that we're able to ask the question "is this address potentially executable?" (**). Or we could dumb the current check down even more, and just check whether there is _a_ module at the given address (similar to the first step of `InitializeNonZerothFrame`), and completely avoid permission checking. For an initial implementation, I would be happy with a super simple heuristic like that. WDYT?

(*) "definitively" might be too strong a word, as there might be some executable memory mapped there that we don't know about, but that is fairly unlikely. OTOH, an address belonging to a module is quite likely to be executable.

(**) Of course, all of this would kick in only when we don't have a source which can provide an exhaustive list of memory regions and their permissions. linux /proc/pid/maps is one such source, and I also believe the MemoryInfoList stream can be considered to be exhaustive.

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

https://reviews.llvm.org/D66638