Page MenuHomePhabricator

[compiler-rt] Symbolizer refactoring: Implement GetListOfModules on Windows
AbandonedPublic

Authored by kubamracek on Mar 21 2015, 4:52 PM.

Details

Reviewers
None
Summary

One more step towards unifying the symbolizer classes. This time, I'm implementing GetListOfModules on Windows, which then allows to use the current "POSIX" implementation of FindModuleNameAndOffsetForAddress and FindModuleForAddress on all platforms.

Diff Detail

Event Timeline

kubamracek updated this revision to Diff 22415.Mar 21 2015, 4:52 PM
kubamracek retitled this revision from to [compiler-rt] Symbolizer refactoring: Implement GetListOfModules on Windows.
kubamracek updated this object.
kubamracek edited the test plan for this revision. (Show Details)
kubamracek added subscribers: Unknown Object (MLST), zaks.anna, glider and 2 others.

Hi Kuba,

Have you seen the "Sanitizer coverage: PC vs modules are not handled efficiently?" email thread on llvm-commits I've started recently?

Basically, I'm up to refactor this very code and I think the current interface and implementation are suboptimal.
How blocked is your progress by this change? If you can wait a week or two, I can improve the code that uses these functions first and try to simplify the interface.
Since I have to do that anyways, I'm not sure it's efficient for us to do the same thing twice in parallel :)

lib/sanitizer_common/sanitizer_symbolizer.cc
164

This line leaks a lot of memory every time a module lookup fails [e.g. a new shared object is dynamically loaded].
I understand you've just moved this code around, but I think we should come up with a better solution if we've decided to refactor this code.

lib/sanitizer_common/sanitizer_win.cc
226

This line looks dangerous to me now.
LoadedModule is not guaranteed to be qsort-friendly.
[ I think "trivially copyable" is what matters here http://www.cplusplus.com/reference/type_traits/is_trivially_copyable/ ]

It already owns a const char *full_name_;, and one day somebody might decide ~LoadedModule dtor should free up some memory, or hold some non-POD member.

Hi Timur,

Have you seen the "Sanitizer coverage: PC vs modules are not handled efficiently?" email thread on llvm-commits I've started recently?
Basically, I'm up to refactor this very code and I think the current interface and implementation are suboptimal.
How blocked is your progress by this change? If you can wait a week or two, I can improve the code that uses these functions first and try to simplify the interface.

Please go ahead with your planned changes and don't worry about this patch now, we can revisit it later. Can you cc me on patches/commits?

Kuba

Sure, no problem!

timurrrr added inline comments.Mar 26 2015, 10:08 AM
lib/sanitizer_common/sanitizer_symbolizer.cc
167

Did it work for you on Linux?

GetListOfModules depends on LibC, common symbolizer interface doesn't.

kubamracek abandoned this revision.Apr 9 2015, 3:37 AM