This is an archive of the discontinued LLVM Phabricator instance.

[sanitizer]: fix warnings reported by SVACE static analyzer.
Needs ReviewPublic

Authored by drmtmych on Feb 12 2019, 1:09 AM.

Details

Reviewers
kcc
yuri
Summary

The warnings are:

  • lsan_thread.cc: return value of a function '__lsan::CurrentThreadContext' is dereferenced without checking.
  • sanitizer_libc.cc: casting a signed value which has type 'char' to a bigger unsigned integer type 'unsigned int' while initializing a variable.
  • sanitizer_libignore.cc: constructor may not initialize class members of '__sanitizer::LibIgnore'.
  • sanitizer_printf.cc: 'minimal_num_length' with type 'u8', is promoted to type 'int' 32b in 'minimal_num_length - pos', then sign-extended to type 'unsigned long' 64b.
  • sanitizer_symbolizer_posix_libcdep.cc: after having been compared to NULL value, pointer (...)->path is passed as 1st parameter in call to function '__sanitizer::LLVMSymbolizer::LLVMSymbolizer', where it is dereferenced.

Diff Detail

Event Timeline

drmtmych created this revision.Feb 12 2019, 1:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 12 2019, 1:09 AM
Herald added subscribers: Restricted Project, llvm-commits, jdoerfert and 7 others. · View Herald Transcript
dvyukov added inline comments.Feb 12 2019, 1:24 AM
lib/lsan/lsan_thread.cc
125

Please add a test that triggers NULL deref here. It worked all the time, so this must be some tricky corner case. It would be useful to have it captured in a test.

lib/sanitizer_common/sanitizer_libc.cc
118

c1 and c2 are compared with less below. It looks like this can change semantics. Is it intentional? How does it change semantics? Char can be either signed or unsigned, so result of this function will be implementation defined, which looks strange.

lib/sanitizer_common/sanitizer_libignore.cc
26

This breaks linker-initialized-ness. It looks like this makes correct code incorrect...

lib/sanitizer_common/sanitizer_printf.cc
65

What is the type of bug this warning finds? Does it find any true positives in this code base? Or is this a true positive? If yes, then please add a test.

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc
458

How can NULL path be passed to LLVMSymbolizer? I fail to see the exact scenario.