Fix clangsa on LLVM 10 (#37828)

* [GCChecker] Fix assertion

We were associating a symbol with a `void` location. This is illegal,
but happened to let us keep track of arraylist entries. Stop that
abuse by manually modeling indexing into arraylist and fix the
GCChecker assertion.

* Backport LLVM 10 clangsa patch from Yggdrasil

Author: Keno

deps/llvm.mk

@@ -491,6 +491,7 @@ $(eval $(call LLVM_PATCH,llvm-D84031)) # remove for LLVM 12
 $(eval $(call LLVM_PATCH,llvm-10-D85553)) # remove for LLVM 12
 $(eval $(call LLVM_PATCH,llvm-10-r_aarch64_prel32)) # remove for LLVM 12
 $(eval $(call LLVM_PATCH,llvm-10-r_ppc_rel)) # remove for LLVM 12
+$(eval $(call LLVM_PATCH,llvm-10-unique_function_clang-sa))
 ifeq ($(BUILD_LLVM_CLANG),1)
 $(eval $(call LLVM_PATCH,llvm-D88630-clang-cmake))
 endif
@@ -508,6 +509,7 @@ $(eval $(call LLVM_PATCH,llvm-julia-tsan-custom-as))
 $(eval $(call LLVM_PATCH,llvm-D80101)) # remove for LLVM 12
 $(eval $(call LLVM_PATCH,llvm-D84031)) # remove for LLVM 12
 $(eval $(call LLVM_PATCH,llvm-10-D85553)) # remove for LLVM 12
+$(eval $(call LLVM_PATCH,llvm-10-unique_function_clang-sa))
 ifeq ($(BUILD_LLVM_CLANG),1)
 $(eval $(call LLVM_PATCH,llvm-D88630-clang-cmake))
 endif

deps/patches/llvm-10-unique_function_clang-sa.patch

@@ -0,0 +1,28 @@
+From 1fa6efaa946243004c45be92e66b324dc980df7d Mon Sep 17 00:00:00 2001
+From: Valentin Churavy <v.churavy@gmail.com>
+Date: Thu, 17 Sep 2020 23:22:45 +0200
+Subject: [PATCH] clang-sa can't determine that !RHS implies !LHS
+
+---
+ llvm/include/llvm/ADT/FunctionExtras.h | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/include/llvm/ADT/FunctionExtras.h b/include/llvm/ADT/FunctionExtras.h
+index 121aa527a5d..b9b6d829b14 100644
+--- a/include/llvm/ADT/FunctionExtras.h
++++ b/include/llvm/ADT/FunctionExtras.h
+@@ -193,9 +193,11 @@ public:
+     // Copy the callback and inline flag.
+     CallbackAndInlineFlag = RHS.CallbackAndInlineFlag;
+ 
++#ifndef __clang_analyzer__
+     // If the RHS is empty, just copying the above is sufficient.
+     if (!RHS)
+       return;
++#endif
+ 
+     if (!isInlineStorage()) {
+       // The out-of-line case is easiest to move.
+-- 
+2.28.0
+

src/clangsa/GCChecker.cpp

@@ -1030,15 +1030,15 @@ SymbolRef GCChecker::getSymbolForResult(const Expr *Result,
                                         const ValueState *OldValS,
                                         ProgramStateRef &State,
                                         CheckerContext &C) const {
+  QualType QT = Result->getType();
+  if (!QT->isPointerType() || QT->getPointeeType()->isVoidType())
+    return nullptr;
   auto ValLoc = State->getSVal(Result, C.getLocationContext()).getAs<Loc>();
   if (!ValLoc) {
     return nullptr;
   }
   SVal Loaded = State->getSVal(*ValLoc);
   if (Loaded.isUnknown() || !Loaded.getAsSymbol()) {
-    QualType QT = Result->getType();
-    if (!QT->isPointerType())
-      return nullptr;
     if (OldValS || GCChecker::isGCTracked(Result)) {
       Loaded = C.getSValBuilder().conjureSymbolVal(
           nullptr, Result, C.getLocationContext(), Result->getType(),

src/module.c

@@ -258,14 +258,24 @@ typedef struct _modstack_t {
 
 static jl_binding_t *jl_get_binding_(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var, modstack_t *st);
 
+static inline jl_module_t *module_usings_getidx(jl_module_t *m JL_PROPAGATES_ROOT, size_t i) JL_NOTSAFEPOINT;
+
+#ifndef __clang_analyzer__
+// The analyzer doesn't like looking through the arraylist, so just model the
+// access for it using this function
+static inline jl_module_t *module_usings_getidx(jl_module_t *m JL_PROPAGATES_ROOT, size_t i) JL_NOTSAFEPOINT {
+    return (jl_module_t*)m->usings.items[i];
+}
+#endif
+
 // find a binding from a module's `usings` list
 // called while holding m->lock
 static jl_binding_t *using_resolve_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var, modstack_t *st, int warn)
 {
     jl_binding_t *b = NULL;
     jl_module_t *owner = NULL;
     for(int i=(int)m->usings.len-1; i >= 0; --i) {
-        jl_module_t *imp = (jl_module_t*)m->usings.items[i];
+        jl_module_t *imp = module_usings_getidx(m, i);
         // TODO: make sure this can't deadlock
         JL_LOCK(&imp->lock);
         jl_binding_t *tempb = _jl_get_module_binding(imp, var);

test/clangsa/MissingRoots.c

@@ -329,7 +329,7 @@ jl_module_t *propagation(jl_module_t *m JL_PROPAGATES_ROOT);
 void module_member(jl_module_t *m)
 {
     for(int i=(int)m->usings.len-1; i >= 0; --i) {
-      jl_module_t *imp = (jl_module_t*)m->usings.items[i];
+      jl_module_t *imp = propagation(m);
       jl_gc_safepoint();
       look_at_value((jl_value_t*)imp);
       jl_module_t *prop = propagation(imp);