From 0ab3689f1f51674b5146915cafed7ad4c7108f59 Mon Sep 17 00:00:00 2001
From: Daiva Naudziuniene <daivan@fb.com>
Date: Thu, 2 Jul 2020 01:59:36 -0700
Subject: [PATCH] [infer] NULLPTR_DEREFERENCE false positive caused by
 thread_local variable

Summary:
Keyword `thread_local` in cpp allows us to create a variable with thread storage duration, meaning that the object's lifetime begins when the thread begins and ends when the thread ends.

We get `NULLPTR_DEREFERENCE` false positive for `thread_local` variable since we reallocate it in the `VariableLifetimeBegins` metadata instruction and we do not see further updates to the variable. To solve the issue we special case `VariableLifetimeBegins`  instruction for global variables.

Reviewed By: jvillard

Differential Revision: D22284135

fbshipit-source-id: 13c14ef90
---
 infer/src/pulse/Pulse.ml                      |  4 +--
 .../tests/codetoanalyze/cpp/pulse/nullptr.cpp | 32 +++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/infer/src/pulse/Pulse.ml b/infer/src/pulse/Pulse.ml
index 2dcfadf6a..8bb805539 100644
--- a/infer/src/pulse/Pulse.ml
+++ b/infer/src/pulse/Pulse.ml
@@ -281,9 +281,9 @@ module PulseTransferFunctions = struct
               if List.is_empty ret_vars then vars else List.rev_append vars ret_vars
             in
             remove_vars vars_to_remove astates
-      | Metadata (VariableLifetimeBegins (pvar, _, location)) ->
+      | Metadata (VariableLifetimeBegins (pvar, _, location)) when not (Pvar.is_global pvar) ->
           [PulseOperations.realloc_pvar pvar location astate |> Domain.continue]
-      | Metadata (Abstract _ | Nullify _ | Skip) ->
+      | Metadata (Abstract _ | VariableLifetimeBegins _ | Nullify _ | Skip) ->
           [Domain.ContinueProgram astate] )
 
 
diff --git a/infer/tests/codetoanalyze/cpp/pulse/nullptr.cpp b/infer/tests/codetoanalyze/cpp/pulse/nullptr.cpp
index bb628a846..e6e89209a 100644
--- a/infer/tests/codetoanalyze/cpp/pulse/nullptr.cpp
+++ b/infer/tests/codetoanalyze/cpp/pulse/nullptr.cpp
@@ -122,3 +122,35 @@ X* getX(bool b) {
 }
 
 void call_modeled_abort_ok() { getX(false)->foo(); }
+
+struct S {
+  int field;
+};
+
+void set_S();
+
+struct T {
+  static S*& get() {
+    auto& s = T::getRaw();
+    if (T::getRaw() == nullptr) {
+      set_S();
+    }
+    return s;
+  }
+
+  static S*& getRaw() {
+    thread_local S* s = nullptr;
+    return s;
+  }
+};
+
+void set_S() {
+  auto& s = T::getRaw();
+  if (s != nullptr) {
+    return;
+  }
+
+  s = (S*)calloc(1, sizeof(S));
+}
+
+int thread_local_was_set_ok() { return T::get()->field; }