This is an archive of the discontinued LLVM Phabricator instance.

[ASan/Win] Unify handling of loaded modules between POSIX and Windows
ClosedPublic

Authored by timurrrr on Apr 2 2015, 10:47 AM.

Details

Reviewers
samsonov
Summary

This is somewhat based on D8513 by Kuba Brecka.

Diff Detail

Event Timeline

timurrrr updated this revision to Diff 23165.Apr 2 2015, 10:47 AM
timurrrr retitled this revision from to [ASan/Win] Unify handling of loaded modules between POSIX and Windows.
timurrrr updated this object.
timurrrr edited the test plan for this revision. (Show Details)
timurrrr added a reviewer: samsonov.
timurrrr added subscribers: kubamracek, earthdok, Unknown Object (MLST).
timurrrr updated this revision to Diff 23213.Apr 3 2015, 4:04 AM
timurrrr updated this object.

Fixed hangs on Linux

timurrrr updated this revision to Diff 23217.Apr 3 2015, 5:11 AM
timurrrr updated this object.

Addressed the remaining TODO item.

Alexey, can you please take a look?

samsonov accepted this revision.Apr 3 2015, 3:32 PM
samsonov edited edge metadata.

Cool, I believe it's a step in right direction. Feel free to submit after fixing a few comments below.

sanitizer_linux_libcdep.cc
439

Nit: we don't usually use non-const references. Can you change it to pointer?

sanitizer_procmaps_common.cc
145

Ditto

sanitizer_procmaps_mac.cc
174

nullptr

sanitizer_symbolizer.h
143

You don't need this function - it just calls ::GetListOfModules() in both implementations. Please delete it.

sanitizer_win.cc
363

See comment about non-const reference above.

This revision is now accepted and ready to land.Apr 3 2015, 3:32 PM

r234150, thanks for the review!
Please take a look at the FIXME I've added to lib/sanitizer_common/sanitizer_common.h

sanitizer_linux_libcdep.cc
439

Umm, yeah. Fixed

sanitizer_procmaps_common.cc
145

Done

sanitizer_procmaps_mac.cc
174

I haven't changed that code really, but fixed anyways.

sanitizer_symbolizer.h
143

This is a little tricky, as it fails to link as GetListOfModules is in libcdep nowadays...
I've added a FIXME -- can you please take a look? I don't really understand the logic behind libcdep yet.

timurrrr closed this revision.Apr 24 2015, 10:20 AM