Not all modules were added to the modules list resulting in <unknown> stack traces #948
Conversation
… stack traces. The problem was that in order to a module to get added to the list, the next module needed to have a valid elf header, which is not always the case. The fixed code checks if the module names are different.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #948 +/- ##
==========================================
- Coverage 86.53% 82.63% -3.91%
==========================================
Files 40 53 +13
Lines 3676 7375 +3699
Branches 0 1188 +1188
==========================================
+ Hits 3181 6094 +2913
- Misses 495 1171 +676
- Partials 0 110 +110 |
| if ((!last_module.file.len | ||
| && (!module.file.len || last_module.file.len != module.file.len | ||
| || memcmp(last_module.file.ptr, module.file.ptr, | ||
| module.file.len))) | ||
| || (!module.file.len || last_module.file.len != module.file.len | ||
| || memcmp( | ||
| last_module.file.ptr, module.file.ptr, module.file.len))) { |
There was a problem hiding this comment.
you should use !sentry__slice_eq(module.file, last_module.file) here to make this more readable.
There was a problem hiding this comment.
The key here is that the ELF check must be removed. Not all memory areas are valid ELF binaries. It doesn't make sense to search for ELF header of the next memory area as a condition to add a module to the list. Quite often there's anonymous memory areas between loaded libraries, and in such case the original code doesn't work.
There was a problem hiding this comment.
Hi @sc-aki. Can you provide an example of such a memory map (or just a few entries that visualize the problem)?
Not all memory areas are valid ELF binaries.
But do they still contain valid ELF code referenced from the stack trace?
Quite often there's anonymous memory areas between loaded libraries, and in such case the original code doesn't work.
Yes, in these cases, we would ignore that map entry. Are you saying that you have anonymous entries that contain code that your stack trace is referencing? How do you ensure the code is readable in the backend symbolication?
Wouldn't this ignore anonymous map entries as well: https://github.com/getsentry/sentry-native/blob/master/src/modulefinder/sentry_modulefinder_linux.c#L677-L681
There was a problem hiding this comment.
How do you ensure the code is readable in the backend symbolication?
Or is this change only meant for Android (where symbolication happens in the client)?
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
10 similar comments
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
Not all modules were added to the modules list resulting in stack traces.
The problem was that in order to a module to get added to the list, the next module needed to have a valid elf header, which is not always the case. The fixed code checks if the module names are different.