This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer_common] Fix readlink error handling in sanitizer_procmaps_solaris.cpp
ClosedPublic

Authored by ro on Nov 2 2021, 3:33 AM.

Details

Summary

As Rich Lowe pointed out in Bug 52371, the Solaris version of MemoryMappingLayout::Next completely failed to handle readlink errors or properly
NUL-terminate the result.

This patch fixes this. Originally provided in the PR with slight formatting changes.

Tested on amd64-pc-solaris2.11.

I'm uncertain how to properly attribute the patch on commit, though.

Diff Detail

Event Timeline

ro created this revision.Nov 2 2021, 3:33 AM
ro requested review of this revision.Nov 2 2021, 3:33 AM
Herald added a subscriber: Restricted Project. · View Herald TranscriptNov 2 2021, 3:33 AM
vitalybuka accepted this revision.Nov 2 2021, 10:46 AM
vitalybuka added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_procmaps_solaris.cpp
58–66

Can you check if sanitizer_common/tests/sanitizer_procmaps_test.cc can be enabled for Solaris and does it trigger this error?

This revision is now accepted and ready to land.Nov 2 2021, 10:46 AM
vitalybuka added inline comments.Nov 2 2021, 10:57 AM
compiler-rt/lib/sanitizer_common/sanitizer_procmaps_solaris.cpp
58–66

Would you like to fix usage of internal_readlink in ReadBinaryName?
it's called only for zero-initialized global buffer so, there is very low probability to hit error, but it's incorrect in general.

ro marked an inline comment as done.Nov 2 2021, 1:55 PM
ro added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_procmaps_solaris.cpp
58–66

The test already runs and PASSes on Solaris/i386 and x86_64.
sanitizer_common tests are not yet enabled on sparc, though (D91608).

If my suspicion mentioned in the PR is true, the random failures
only seen on the Solaris/amd64 buildbot once in a while would
be gone for this patch.

58–66

Will do in a separate patch.