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

Dmitry Vyukov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 12 01:25:02 PST 2019

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.

  rCRT Compiler Runtime



More information about the llvm-commits mailing list