[PATCH] D112998: [sanitizer_common] Fix readlink error handling in sanitizer_procmaps_solaris.cpp

[Rainer Orth via Phabricator via llvm-commits]

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