Tweak our symbol resolution again

[godlygeek]

I've found another case where the libpython gets mmap'ed by a process and breaks pystack's ability to find _PyRuntime. This time, I noticed it happening when Memray is symbolizing native stacks, which can trigger elfutils to mmap the text section of libpython if one of those native stack frames resolves to an address inside libpython.

When this happened, the symbol resolution callback was finding an address for the symbol, and discovering that that address was mapped, but not noticing that it fell within a different module entirely, since only a small part of libpython was mapped at that first mapping.

Address this by preferring matches which fall within the module we searched in over mappings which don't, while continuing to prefer a match in an earlier module over one in a later one.

[godlygeek]

The 3.15 CI failures can be ignored. Those failures currently exist on main, and will be fixed by rebasing after #337 is merged.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 50.00000% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.55%. Comparing base (902ac74) to head (ca8042d).

Files with missing lines	Patch %	Lines
src/pystack/_pystack/unwinder.cpp	50.00%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #336      +/-   ##
==========================================
- Coverage   77.63%   77.55%   -0.09%     
==========================================
  Files          57       57              
  Lines        6601     6612      +11     
  Branches      628      630       +2     
==========================================
+ Hits         5125     5128       +3     
- Misses       1476     1484       +8     

Flag	Coverage Δ
cpp	77.55% <50.00%> (-0.09%)	⬇️
python	77.55% <50.00%> (-0.09%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[pablogsal]

More hacks for the hacks god 😆

[pablogsal]

I think the way to solve this is to actually try to resolve the main thread in one of those images and then once we know which one contain the actual data only use that one.

[godlygeek]

I think the way to solve this is to actually try to resolve the main thread in one of those images and then once we know which one contain the actual data only use that one.

I think I follow what you're saying. You're suggesting that we could unwind the main thread, and then some of the main thread's frames have to have been in the interpreter, so we can look at the image that each frame's code falls within to see if that image contains a _PyRuntime, and stop when we find one.

If I've got that idea right, it does sound more reliable than this hack, but it does have some disadvantages:

• If DWARF isn't available and there are no frame pointers, we might not be able to unwind successfully
• If the most recent frames are below something JIT'ed, we might not be able to unwind successfully
• In order for this to not have false positives, we'd need to walk the stack from least recent to most recent frame, not most to least. Otherwise we could get it wrong if the interpreter called into an unrelated shared library that happened to also have a _PyRuntime symbol.

And it's also slower and more complex, on top of all of that.

If I understand your proposal right, I lean towards landing this PR, but keeping that idea in our back pocket in case we find another case where this still isn't good enough. Does that sound reasonable to you, @pablogsal ?

[pablogsal]

I think I follow what you're saying. You're suggesting that we could unwind the main thread, and then some of the main thread's frames have to have been in the interpreter, so we can look at the image that each frame's code falls within to see if that image contains a _PyRuntime, and stop when we find one.

If I've got that idea right, it does sound more reliable than this hack, but it does have some disadvantages:

• If DWARF isn't available and there are no frame pointers, we might not be able to unwind successfully
• If the most recent frames are below something JIT'ed, we might not be able to unwind successfully
• In order for this to not have false positives, we'd need to walk the stack from least recent to most recent frame, not most to least. Otherwise we could get it wrong if the interpreter called into an unrelated shared library that happened to also have a _PyRuntime symbol.

And it's also slower and more complex, on top of all of that.

If I understand your proposal right, I lean towards landing this PR, but keeping that idea in our back pocket in case we find another case where this still isn't good enough. Does that sound reasonable to you, @pablogsal ?

No, much simpler than this. There is only ONE of these images that will correspond to the executable and that image will have its bss initialized. So what i propose is to iterate over all valid candidate libpythons and for every libpython:

• Check if the libpython ahs the _PyRuntime
• If it has it, check if the cookie is set and the version is set
• If so, se the data there to find _PyRuntime.interpreters.head.main that is not NULL

If these conditions are satisfied this map collection will be the actual libpython driving the process and symbols for python should be resolved there. All symbols can be calculated as well in an offset-relative way to the elf file so it should work for all cases.

[godlygeek]

No, much simpler than this. There is only ONE of these images that will correspond to the executable and that image will have its bss initialized.

Ah, indeed. Although...

• If it has it, check if the cookie is set and the version is set

this only works from 3.13 onwards. There's no cookie and version in the _PyRuntime until _Py_DebugOffsets is introduced.

It may still be true that we can simply check candidates to see if they do or don't start with too many zero bytes to have been initialized, though... At least interpreters.head should be set...