[PATCH] D112998: [sanitizer_common] Fix readlink error handling in sanitizer_procmaps_solaris.cpp
Rainer Orth via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 2 13:55:46 PDT 2021
ro marked an inline comment as done.
ro added inline comments.
================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_procmaps_solaris.cpp:58
xmapentry->pr_mapname);
- internal_readlink(proc_path, segment->filename, segment->filename_size);
+ ssize_t sz = internal_readlink(proc_path, segment->filename,
+ segment->filename_size - 1);
----------------
vitalybuka wrote:
> vitalybuka wrote:
> > Can you check if sanitizer_common/tests/sanitizer_procmaps_test.cc can be enabled for Solaris and does it trigger this error?
> 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.
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.
================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_procmaps_solaris.cpp:58
xmapentry->pr_mapname);
- internal_readlink(proc_path, segment->filename, segment->filename_size);
+ ssize_t sz = internal_readlink(proc_path, segment->filename,
+ segment->filename_size - 1);
----------------
ro wrote:
> vitalybuka wrote:
> > vitalybuka wrote:
> > > Can you check if sanitizer_common/tests/sanitizer_procmaps_test.cc can be enabled for Solaris and does it trigger this error?
> > 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.
> 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.
Will do in a separate patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112998/new/
https://reviews.llvm.org/D112998
More information about the llvm-commits
mailing list