[PATCH] D58108: [sanitizer]: fix warnings reported by SVACE static analyzer.

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added inline comments.


================
Comment at: lib/lsan/lsan_thread.cc:125
+    ThreadContext *tc = CurrentThreadContext();
+    if (tc) tc->os_id = GetTid();
+  }
----------------
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.


================
Comment at: lib/sanitizer_common/sanitizer_libc.cc:118
+    char c1 = *s1;
+    char c2 = *s2;
     if (c1 != c2) return (c1 < c2) ? -1 : 1;
----------------
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.


================
Comment at: lib/sanitizer_common/sanitizer_libignore.cc:26
+  atomic_store_relaxed(&instrumented_ranges_count_, 0);
+  internal_memset(ignored_code_ranges_, 0, sizeof(ignored_code_ranges_));
+  internal_memset(instrumented_code_ranges_, 0,
----------------
This breaks linker-initialized-ness. It looks like this makes correct code incorrect...


================
Comment at: lib/sanitizer_common/sanitizer_printf.cc:65
+                    sizeof(num_buffer[0]) *
+                        ((u64)minimal_num_length - (u64)pos));
     pos = minimal_num_length;
----------------
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.


================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:458
+      VReport(2, "Using llvm-symbolizer at user-specified path: %s\n", path);
+      return new(*allocator) LLVMSymbolizer(path, allocator);
+    } else if (!internal_strcmp(binary_name, "atos")) {
----------------
How can NULL path be passed to LLVMSymbolizer? I fail to see the exact scenario.


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58108/new/

https://reviews.llvm.org/D58108