C#: Make query only apply to reftypes, since I believe valuetypes are safe or cannot be fixed trivially using the volatile keyword.

Author: calumgrant

csharp/ql/src/Concurrency/UnsafeLazyInitialization.ql

@@ -3,7 +3,7 @@
  * @description A repeated check on a non-volatile field is not thread-safe on some platforms,
  *              and could result in unexpected behavior.
  * @kind problem
- * @problem.severity recommendation
+ * @problem.severity error
  * @precision medium
  * @id cs/unsafe-double-checked-lock
  * @tags correctness
@@ -40,5 +40,5 @@ where
   doubleCheckedLock(field, ifs) and
   not field.isVolatile() and
   exists(VariableWrite write | write = ifs.getThen().getAChild+() and write.getTarget() = field) and
-  not field.getType() instanceof Struct
+  field.getType() instanceof RefType
 select ifs, "Field $@ should be 'volatile' for this double-checked lock.", field, field.getName()

csharp/ql/test/query-tests/Concurrency/UnsafeLazyInitialization/UnsafeLazyInitialization.cs

@@ -31,7 +31,7 @@ void Fn()
                 if (obj1 == null)
                     obj1 = null;
 
-        // BAD
+        // GOOD: A value-type
         if (cond1)
             lock (mutex)
                 if (cond1)
@@ -132,9 +132,9 @@ void Fn()
         }
 
         // BAD: Field x should be volatile
-        if (struct1.x == 2)
+        if (struct1.x is null)
             lock (mutex)
-                if(struct1.x == 2)
+                if(struct1.x is null)
                     struct1.x = 3;
 
         // GOOD: Tuples are structs so cannot be volatile.
@@ -151,7 +151,7 @@ void Fn()
 
 struct Coord
 {
-    public int x, y;
+    public object x, y;
 
     public static bool operator==(Coord c1, Coord c2) => c1.x==c2.x && c1.y == c2.y;
     public static bool operator!=(Coord c1, Coord c2) => !(c1==c2);

csharp/ql/test/query-tests/Concurrency/UnsafeLazyInitialization/UnsafeLazyInitialization.expected

@@ -1,9 +1,7 @@
 | UnsafeLazyInitialization.cs:17:9:26:9 | if (...) ... | Field $@ should be 'volatile' for this double-checked lock. | UnsafeLazyInitialization.cs:6:19:6:22 | obj1 | obj1 |
 | UnsafeLazyInitialization.cs:29:9:32:32 | if (...) ... | Field $@ should be 'volatile' for this double-checked lock. | UnsafeLazyInitialization.cs:6:19:6:22 | obj1 | obj1 |
-| UnsafeLazyInitialization.cs:35:9:38:34 | if (...) ... | Field $@ should be 'volatile' for this double-checked lock. | UnsafeLazyInitialization.cs:9:10:9:14 | cond1 | cond1 |
 | UnsafeLazyInitialization.cs:80:9:88:9 | if (...) ... | Field $@ should be 'volatile' for this double-checked lock. | UnsafeLazyInitialization.cs:6:19:6:22 | obj1 | obj1 |
 | UnsafeLazyInitialization.cs:98:9:107:9 | if (...) ... | Field $@ should be 'volatile' for this double-checked lock. | UnsafeLazyInitialization.cs:6:19:6:22 | obj1 | obj1 |
 | UnsafeLazyInitialization.cs:110:9:120:9 | if (...) ... | Field $@ should be 'volatile' for this double-checked lock. | UnsafeLazyInitialization.cs:6:19:6:22 | obj1 | obj1 |
 | UnsafeLazyInitialization.cs:110:9:120:9 | if (...) ... | Field $@ should be 'volatile' for this double-checked lock. | UnsafeLazyInitialization.cs:8:12:8:15 | obj3 | obj3 |
-| UnsafeLazyInitialization.cs:135:9:138:34 | if (...) ... | Field $@ should be 'volatile' for this double-checked lock. | UnsafeLazyInitialization.cs:154:16:154:16 | x | x |
-| UnsafeLazyInitialization.cs:141:9:148:9 | if (...) ... | Field $@ should be 'volatile' for this double-checked lock. | UnsafeLazyInitialization.cs:12:15:12:19 | pair1 | pair1 |
+| UnsafeLazyInitialization.cs:135:9:138:34 | if (...) ... | Field $@ should be 'volatile' for this double-checked lock. | UnsafeLazyInitialization.cs:154:19:154:19 | x | x |