Evaluate writing out a MemoryInfoListStream

[gabrielesvelto]

To improve stack walking heuristics we'd like to validate ambiguous instruction pointers by checking if they fall within an executable reason. On Windows this is already possible because windbg.dll populates the MemoryInfoListStream with the whole state of the process' memory when it snapshots it. On Linux we currently have /proc/self/maps which contains the required information but we'd have to parse it in the stackwalker to yield the same information. Alternatively we might try and populate the MemoryInfoListStream on Linux too in order to be able to use the same code across different platforms.

Populating the stream should be relatively simple, info regarding the header and entries are readily available. The biggset issue is that the AllocationProtect, State, Protect and Type fields are supposed to contain Windows-specific constants. There's two ways to tackle this: either we use those fields to store Linux-specific constants - but that would defeat the purpose of having identical code for all platforms in the stack walker - or we map them to their closest Windows equivalents. Interestingly crashpad does the latter for fuchsia but it does not populate the stream on Linux and macOS.

[luser]

If someone writes a useful parser for maps you could just reuse the same implementation in this crate + the stackwalker. :)

[afranchuk]

@luser Would minidump-common be an okay common place for the maps parser? Or should it be a separate crate entirely?

[luser]

That doesn't quite feel like the right place for it, since currently minidump-common is mostly type definitions.

n.b.: @Gankra implemented stackwalker parsing of this along with a unified interface atop that + MemoryInfoListStream in rust-minidump/rust-minidump#268 . See also rust-minidump/rust-minidump#256

[afranchuk]

I agree, I was asking on the off-chance that others didn't agree with my thought so it'd be simpler :)

Thanks for the info, I was aware of the existing impl. I was just going to move it somewhere common and maybe slightly clean up the interface so that it could be used in minidump-writer!

[afranchuk]

Any suggestion on crate name/location? Alternatively, maybe we could contribute to the procfs crate and expose its process::MemoryMap::from_line() implementation?

[gabrielesvelto]

Contributing to an external crate sounds like a good idea if the crate is sufficiently widespread and useful. We parse /proc files in a lot of places within Firefox code and the procfs crate sounds like something we'll end up using eventually.

[afranchuk]

Yeah, I figured with the other procfs needs we have, it'd be great to take advantage of procfs. My only hesitation is that procfs describes itself as a crate for accessing the current system's procfs mount, so I'd like to confirm with the main developers as to whether they'd be okay with expanding the purpose of the crate (I don't expect anyone would have a problem with it, but it's best to confirm).

[afranchuk]

The procfs team seems open to contribution along these lines, so that is probably the path to follow.

[gabrielesvelto]

The procfs team seems open to contribution along these lines, so that is probably the path to follow.

Excellent!

[afranchuk]

I'm currently waiting (patiently, there's no rush) for procfs to tag a new version. Then this can proceed.

[eminence]

procfs maintainer here 👋 To set some rough expectations, I'm going to try to get a new release of procfs out within a week. Thanks to @afranchuk who did the implementation within procfs

[afranchuk]

@eminence thanks for chiming in with an update! I will set a reminder to check on things in a week.