Merge branch 'main' into macrometric2

actions/ql/lib/codeql/actions/security/ArgumentInjectionQuery.qll

@@ -100,8 +100,6 @@ private module ArgumentInjectionConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     result = sink.getLocation()
     or

actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll

@@ -333,8 +333,6 @@ private module ArtifactPoisoningConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     result = sink.getLocation()
     or

actions/ql/lib/codeql/actions/security/CodeInjectionQuery.qll

@@ -80,8 +80,6 @@ private module CodeInjectionConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     result = sink.getLocation()
     or

actions/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll

@@ -130,8 +130,6 @@ private module EnvPathInjectionConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     result = sink.getLocation()
     or

actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll

@@ -184,8 +184,6 @@ private module EnvVarInjectionConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     result = sink.getLocation()
     or

actions/ql/lib/codeql/actions/security/OutputClobberingQuery.qll

@@ -212,8 +212,6 @@ private module OutputClobberingConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 /** Tracks flow of unsafe user input that is used to construct and evaluate an environment variable. */

actions/ql/lib/codeql/actions/security/RequestForgeryQuery.qll

@@ -18,8 +18,6 @@ private module RequestForgeryConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { sink instanceof RequestForgerySink }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 /** Tracks flow of unsafe user input that is used to construct and evaluate a system command. */

actions/ql/lib/codeql/actions/security/SecretExfiltrationQuery.qll

@@ -17,8 +17,6 @@ private module SecretExfiltrationConfig implements DataFlow::ConfigSig {
   predicate isSink(DataFlow::Node sink) { sink instanceof SecretExfiltrationSink }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 /** Tracks flow of unsafe user input that is used in a context where it may lead to a secret exfiltration. */

actions/ql/src/Models/CompositeActionsSinks.ql

@@ -26,8 +26,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

actions/ql/src/Models/CompositeActionsSources.ql

@@ -36,8 +36,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

actions/ql/src/Models/CompositeActionsSummaries.ql

@@ -27,8 +27,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

actions/ql/src/Models/ReusableWorkflowsSinks.ql

@@ -26,8 +26,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

actions/ql/src/Models/ReusableWorkflowsSources.ql

@@ -36,8 +36,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

actions/ql/src/Models/ReusableWorkflowsSummaries.ql

@@ -27,8 +27,6 @@ private module MyConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module MyFlow = TaintTracking::Global<MyConfig>;

cpp/ql/src/Critical/OverflowDestination.ql

@@ -85,10 +85,8 @@ module OverflowDestinationConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(FunctionCall fc | result = fc.getLocation() |
+    exists(FunctionCall fc | result = [fc.getLocation(), sink.getLocation()] |
       sourceSized(fc, sink.asIndirectConvertedExpr())
     )
   }

cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql

@@ -171,12 +171,10 @@ module NonConstFlowConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    result = sink.getLocation()
+    exists(FormattingFunctionCall call, Expr formatString |
-    or
+      result = [call.getLocation(), sink.getLocation()]
-    exists(FormattingFunctionCall call, Expr formatString | result = call.getLocation() |
+    |
       isSinkImpl(sink, formatString) and
       call.getArgument(call.getFormatParameterIndex()) = formatString
     )

cpp/ql/src/Security/CWE/CWE-078/ExecTainted.ql

@@ -155,7 +155,7 @@ module ExecTaintConfig implements DataFlow::StateConfigSig {
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     exists(DataFlow::Node concatResult, Expr command, ExecState state |
-      result = [concatResult.getLocation(), command.getLocation()] and
+      result = [concatResult.getLocation(), command.getLocation(), sink.getLocation()] and
       isSink(sink, state) and
       isSinkImpl(sink, command, _) and
       concatResult = state.getOutgoingNode()

cpp/ql/src/Security/CWE/CWE-089/SqlTainted.ql

@@ -58,7 +58,9 @@ module SqlTaintedConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(Expr taintedArg | result = taintedArg.getLocation() | taintedArg = asSinkExpr(sink))
+    exists(Expr taintedArg | result = [taintedArg.getLocation(), sink.getLocation()] |
+      taintedArg = asSinkExpr(sink)
+    )
   }
 }
 

cpp/ql/src/Security/CWE/CWE-120/UnboundedWrite.ql

@@ -128,7 +128,7 @@ module Config implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(BufferWrite bw | result = bw.getLocation() | isSink(sink, bw, _))
+    exists(BufferWrite bw | result = [bw.getLocation(), sink.getLocation()] | isSink(sink, bw, _))
   }
 }
 

cpp/ql/src/Security/CWE/CWE-190/ArithmeticUncontrolled.ql

@@ -124,7 +124,8 @@ module UncontrolledArithConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSourceLocation(DataFlow::Node source) {
-    result = getExpr(source).getLocation()
+    isSource(source) and
+    result = [getExpr(source).getLocation(), source.getLocation()]
   }
 }
 

cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql

@@ -95,7 +95,7 @@ module TaintedAllocationSizeConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(Expr alloc | result = alloc.getLocation() | allocSink(alloc, sink))
+    exists(Expr alloc | result = [alloc.getLocation(), sink.getLocation()] | allocSink(alloc, sink))
   }
 }
 

cpp/ql/src/Security/CWE/CWE-290/AuthenticationBypass.ql

@@ -76,7 +76,9 @@ module Config implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(Expr condition | result = condition.getLocation() | isSink(sink, condition))
+    exists(Expr condition | result = [condition.getLocation(), sink.getLocation()] |
+      isSink(sink, condition)
+    )
   }
 }
 

cpp/ql/src/Security/CWE/CWE-311/CleartextBufferWrite.ql

@@ -51,7 +51,9 @@ module ToBufferConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(SensitiveBufferWrite w | result = w.getLocation() | isSinkImpl(sink, w))
+    exists(SensitiveBufferWrite w | result = [w.getLocation(), sink.getLocation()] |
+      isSinkImpl(sink, w)
+    )
   }
 }
 

cpp/ql/src/Security/CWE/CWE-311/CleartextFileWrite.ql

@@ -35,11 +35,13 @@ module FromSensitiveConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSourceLocation(DataFlow::Node sourceNode) {
-    exists(SensitiveExpr source | result = source.getLocation() | isSourceImpl(sourceNode, source))
+    exists(SensitiveExpr source | result = [source.getLocation(), sourceNode.getLocation()] |
+      isSourceImpl(sourceNode, source)
+    )
   }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(FileWrite w | result = w.getLocation() | isSinkImpl(sink, w, _))
+    exists(FileWrite w | result = [w.getLocation(), sink.getLocation()] | isSinkImpl(sink, w, _))
   }
 }
 

cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql

@@ -249,7 +249,9 @@ module FromSensitiveConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(NetworkSendRecv networkSendRecv | result = networkSendRecv.getLocation() |
+    exists(NetworkSendRecv networkSendRecv |
+      result = [networkSendRecv.getLocation(), sink.getLocation()]
+    |
       isSinkSendRecv(sink, networkSendRecv)
     )
   }

cpp/ql/src/Security/CWE/CWE-313/CleartextSqliteDatabase.ql

@@ -127,13 +127,13 @@ module FromSensitiveConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSourceLocation(DataFlow::Node source) {
-    exists(SensitiveExpr sensitive | result = sensitive.getLocation() |
+    exists(SensitiveExpr sensitive | result = [sensitive.getLocation(), source.getLocation()] |
       isSourceImpl(source, sensitive)
     )
   }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(SqliteFunctionCall sqliteCall | result = sqliteCall.getLocation() |
+    exists(SqliteFunctionCall sqliteCall | result = [sqliteCall.getLocation(), sink.getLocation()] |
       isSinkImpl(sink, sqliteCall, _)
     )
   }

cpp/ql/src/Security/CWE/CWE-319/UseOfHttp.ql

@@ -91,10 +91,9 @@ module HttpStringToUrlOpenConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSourceLocation(DataFlow::Node source) {
-    result = source.asIndirectExpr().getLocation()
+    isSource(source) and
+    result = [source.asIndirectExpr().getLocation(), source.getLocation()]
   }
-
-  Location getASelectedSinkLocation(DataFlow::Node sink) { none() }
 }
 
 module HttpStringToUrlOpen = TaintTracking::Global<HttpStringToUrlOpenConfig>;

cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.ql

@@ -50,8 +50,6 @@ module WordexpTaintConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
 }
 
 module WordexpTaint = TaintTracking::Global<WordexpTaintConfig>;

cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql

@@ -187,12 +187,14 @@ module ArrayAddressToDerefConfig implements DataFlow::StateConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSourceLocation(DataFlow::Node source) {
-    exists(Variable v | result = v.getLocation() | isSourceImpl(source, v))
+    exists(Variable v | result = v.getLocation() or result = source.getLocation() |
+      isSourceImpl(source, v)
+    )
   }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     exists(PointerArithmeticInstruction pai, Instruction deref |
-      result = [pai, deref].getLocation() and
+      result = [[pai, deref].getLocation(), sink.getLocation()] and
       isInvalidPointerDerefSink2(sink, deref, _) and
       isSink(sink, ArrayAddressToDerefConfig::TOverflowArithmetic(pai))
     )

cpp/ql/src/experimental/Security/CWE/CWE-409/DecompressionBombs.ql

@@ -31,8 +31,6 @@ module DecompressionTaintConfig implements DataFlow::ConfigSig {
 
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     exists(FunctionCall fc | result = [sink.getLocation(), fc.getLocation()] | isSink(fc, sink))
   }

csharp/ql/src/Security Features/CWE-327/DontInstallRootCert.ql

@@ -39,8 +39,6 @@ module AddCertToRootStoreConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module AddCertToRootStore = DataFlow::Global<AddCertToRootStoreConfig>;

docs/codeql/codeql-overview/system-requirements.rst

@@ -46,6 +46,10 @@ For Rust extraction:
 
 - ``rustup`` and ``cargo`` must be installed.
 
+For Swift extraction:
+
+- Only macOS is supported as a host platform.
+
 For Java extraction:
 
 - There must be a ``java`` or ``java.exe`` executable available on the ``PATH``, and the ``JAVA_HOME`` environment variable must point to the corresponding JDK's home directory.

docs/codeql/reusables/supported-versions-compilers.rst

@@ -26,8 +26,8 @@
    Python [9]_,"2.7, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10, 3.11, 3.12, 3.13",Not applicable,``.py``
    Ruby [10]_,"up to 3.3",Not applicable,"``.rb``, ``.erb``, ``.gemspec``, ``Gemfile``"
    Rust [11]_,"Rust editions 2021 and 2024","Rust compiler","``.rs``, ``Cargo.toml``"
-   Swift [12]_,"Swift 5.4-6.1","Swift compiler","``.swift``"
+   Swift [12]_ [13]_,"Swift 5.4-6.1","Swift compiler","``.swift``"
-   TypeScript [13]_,"2.6-5.9",Standard TypeScript compiler,"``.ts``, ``.tsx``, ``.mts``, ``.cts``"
+   TypeScript [14]_,"2.6-5.9",Standard TypeScript compiler,"``.ts``, ``.tsx``, ``.mts``, ``.cts``"
 
 .. container:: footnote-group
 
@@ -43,4 +43,5 @@
     .. [10] Requires glibc 2.17.
     .. [11] Requires ``rustup`` and ``cargo`` to be installed. Features from nightly toolchains are not supported.
     .. [12] Support for the analysis of Swift requires macOS.
-    .. [13] TypeScript analysis is performed by running the JavaScript extractor with TypeScript enabled. This is the default.
+    .. [13] Embedded Swift is not supported.
+    .. [14] TypeScript analysis is performed by running the JavaScript extractor with TypeScript enabled. This is the default.

go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql

@@ -132,7 +132,7 @@ module UnhandledFileCloseConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSourceLocation(DataFlow::Node source) {
-    exists(DataFlow::CallNode openCall | result = openCall.getLocation() |
+    exists(DataFlow::CallNode openCall | result = [openCall.getLocation(), source.getLocation()] |
       isWritableFileHandle(source, openCall)
     )
   }

go/ql/src/Security/CWE-326/InsufficientKeySize.ql

@@ -27,8 +27,6 @@ module Config implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 /**

go/ql/src/experimental/CWE-369/DivideByZero.ql

@@ -47,8 +47,6 @@ module Config implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 /**

go/ql/src/experimental/CWE-918/SSRF.qll

@@ -33,9 +33,9 @@ module ServerSideRequestForgery {
 
     predicate observeDiffInformedIncrementalMode() { any() }
 
-    Location getASelectedSourceLocation(DataFlow::Node source) { none() }
-
     Location getASelectedSinkLocation(DataFlow::Node sink) {
+      result = sink.(Sink).getLocation()
+      or
       result = sink.(Sink).getARequest().getLocation()
     }
   }

java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected

@@ -67,15 +67,18 @@ ql/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.ql
 ql/java/ql/src/Likely Bugs/Concurrency/DateFormatThreadUnsafe.ql
 ql/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLocking.ql
 ql/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLockingWithInitRace.ql
+ql/java/ql/src/Likely Bugs/Concurrency/Escaping.ql
 ql/java/ql/src/Likely Bugs/Concurrency/FutileSynchOnField.ql
 ql/java/ql/src/Likely Bugs/Concurrency/NonSynchronizedOverride.ql
 ql/java/ql/src/Likely Bugs/Concurrency/NotifyNotNotifyAll.ql
+ql/java/ql/src/Likely Bugs/Concurrency/SafePublication.ql
 ql/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql
 ql/java/ql/src/Likely Bugs/Concurrency/SleepWithLock.ql
 ql/java/ql/src/Likely Bugs/Concurrency/StartInConstructor.ql
 ql/java/ql/src/Likely Bugs/Concurrency/SynchOnBoxedType.ql
 ql/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql
 ql/java/ql/src/Likely Bugs/Concurrency/SynchWriteObject.ql
+ql/java/ql/src/Likely Bugs/Concurrency/ThreadSafe.ql
 ql/java/ql/src/Likely Bugs/Finalization/NullifiedSuperFinalize.ql
 ql/java/ql/src/Likely Bugs/Frameworks/JUnit/BadSuiteMethod.ql
 ql/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql

java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected

@@ -30,10 +30,13 @@ ql/java/ql/src/Likely Bugs/Comparison/WrongNanComparison.ql
 ql/java/ql/src/Likely Bugs/Concurrency/CallsToRunnableRun.ql
 ql/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLocking.ql
 ql/java/ql/src/Likely Bugs/Concurrency/DoubleCheckedLockingWithInitRace.ql
+ql/java/ql/src/Likely Bugs/Concurrency/Escaping.ql
 ql/java/ql/src/Likely Bugs/Concurrency/NonSynchronizedOverride.ql
+ql/java/ql/src/Likely Bugs/Concurrency/SafePublication.ql
 ql/java/ql/src/Likely Bugs/Concurrency/ScheduledThreadPoolExecutorZeroThread.ql
 ql/java/ql/src/Likely Bugs/Concurrency/SynchOnBoxedType.ql
 ql/java/ql/src/Likely Bugs/Concurrency/SynchSetUnsynchGet.ql
+ql/java/ql/src/Likely Bugs/Concurrency/ThreadSafe.ql
 ql/java/ql/src/Likely Bugs/Frameworks/JUnit/JUnit5MissingNestedAnnotation.ql
 ql/java/ql/src/Likely Bugs/Inheritance/NoNonFinalInConstructor.ql
 ql/java/ql/src/Likely Bugs/Likely Typos/ContainerSizeCmpZero.ql

java/ql/lib/semmle/code/java/Concurrency.qll

@@ -2,6 +2,57 @@ overlay[local?]
 module;
 
 import java
+import semmle.code.java.frameworks.Mockito
+
+/**
+ * A Java type representing a lock.
+ * We identify a lock type as one that has both `lock` and `unlock` methods.
+ */
+class LockType extends RefType {
+  LockType() {
+    this.getAMethod().hasName("lock") and
+    this.getAMethod().hasName("unlock")
+  }
+
+  /** Gets a method that is locking this lock type. */
+  private Method getLockMethod() {
+    result.getDeclaringType() = this and
+    result.hasName(["lock", "lockInterruptibly", "tryLock"])
+  }
+
+  /** Gets a method that is unlocking this lock type. */
+  private Method getUnlockMethod() {
+    result.getDeclaringType() = this and
+    result.hasName("unlock")
+  }
+
+  /** Gets an `isHeldByCurrentThread` method of this lock type. */
+  private Method getIsHeldByCurrentThreadMethod() {
+    result.getDeclaringType() = this and
+    result.hasName("isHeldByCurrentThread")
+  }
+
+  /** Gets a call to a method that is locking this lock type. */
+  MethodCall getLockAccess() {
+    result.getMethod() = this.getLockMethod() and
+    // Not part of a Mockito verification call
+    not result instanceof MockitoVerifiedMethodCall
+  }
+
+  /** Gets a call to a method that is unlocking this lock type. */
+  MethodCall getUnlockAccess() {
+    result.getMethod() = this.getUnlockMethod() and
+    // Not part of a Mockito verification call
+    not result instanceof MockitoVerifiedMethodCall
+  }
+
+  /** Gets a call to a method that checks if the lock is held by the current thread. */
+  MethodCall getIsHeldByCurrentThreadAccess() {
+    result.getMethod() = this.getIsHeldByCurrentThreadMethod() and
+    // Not part of a Mockito verification call
+    not result instanceof MockitoVerifiedMethodCall
+  }
+}
 
 /**
  * Holds if `e` is synchronized by a local synchronized statement `sync` on the variable `v`.
@@ -49,3 +100,136 @@ class SynchronizedCallable extends Callable {
     )
   }
 }
+
+/**
+ * This module provides predicates, chiefly `locallyMonitors`, to check if a given expression is synchronized on a specific monitor.
+ */
+module Monitors {
+  /**
+   * A monitor is any object that is used to synchronize access to a shared resource.
+   * This includes locks as well as variables used in synchronized blocks (including `this`).
+   */
+  newtype TMonitor =
+    /** Either a lock or a variable used in a synchronized block. */
+    TVariableMonitor(Variable v) {
+      v.getType() instanceof LockType or locallySynchronizedOn(_, _, v)
+    } or
+    /** An instance reference used as a monitor. */
+    TInstanceMonitor(RefType thisType) { locallySynchronizedOnThis(_, thisType) } or
+    /** A class used as a monitor. */
+    TClassMonitor(RefType classType) { locallySynchronizedOnClass(_, classType) }
+
+  /**
+   * A monitor is any object that is used to synchronize access to a shared resource.
+   * This includes locks as well as variables used in synchronized blocks (including `this`).
+   */
+  class Monitor extends TMonitor {
+    /** Gets the location of this monitor. */
+    abstract Location getLocation();
+
+    /** Gets a textual representation of this element. */
+    abstract string toString();
+  }
+
+  /**
+   * A variable used as a monitor.
+   * The variable is either a lock or is used in a synchronized block.
+   * E.g `synchronized (m) { ... }` or `m.lock();`
+   */
+  class VariableMonitor extends Monitor, TVariableMonitor {
+    override Location getLocation() { result = this.getVariable().getLocation() }
+
+    override string toString() { result = "VariableMonitor(" + this.getVariable().toString() + ")" }
+
+    /** Gets the variable being used as a monitor. */
+    Variable getVariable() { this = TVariableMonitor(result) }
+  }
+
+  /**
+   * An instance reference used as a monitor.
+   * Either via `synchronized (this) { ... }` or by marking a non-static method as `synchronized`.
+   */
+  class InstanceMonitor extends Monitor, TInstanceMonitor {
+    override Location getLocation() { result = this.getThisType().getLocation() }
+
+    override string toString() { result = "InstanceMonitor(" + this.getThisType().toString() + ")" }
+
+    /** Gets the instance reference being used as a monitor. */
+    RefType getThisType() { this = TInstanceMonitor(result) }
+  }
+
+  /**
+   * A class used as a monitor.
+   * This is achieved by marking a static method as `synchronized`.
+   */
+  class ClassMonitor extends Monitor, TClassMonitor {
+    override Location getLocation() { result = this.getClassType().getLocation() }
+
+    override string toString() { result = "ClassMonitor(" + this.getClassType().toString() + ")" }
+
+    /** Gets the class being used as a monitor. */
+    RefType getClassType() { this = TClassMonitor(result) }
+  }
+
+  /** Holds if the expression `e` is synchronized on the monitor `m`. */
+  predicate locallyMonitors(Expr e, Monitor m) {
+    exists(Variable v | v = m.(VariableMonitor).getVariable() |
+      locallyLockedOn(e, v)
+      or
+      locallySynchronizedOn(e, _, v)
+    )
+    or
+    locallySynchronizedOnThis(e, m.(InstanceMonitor).getThisType())
+    or
+    locallySynchronizedOnClass(e, m.(ClassMonitor).getClassType())
+  }
+
+  /** Gets the control flow node that must dominate `e` when `e` is synchronized on a lock. */
+  ControlFlowNode getNodeToBeDominated(Expr e) {
+    // If `e` is the LHS of an assignment, use the control flow node for the assignment
+    exists(Assignment asgn | asgn.getDest() = e | result = asgn.getControlFlowNode())
+    or
+    // if `e` is not the LHS of an assignment, use the default control flow node
+    not exists(Assignment asgn | asgn.getDest() = e) and
+    result = e.getControlFlowNode()
+  }
+
+  /** A field storing a lock. */
+  class LockField extends Field {
+    LockField() { this.getType() instanceof LockType }
+
+    /** Gets a call to a method locking the lock stored in this field. */
+    MethodCall getLockCall() {
+      result.getQualifier() = this.getRepresentative().getAnAccess() and
+      result = this.getType().(LockType).getLockAccess()
+    }
+
+    /** Gets a call to a method unlocking the lock stored in this field. */
+    MethodCall getUnlockCall() {
+      result.getQualifier() = this.getRepresentative().getAnAccess() and
+      result = this.getType().(LockType).getUnlockAccess()
+    }
+
+    /**
+     * Gets a variable representing this field.
+     * It can be the field itself or a local variable initialized to the field.
+     */
+    private Variable getRepresentative() {
+      result = this
+      or
+      result.getInitializer() = this.getAnAccess()
+    }
+  }
+
+  /** Holds if `e` is synchronized on the `Lock` `lock` by a locking call. */
+  predicate locallyLockedOn(Expr e, LockField lock) {
+    exists(MethodCall lockCall, MethodCall unlockCall |
+      lockCall = lock.getLockCall() and
+      unlockCall = lock.getUnlockCall()
+    |
+      dominates(lockCall.getControlFlowNode(), unlockCall.getControlFlowNode()) and
+      dominates(lockCall.getControlFlowNode(), getNodeToBeDominated(e)) and
+      postDominates(unlockCall.getControlFlowNode(), getNodeToBeDominated(e))
+    )
+  }
+}

java/ql/lib/semmle/code/java/ConflictingAccess.qll

@@ -0,0 +1,286 @@
+/**
+ * Provides classes and predicates for detecting conflicting accesses in the sense of the Java Memory Model.
+ */
+overlay[local?]
+module;
+
+import java
+import Concurrency
+
+/** Provides predicates, chiefly `isModifying`, to check if a given expression modifies a shared resource. */
+module Modification {
+  import semmle.code.java.dataflow.FlowSummary
+
+  /** Holds if the field access `a` modifies a shared resource. */
+  predicate isModifying(FieldAccess a) {
+    a.isVarWrite()
+    or
+    exists(Call c | c.(MethodCall).getQualifier() = a | isModifyingCall(c))
+    or
+    exists(ArrayAccess aa, Assignment asa | aa.getArray() = a | asa.getDest() = aa)
+  }
+
+  /** Holds if the call `c` modifies a shared resource. */
+  predicate isModifyingCall(Call c) {
+    exists(SummarizedCallable sc, string output | sc.getACall() = c |
+      sc.propagatesFlow(_, output, _, _) and
+      output.matches("Argument[this]%")
+    )
+  }
+}
+
+/** Holds if the type `t` is thread-safe. */
+predicate isThreadSafeType(Type t) {
+  t.(RefType).getSourceDeclaration().getName().matches(["Atomic%", "Concurrent%"])
+  or
+  t.(RefType).getSourceDeclaration().getName() = "ThreadLocal"
+  or
+  // Anything in `java.util.concurrent` is thread safe.
+  // See https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/package-summary.html#MemoryVisibility
+  t.(RefType).getPackage().getName() = "java.util.concurrent"
+  or
+  t instanceof ClassAnnotatedAsThreadSafe
+}
+
+/** Holds if the expression `e` is a thread-safe initializer. */
+private predicate isThreadSafeInitializer(Expr e) {
+  exists(string name |
+    e.(Call).getCallee().getSourceDeclaration().hasQualifiedName("java.util", "Collections", name)
+  |
+    name.matches("synchronized%")
+  )
+}
+
+/**
+ * A field that is exposed to potential data races.
+ * We require the field to be in a class that is annotated as `@ThreadSafe`.
+ */
+class ExposedField extends Field {
+  ExposedField() {
+    this.getDeclaringType() instanceof ClassAnnotatedAsThreadSafe and
+    not this.isVolatile() and
+    // field is not a lock
+    not this.getType() instanceof LockType and
+    // field is not thread-safe
+    not isThreadSafeType(this.getType()) and
+    not isThreadSafeType(this.getInitializer().getType()) and
+    // the initializer guarantees thread safety
+    not isThreadSafeInitializer(this.getInitializer())
+  }
+}
+
+/**
+ * A field access that is exposed to potential data races.
+ * We require the field to be in a class that is annotated as `@ThreadSafe`.
+ */
+class ExposedFieldAccess extends FieldAccess {
+  ExposedFieldAccess() {
+    // access is to an exposed field
+    this.getField() instanceof ExposedField and
+    // access is not the initializer of the field
+    not this.(VarWrite).getASource() = this.getField().getInitializer() and
+    // access not in a constructor
+    not this.getEnclosingCallable() = this.getField().getDeclaringType().getAConstructor() and
+    // not a field on a local variable
+    not this.getQualifier+().(VarAccess).getVariable() instanceof LocalVariableDecl and
+    // not the variable mentioned in a synchronized statement
+    not this = any(SynchronizedStmt sync).getExpr()
+  }
+}
+
+/**
+ * A class annotated as `@ThreadSafe`.
+ * Provides predicates to check for concurrency issues.
+ */
+class ClassAnnotatedAsThreadSafe extends Class {
+  ClassAnnotatedAsThreadSafe() { this.getAnAnnotation().getType().getName() = "ThreadSafe" }
+
+  // We wish to find conflicting accesses that are reachable from public methods
+  // and to know which monitors protect them.
+  //
+  // It is very easy and natural to write a predicate for conflicting accesses,
+  // but that would be binary, and hence not suited for reachability analysis.
+  //
+  // It is also easy to state that all accesses to a field are protected by a single monitor,
+  // but that would require a forall, which is not suited for recursion.
+  // (The recursion occurs for example as you traverse the access path and keep requiring that all tails are protected.)
+  //
+  // We therefore use a dual solution:
+  // - We write a unary recursive predicate for accesses that are not protected by any monitor.
+  //   Any such write access, reachable from a public method, is conflicting with itself.
+  //   And any such read will be conflicting with any publicly reachable write access (locked or not).
+  //
+  // - We project the above predicate down to fields, so we can find fields with unprotected accesses.
+  // - From this we can derive a unary recursive predicate for fields whose accesses are protected by exactly one monitor.
+  //   The predicate tracks the monitor.
+  //   If such a field has two accesses protected by different monitors, we have a concurrency issue.
+  //   This can be determined by simple counting at the end of the recursion.
+  //   Technically, we only have a concurrency issue if there is a write access,
+  //   but if you are locking your reads with different locks, you likely made a typo.
+  //
+  // - From the above, we can derive a unary recursive predicate for fields whose accesses are protected by at least one monitor.
+  //   This predicate tracks all the monitors that protect accesses to the field.
+  //   This is combined with a predicate that checks if any access escapes a given monitor.
+  //   If all the monitors that protect accesses to a field are escaped by at least one access,
+  //   we have a concurrency issue.
+  //   This can be determined by a single forall at the end of the recursion.
+  //
+  // With this formulation we avoid binary predicates and foralls in recursion.
+  //
+  // Cases where a field access is not protected by any monitor
+  /**
+   * Holds if the field access `a` to the field `f` is not protected by any monitor, and it can be reached via the expression `e` in the method `m`.
+   * We maintain the invariant that `m = e.getEnclosingCallable()`.
+   */
+  private predicate unlockedAccess(
+    ExposedField f, Expr e, Method m, ExposedFieldAccess a, boolean write
+  ) {
+    m.getDeclaringType() = this and
+    (
+      // base case
+      f.getDeclaringType() = this and
+      m = e.getEnclosingCallable() and
+      a.getField() = f and
+      a = e and
+      (if Modification::isModifying(a) then write = true else write = false)
+      or
+      // recursive case
+      exists(MethodCall c, Expr e0, Method m0 | this.unlockedAccess(f, e0, m0, a, write) |
+        m = c.getEnclosingCallable() and
+        not m0.isPublic() and
+        c.getCallee().getSourceDeclaration() = m0 and
+        c = e and
+        not Monitors::locallyMonitors(e0, _)
+      )
+    )
+  }
+
+  /** Holds if the class has an unlocked access to the field `f` via the expression `e` in the method `m`. */
+  private predicate hasUnlockedAccess(ExposedField f, Expr e, Method m, boolean write) {
+    this.unlockedAccess(f, e, m, _, write)
+  }
+
+  /** Holds if the field access `a` to the field `f` is not protected by any monitor, and it can be reached via the expression `e` in the public method `m`. */
+  predicate unlockedPublicAccess(
+    ExposedField f, Expr e, Method m, ExposedFieldAccess a, boolean write
+  ) {
+    this.unlockedAccess(f, e, m, a, write) and
+    m.isPublic() and
+    not Monitors::locallyMonitors(e, _)
+  }
+
+  /** Holds if the class has an unlocked access to the field `f` via the expression `e` in the public method `m`. */
+  private predicate hasUnlockedPublicAccess(ExposedField f, Expr e, Method m, boolean write) {
+    this.unlockedPublicAccess(f, e, m, _, write)
+  }
+
+  // Cases where all accesses to a field are protected by exactly one monitor
+  //
+  /**
+   * Holds if the class has an access, locked by exactly one monitor, to the field `f` via the expression `e` in the method `m`.
+   */
+  private predicate hasOnelockedAccess(
+    ExposedField f, Expr e, Method m, boolean write, Monitors::Monitor monitor
+  ) {
+    //base
+    this.hasUnlockedAccess(f, e, m, write) and
+    Monitors::locallyMonitors(e, monitor)
+    or
+    // recursive case
+    exists(MethodCall c, Method m0 | this.hasOnelockedAccess(f, _, m0, write, monitor) |
+      m = c.getEnclosingCallable() and
+      not m0.isPublic() and
+      c.getCallee().getSourceDeclaration() = m0 and
+      c = e and
+      // consider allowing idempotent monitors
+      not Monitors::locallyMonitors(e, _) and
+      m.getDeclaringType() = this
+    )
+  }
+
+  /** Holds if the class has an access, locked by exactly one monitor, to the field `f` via the expression `e` in the public method `m`. */
+  private predicate hasOnelockedPublicAccess(
+    ExposedField f, Expr e, Method m, boolean write, Monitors::Monitor monitor
+  ) {
+    this.hasOnelockedAccess(f, e, m, write, monitor) and
+    m.isPublic() and
+    not this.hasUnlockedPublicAccess(f, e, m, write)
+  }
+
+  /** Holds if the field `f` has more than one access, all locked by a single monitor, but different monitors are used. */
+  predicate singleMonitorMismatch(ExposedField f) {
+    2 <= strictcount(Monitors::Monitor monitor | this.hasOnelockedPublicAccess(f, _, _, _, monitor))
+  }
+
+  // Cases where all accesses to a field are protected by at least one monitor
+  //
+  /** Holds if the class has an access, locked by at least one monitor, to the field `f` via the expression `e` in the method `m`. */
+  private predicate hasOnepluslockedAccess(
+    ExposedField f, Expr e, Method m, boolean write, Monitors::Monitor monitor
+  ) {
+    //base
+    this.hasOnelockedAccess(f, e, m, write, monitor) and
+    not this.singleMonitorMismatch(f) and
+    not this.hasUnlockedPublicAccess(f, _, _, _)
+    or
+    // recursive case
+    exists(MethodCall c, Method m0, Monitors::Monitor monitor0 |
+      this.hasOnepluslockedAccess(f, _, m0, write, monitor0) and
+      m = c.getEnclosingCallable() and
+      not m0.isPublic() and
+      c.getCallee().getSourceDeclaration() = m0 and
+      c = e and
+      m.getDeclaringType() = this
+    |
+      monitor = monitor0
+      or
+      Monitors::locallyMonitors(e, monitor)
+    )
+  }
+
+  /** Holds if the class has a write access to the field `f` that can be reached via a public method. */
+  predicate hasPublicWriteAccess(ExposedField f) {
+    this.hasUnlockedPublicAccess(f, _, _, true)
+    or
+    this.hasOnelockedPublicAccess(f, _, _, true, _)
+    or
+    exists(Method m | m.getDeclaringType() = this and m.isPublic() |
+      this.hasOnepluslockedAccess(f, _, m, true, _)
+    )
+  }
+
+  /** Holds if the class has an access, not protected by the monitor `m`, to the field `f` via the expression `e` in the method `m`. */
+  private predicate escapesMonitor(
+    ExposedField f, Expr e, Method m, boolean write, Monitors::Monitor monitor
+  ) {
+    //base
+    this.hasOnepluslockedAccess(f, _, _, _, monitor) and
+    this.hasUnlockedAccess(f, e, m, write) and
+    not Monitors::locallyMonitors(e, monitor)
+    or
+    // recursive case
+    exists(MethodCall c, Method m0 | this.escapesMonitor(f, _, m0, write, monitor) |
+      m = c.getEnclosingCallable() and
+      not m0.isPublic() and
+      c.getCallee().getSourceDeclaration() = m0 and
+      c = e and
+      not Monitors::locallyMonitors(e, monitor) and
+      m.getDeclaringType() = this
+    )
+  }
+
+  /** Holds if the class has an access, not protected by the monitor `m`, to the field `f` via the expression `e` in the public method `m`. */
+  private predicate escapesMonitorPublic(
+    ExposedField f, Expr e, Method m, boolean write, Monitors::Monitor monitor
+  ) {
+    this.escapesMonitor(f, e, m, write, monitor) and
+    m.isPublic()
+  }
+
+  /** Holds if no monitor protects all accesses to the field `f`. */
+  predicate notFullyMonitored(ExposedField f) {
+    forex(Monitors::Monitor monitor | this.hasOnepluslockedAccess(f, _, _, _, monitor) |
+      this.escapesMonitorPublic(f, _, _, _, monitor)
+    )
+  }
+}

java/ql/lib/semmle/code/java/security/ArbitraryApkInstallationQuery.qll

@@ -25,8 +25,6 @@ module ApkInstallationConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
 }
 
 module ApkInstallationFlow = DataFlow::Global<ApkInstallationConfig>;

java/ql/lib/semmle/code/java/security/ArithmeticTaintedQuery.qll

@@ -19,7 +19,9 @@ module ArithmeticOverflowConfig implements DataFlow::ConfigSig {
   }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(ArithExpr exp | result = exp.getLocation() | overflowSink(exp, sink.asExpr()))
+    exists(ArithExpr exp | result = [exp.getLocation(), sink.getLocation()] |
+      overflowSink(exp, sink.asExpr())
+    )
   }
 }
 
@@ -43,7 +45,9 @@ module ArithmeticUnderflowConfig implements DataFlow::ConfigSig {
   }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(ArithExpr exp | result = exp.getLocation() | underflowSink(exp, sink.asExpr()))
+    exists(ArithExpr exp | result = [exp.getLocation(), sink.getLocation()] |
+      underflowSink(exp, sink.asExpr())
+    )
   }
 }
 

java/ql/lib/semmle/code/java/security/ArithmeticUncontrolledQuery.qll

@@ -25,7 +25,9 @@ module ArithmeticUncontrolledOverflowConfig implements DataFlow::ConfigSig {
   }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(ArithExpr exp | result = exp.getLocation() | overflowSink(exp, sink.asExpr()))
+    exists(ArithExpr exp | result = [exp.getLocation(), sink.getLocation()] |
+      overflowSink(exp, sink.asExpr())
+    )
   }
 }
 
@@ -46,7 +48,9 @@ module ArithmeticUncontrolledUnderflowConfig implements DataFlow::ConfigSig {
   }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(ArithExpr exp | result = exp.getLocation() | underflowSink(exp, sink.asExpr()))
+    exists(ArithExpr exp | result = [exp.getLocation(), sink.getLocation()] |
+      underflowSink(exp, sink.asExpr())
+    )
   }
 }
 

java/ql/lib/semmle/code/java/security/BrokenCryptoAlgorithmQuery.qll

@@ -36,7 +36,11 @@ module InsecureCryptoConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(CryptoAlgoSpec c | sink.asExpr() = c.getAlgoSpec() | result = c.getLocation())
+    exists(CryptoAlgoSpec c | sink.asExpr() = c.getAlgoSpec() |
+      result = c.getLocation()
+      or
+      result = sink.getLocation()
+    )
   }
 }
 

java/ql/lib/semmle/code/java/security/CommandLineQuery.qll

@@ -63,10 +63,14 @@ module InputToArgumentToExecFlowConfig implements DataFlow::ConfigSig {
   // only to prevent overlapping results between two queries.
   predicate observeDiffInformedIncrementalMode() { any() }
 
-  // All queries use the argument as the primary location and do not use the
+  // ExecTainted.ql queries use the argument as the primary location;
-  // sink as an associated location.
+  // ExecUnescaped.ql does not (used to prevent overlapping results).
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(Expr argument | argumentToExec(argument, sink) | result = argument.getLocation())
+    exists(Expr argument | argumentToExec(argument, sink) |
+      result = argument.getLocation()
+      or
+      result = sink.getLocation()
+    )
   }
 }
 

java/ql/lib/semmle/code/java/security/ConditionalBypassQuery.qll

@@ -51,7 +51,7 @@ module ConditionalBypassFlowConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(MethodCall m, Expr e | result = [m, e].getLocation() |
+    exists(MethodCall m, Expr e | result = [[m, e].getLocation(), sink.getLocation()] |
       conditionControlsMethod(m, e) and
       sink.asExpr() = e
     )

java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayConstructionCodeSpecifiedQuery.qll

@@ -22,7 +22,10 @@ module BoundedFlowSourceConfig implements DataFlow::ConfigSig {
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     exists(ArrayCreationExpr arrayCreation, CheckableArrayAccess arrayAccess |
-      result = [arrayCreation, arrayAccess.getIndexExpr()].getLocation() and
+      result = [arrayCreation, arrayAccess.getIndexExpr()].getLocation()
+      or
+      result = sink.getLocation()
+    |
       arrayAccess.canThrowOutOfBoundsDueToEmptyArray(sink.asExpr(), arrayCreation)
     )
   }

java/ql/lib/semmle/code/java/security/ImproperValidationOfArrayConstructionQuery.qll

@@ -19,7 +19,10 @@ module ImproperValidationOfArrayConstructionConfig implements DataFlow::ConfigSi
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     exists(ArrayCreationExpr arrayCreation, CheckableArrayAccess arrayAccess |
-      result = [arrayCreation, arrayAccess.getIndexExpr()].getLocation() and
+      result = [arrayCreation, arrayAccess.getIndexExpr()].getLocation()
+      or
+      result = sink.getLocation()
+    |
       arrayAccess.canThrowOutOfBoundsDueToEmptyArray(sink.asExpr(), arrayCreation)
     )
   }

java/ql/lib/semmle/code/java/security/MaybeBrokenCryptoAlgorithmQuery.qll

@@ -81,7 +81,9 @@ module InsecureCryptoConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(CryptoAlgoSpec c | result = c.getLocation() | sink.asExpr() = c.getAlgoSpec())
+    exists(CryptoAlgoSpec c | result = sink.getLocation() or result = c.getLocation() |
+      sink.asExpr() = c.getAlgoSpec()
+    )
   }
 }
 

java/ql/lib/semmle/code/java/security/NumericCastTaintedQuery.qll

@@ -106,8 +106,9 @@ module NumericCastFlowConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(NumericNarrowingCastExpr cast |
+    exists(NumericNarrowingCastExpr cast | cast.getExpr() = sink.asExpr() |
-      cast.getExpr() = sink.asExpr() and
+      result = sink.getLocation()
+      or
       result = cast.getLocation()
     )
   }

java/ql/lib/semmle/code/java/security/TaintedEnvironmentVariableQuery.qll

@@ -40,8 +40,6 @@ module ExecTaintedEnvironmentConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
 }
 
 /**

java/ql/lib/semmle/code/java/security/TaintedPermissionsCheckQuery.qll

@@ -63,8 +63,9 @@ module TaintedPermissionsCheckFlowConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(PermissionsConstruction p |
+    exists(PermissionsConstruction p | sink.asExpr() = p.getInput() |
-      sink.asExpr() = p.getInput() and
+      result = sink.getLocation()
+      or
       result = p.getLocation()
     )
   }

java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll

@@ -147,8 +147,6 @@ module TempDirSystemGetPropertyToCreateConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSinkLocation(DataFlow::Node sink) { none() }
 }
 
 /**

java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll

@@ -313,6 +313,8 @@ private module UnsafeDeserializationConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(UnsafeDeserializationSink).getLocation()
+    or
     result = sink.(UnsafeDeserializationSink).getMethodCall().getLocation()
   }
 }

java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll

@@ -46,12 +46,6 @@ module WebviewDebugEnabledConfig implements DataFlow::ConfigSig {
   }
 
   predicate observeDiffInformedIncrementalMode() { any() }
-
-  Location getASelectedSourceLocation(DataFlow::Node source) {
-    // This module is only used in `WebviewDebuggingEnabled.ql`, which doesn't
-    // select the source in any "$@" column.
-    none()
-  }
 }
 
 /**

java/ql/src/Likely Bugs/Concurrency/Escaping.qhelp

@@ -0,0 +1,34 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>
+In a thread-safe class, non-final fields should generally be private (or possibly volatile) to ensure that they cannot be accessed by other threads in an unsafe manner.
+</p>
+
+</overview>
+<recommendation>
+
+<p>
+If the field does not change, mark it as <code>final</code>. If the field is mutable, mark it as <code>private</code> and provide properly synchronized accessors.</p>
+
+</recommendation>
+
+<references>
+
+
+<li>
+  Java Language Specification, chapter 17:
+  <a href="https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4">Threads and Locks</a>.
+</li>
+<li>
+  Java concurrency package:
+  <a href="https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/package-summary.html">java.util.concurrent</a>.
+</li>
+
+
+</references>
+</qhelp>

java/ql/src/Likely Bugs/Concurrency/Escaping.ql

@@ -0,0 +1,26 @@
+/**
+ * @name Escaping
+ * @description In a thread-safe class, care should be taken to avoid exposing mutable state.
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id java/escaping
+ * @tags quality
+ *       reliability
+ *       concurrency
+ */
+
+import java
+import semmle.code.java.ConflictingAccess
+
+from Field f, ClassAnnotatedAsThreadSafe c
+where
+  f = c.getAField() and
+  not f.isFinal() and // final fields do not change
+  not f.isPrivate() and
+  // We believe that protected fields are also dangerous
+  // Volatile fields cannot cause data races, but it is dubious to allow changes.
+  // For now, we ignore volatile fields, but there are likely bugs to be caught here.
+  not f.isVolatile()
+select f, "The class $@ is marked as thread-safe, but this field is potentially escaping.", c,
+  c.getName()

java/ql/src/Likely Bugs/Concurrency/SafePublication.java

@@ -0,0 +1,17 @@
+public class SafePublication {
+    private volatile Object value;
+    private final int server_id;
+
+    public SafePublication() {
+        value = new Object(); // Safely published as volatile
+        server_id = 1; // Safely published as final
+    }
+
+    public synchronized Object getValue() {
+        return value;
+    }
+
+    public int getServerId() {
+        return server_id;
+    }
+}

java/ql/src/Likely Bugs/Concurrency/SafePublication.qhelp

@@ -0,0 +1,57 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>
+In a thread-safe class, values must be published safely to avoid inconsistent or unexpected behavior caused by visibility issues between threads. If a value is not safely published, one thread may see a stale or partially constructed value written by another thread, leading to subtle concurrency bugs.
+</p>
+<p>
+In particular, values of primitive types should not be initialised to anything but their default values (which for <code>Object</code> is <code>null</code>) unless this happens in a static context.
+</p>
+<p>
+Techniques for safe publication include:
+</p>
+<ul>
+<li>Using synchronized blocks or methods to ensure that a value is fully constructed before it is published.</li>
+<li>Using volatile fields to ensure visibility of changes across threads.</li>
+<li>Using thread-safe collections or classes that provide built-in synchronization, such as are found in <code>java.util.concurrent</code>.</li>
+<li>Using the <code>final</code> keyword to ensure that a reference to an object is safely published when the object is constructed.</li>
+</ul>
+
+</overview>
+<recommendation>
+
+<p>
+Choose a safe publication technique that fits your use case. If the value only needs to be written once, say for a singleton, consider using the <code>final</code> keyword. If the value is mutable and needs to be shared across threads, consider using synchronized blocks or methods, or using a thread-safe collection from <code>java.util.concurrent</code>.
+</p>
+
+</recommendation>
+<example>
+
+<p>In the following example, the values of <code>value</code> and <code>server_id</code> are not safely published. The constructor creates a new object and assigns it to the field <code>value</code>. However, the field is not declared as <code>volatile</code> or <code>final</code>, and there are no synchronization mechanisms in place to ensure that the value is fully constructed before it is published. A different thread may see the default value <code>null</code>. Similarly, the field <code>server_id</code> may be observed to be <code>0</code>.</p>
+
+<sample src="UnsafePublication.java" />
+
+<p>To fix this example, we declare the field <code>value</code> as volatile. This will ensure that all changes to the field are visible to all threads. The field <code>server_id</code> is only meant to be written once, so we only need the write inside the constructor to be visible to other threads; declaring it <code>final</code> guarantees this:</p>
+
+<sample src="SafePublication.java" />
+
+</example>
+<references>
+
+
+<li>
+  Java Language Specification, chapter 17:
+  <a href="https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4">Threads and Locks</a>.
+</li>
+<li>
+  Java concurrency package:
+  <a href="https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/package-summary.html">java.util.concurrent</a>.
+</li>
+
+
+</references>
+</qhelp>

java/ql/src/Likely Bugs/Concurrency/SafePublication.ql

@@ -0,0 +1,93 @@
+/**
+ * @name Safe publication
+ * @description A field of a thread-safe class is not safely published.
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id java/safe-publication
+ * @tags quality
+ *       reliability
+ *       concurrency
+ */
+
+import java
+import semmle.code.java.ConflictingAccess
+
+/**
+ * Holds if `v` should be the default value for the field `f`.
+ * That is, `v` is an initial (or constructor) assignment of `f`.
+ */
+predicate shouldBeDefaultValueFor(Field f, Expr v) {
+  v = f.getAnAssignedValue() and
+  (
+    v = f.getInitializer()
+    or
+    v.getEnclosingCallable() = f.getDeclaringType().getAConstructor()
+  )
+}
+
+/**
+ * Gets the default value for the field `f`.
+ * See https://docs.oracle.com/javase/tutorial/java/nutsandbolts/datatypes.html
+ * for the default values of the primitive types.
+ * The default value for non-primitive types is null.
+ */
+bindingset[result]
+Expr getDefaultValue(Field f) {
+  f.getType().hasName("byte") and result.(IntegerLiteral).getIntValue() = 0
+  or
+  f.getType().hasName("short") and result.(IntegerLiteral).getIntValue() = 0
+  or
+  f.getType().hasName("int") and result.(IntegerLiteral).getIntValue() = 0
+  or
+  f.getType().hasName("long") and
+  (
+    result.(LongLiteral).getValue() = "0" or
+    result.(IntegerLiteral).getValue() = "0"
+  )
+  or
+  f.getType().hasName("float") and result.(FloatLiteral).getValue() = "0.0"
+  or
+  f.getType().hasName("double") and result.(DoubleLiteral).getValue() = "0.0"
+  or
+  f.getType().hasName("char") and result.(CharacterLiteral).getCodePointValue() = 0
+  or
+  f.getType().hasName("boolean") and result.(BooleanLiteral).getBooleanValue() = false
+  or
+  not f.getType().getName() in [
+      "byte", "short", "int", "long", "float", "double", "char", "boolean"
+    ] and
+  result instanceof NullLiteral
+}
+
+/**
+ * Holds if all constructor or initial assignments (if any) are to the default value.
+ * That is, assignments by the declaration:
+ *   int x = 0; OK
+ *   int x = 3; not OK
+ * or inside a constructor:
+ *   public c(a) {
+ *     x = 0; OK
+ *     x = 3; not OK
+ *     x = a; not OK
+ *   }
+ */
+predicate isAssignedDefaultValue(Field f) {
+  forall(Expr v | shouldBeDefaultValueFor(f, v) | v = getDefaultValue(f))
+}
+
+predicate isSafelyPublished(Field f) {
+  f.isFinal() or // NOTE: For non-primitive types, 'final' alone does not guarantee safe publication unless the object is immutable or safely constructed. Consider reviewing the handling of non-primitive fields for safe publication.
+  f.isStatic() or
+  f.isVolatile() or
+  isThreadSafeType(f.getType()) or
+  isThreadSafeType(f.getInitializer().getType()) or
+  isAssignedDefaultValue(f)
+}
+
+from Field f, ClassAnnotatedAsThreadSafe c
+where
+  f = c.getAField() and
+  not isSafelyPublished(f)
+select f, "The class $@ is marked as thread-safe, but this field is not safely published.", c,
+  c.getName()

java/ql/src/Likely Bugs/Concurrency/ThreadSafe.qhelp

@@ -0,0 +1,33 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>
+In a thread-safe class, all field accesses that can be caused by calls to public methods must be properly synchronized.</p>
+
+</overview>
+<recommendation>
+
+<p>
+Protect the field access with a lock. Alternatively mark the field as <code>volatile</code> if the write operation is atomic. You can also choose to use a data type that guarantees atomic access. If the field is immutable, mark it as <code>final</code>.</p>
+
+</recommendation>
+
+<references>
+
+
+<li>
+  Java Language Specification, chapter 17:
+  <a href="https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4">Threads and Locks</a>.
+</li>
+<li>
+  Java concurrency package:
+  <a href="https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/package-summary.html">java.util.concurrent</a>.
+</li>
+
+
+</references>
+</qhelp>

java/ql/src/Likely Bugs/Concurrency/ThreadSafe.ql

@@ -0,0 +1,51 @@
+/**
+ * @name Not thread-safe
+ * @description This class is not thread-safe. It is annotated as `@ThreadSafe`, but it has a
+ *              conflicting access to a field that is not synchronized with the same monitor.
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id java/not-threadsafe
+ * @tags quality
+ *       reliability
+ *       concurrency
+ */
+
+import java
+import semmle.code.java.ConflictingAccess
+
+predicate unmonitoredAccess(ExposedFieldAccess a, string msg, Expr entry, string entry_desc) {
+  exists(ClassAnnotatedAsThreadSafe cls, ExposedField f |
+    cls.unlockedPublicAccess(f, entry, _, a, true)
+    or
+    cls.unlockedPublicAccess(f, entry, _, a, false) and
+    cls.hasPublicWriteAccess(f)
+  ) and
+  msg =
+    "This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe." and
+  entry_desc = "this expression"
+}
+
+predicate notFullyMonitoredField(
+  ExposedField f, string msg, ClassAnnotatedAsThreadSafe cls, string cls_name
+) {
+  (
+    // Technically there has to be a write access for a conflict to exist.
+    // But if you are locking your reads with different locks, you likely made a typo,
+    // so in this case we alert without requiring `cls.has_public_write_access(f)`
+    cls.singleMonitorMismatch(f)
+    or
+    cls.notFullyMonitored(f) and
+    cls.hasPublicWriteAccess(f)
+  ) and
+  msg =
+    "This field is not properly synchronized in that no single monitor covers all accesses, but the class $@ is annotated as @ThreadSafe." and
+  cls_name = cls.getName()
+}
+
+from Top alert_element, Top alert_context, string alert_msg, string context_desc
+where
+  unmonitoredAccess(alert_element, alert_msg, alert_context, context_desc)
+  or
+  notFullyMonitoredField(alert_element, alert_msg, alert_context, context_desc)
+select alert_element, alert_msg, alert_context, context_desc

java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.ql

@@ -16,47 +16,7 @@
 import java
 import semmle.code.java.controlflow.Guards
 import semmle.code.java.dataflow.SSA
-import semmle.code.java.frameworks.Mockito
+import semmle.code.java.Concurrency
-
-class LockType extends RefType {
-  LockType() {
-    this.getAMethod().hasName("lock") and
-    this.getAMethod().hasName("unlock")
-  }
-
-  Method getLockMethod() {
-    result.getDeclaringType() = this and
-    (result.hasName("lock") or result.hasName("tryLock"))
-  }
-
-  Method getUnlockMethod() {
-    result.getDeclaringType() = this and
-    result.hasName("unlock")
-  }
-
-  Method getIsHeldByCurrentThreadMethod() {
-    result.getDeclaringType() = this and
-    result.hasName("isHeldByCurrentThread")
-  }
-
-  MethodCall getLockAccess() {
-    result.getMethod() = this.getLockMethod() and
-    // Not part of a Mockito verification call
-    not result instanceof MockitoVerifiedMethodCall
-  }
-
-  MethodCall getUnlockAccess() {
-    result.getMethod() = this.getUnlockMethod() and
-    // Not part of a Mockito verification call
-    not result instanceof MockitoVerifiedMethodCall
-  }
-
-  MethodCall getIsHeldByCurrentThreadAccess() {
-    result.getMethod() = this.getIsHeldByCurrentThreadMethod() and
-    // Not part of a Mockito verification call
-    not result instanceof MockitoVerifiedMethodCall
-  }
-}
 
 predicate lockBlock(LockType t, BasicBlock b, int locks) {
   locks = strictcount(int i | b.getNode(i).asExpr() = t.getLockAccess())

java/ql/src/Likely Bugs/Concurrency/UnsafePublication.java

@@ -0,0 +1,17 @@
+public class UnsafePublication {
+    private Object value;
+    private int server_id;
+
+    public UnsafePublication() {
+        value = new Object(); // Not safely published, other threads may see the default value null
+        server_id = 1; // Not safely published, other threads may see the default value 0
+    }
+
+    public Object getValue() {
+        return value;
+    }
+
+    public int getServerId() {
+        return server_id;
+    }
+}

java/ql/src/change-notes/2025-06-22-query-escaping.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `java/escaping`, to detect values escaping from classes marked as `@ThreadSafe`.

java/ql/src/change-notes/2025-06-22-query-not-thread-safe.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `java/not-threadsafe`, to detect data races in classes marked as `@ThreadSafe`.

java/ql/src/change-notes/2025-06-22-query-safe-publication.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `java/safe-publication`, to detect unsafe publication in classes marked as `@ThreadSafe`.

java/ql/test/query-tests/Escaping/Escaping.expected

@@ -0,0 +1,3 @@
+| Escaping.java:3:7:3:7 | x | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |
+| Escaping.java:4:14:4:14 | y | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |
+| Escaping.java:9:18:9:18 | b | The class $@ is marked as thread-safe, but this field is potentially escaping. | Escaping.java:2:14:2:21 | Escaping | Escaping |

java/ql/test/query-tests/Escaping/Escaping.java

@@ -0,0 +1,17 @@
+@ThreadSafe
+public class Escaping {
+  int x;  //$ Alert
+  public int y = 0;  //$ Alert
+  private int z = 3;
+  final int w = 0;
+  public final int u = 4;
+  private final long a = 5;
+  protected long b = 0;  //$ Alert
+  protected final long c = 0L;
+  volatile long d = 3;
+  protected volatile long e = 3L;
+
+  public void methodLocal() {
+    int i;
+  }
+}

java/ql/test/query-tests/Escaping/Escaping.qlref

@@ -0,0 +1,2 @@
+query: Likely Bugs/Concurrency/Escaping.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

java/ql/test/query-tests/Escaping/ThreadSafe.java

@@ -0,0 +1,2 @@
+public @interface ThreadSafe {
+}

java/ql/test/query-tests/SafePublication/SafePublication.expected

@@ -0,0 +1,7 @@
+| SafePublication.java:5:9:5:9 | z | The class $@ is marked as thread-safe, but this field is not safely published. | SafePublication.java:2:14:2:28 | SafePublication | SafePublication |
+| SafePublication.java:6:9:6:9 | w | The class $@ is marked as thread-safe, but this field is not safely published. | SafePublication.java:2:14:2:28 | SafePublication | SafePublication |
+| SafePublication.java:7:9:7:9 | u | The class $@ is marked as thread-safe, but this field is not safely published. | SafePublication.java:2:14:2:28 | SafePublication | SafePublication |
+| SafePublication.java:11:10:11:10 | d | The class $@ is marked as thread-safe, but this field is not safely published. | SafePublication.java:2:14:2:28 | SafePublication | SafePublication |
+| SafePublication.java:12:10:12:10 | e | The class $@ is marked as thread-safe, but this field is not safely published. | SafePublication.java:2:14:2:28 | SafePublication | SafePublication |
+| SafePublication.java:14:11:14:13 | arr | The class $@ is marked as thread-safe, but this field is not safely published. | SafePublication.java:2:14:2:28 | SafePublication | SafePublication |
+| SafePublication.java:17:10:17:11 | cc | The class $@ is marked as thread-safe, but this field is not safely published. | SafePublication.java:2:14:2:28 | SafePublication | SafePublication |

java/ql/test/query-tests/SafePublication/SafePublication.java

@@ -0,0 +1,29 @@
+@ThreadSafe
+public class SafePublication {
+    int x;
+    int y = 0;
+    int z = 3;   //$ Alert
+    int w;       //$ Alert
+    int u;       //$ Alert
+    long a;
+    long b = 0;
+    long c = 0L;
+    long d = 3;  //$ Alert
+    long e = 3L; //$ Alert
+
+    int[] arr = new int[3]; //$ Alert
+    float f = 0.0f;
+    double dd = 00.0d;
+    char cc = 'a';  //$ Alert
+    char ok = '\u0000';
+
+ public SafePublication(int a) {
+    x = 0;
+    w = 3; // not ok
+    u = a; // not ok
+  }
+
+  public void methodLocal() {
+    int i;
+  }
+}

java/ql/test/query-tests/SafePublication/SafePublication.qlref

@@ -0,0 +1,2 @@
+query: Likely Bugs/Concurrency/SafePublication.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

java/ql/test/query-tests/SafePublication/ThreadSafe.java

@@ -0,0 +1,2 @@
+public @interface ThreadSafe {
+}

java/ql/test/query-tests/ThreadSafe/ThreadSafe.expected

@@ -0,0 +1,45 @@
+| examples/C.java:14:9:14:14 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/C.java:14:9:14:14 | this.y | this expression |
+| examples/C.java:15:9:15:14 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/C.java:15:9:15:14 | this.y | this expression |
+| examples/C.java:16:9:16:14 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/C.java:16:9:16:14 | this.y | this expression |
+| examples/C.java:16:18:16:23 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/C.java:16:18:16:23 | this.y | this expression |
+| examples/C.java:20:9:20:14 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/C.java:20:9:20:14 | this.y | this expression |
+| examples/C.java:21:9:21:14 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/C.java:21:9:21:14 | this.y | this expression |
+| examples/C.java:22:9:22:14 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/C.java:22:9:22:14 | this.y | this expression |
+| examples/C.java:22:18:22:23 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/C.java:22:18:22:23 | this.y | this expression |
+| examples/C.java:26:9:26:14 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/C.java:26:9:26:14 | this.y | this expression |
+| examples/C.java:30:13:30:13 | y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/C.java:30:13:30:13 | y | this expression |
+| examples/C.java:33:9:33:9 | y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/C.java:33:9:33:9 | y | this expression |
+| examples/DeepPaths.java:8:17:8:17 | y | This field is not properly synchronized in that no single monitor covers all accesses, but the class $@ is annotated as @ThreadSafe. | examples/DeepPaths.java:7:14:7:22 | DeepPaths | DeepPaths |
+| examples/FaultyTurnstileExample.java:18:5:18:9 | count | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/FaultyTurnstileExample.java:18:5:18:9 | count | this expression |
+| examples/FaultyTurnstileExample.java:26:15:26:19 | count | This field is not properly synchronized in that no single monitor covers all accesses, but the class $@ is annotated as @ThreadSafe. | examples/FaultyTurnstileExample.java:23:7:23:29 | FaultyTurnstileExample2 | FaultyTurnstileExample2 |
+| examples/FlawedSemaphore.java:15:14:15:18 | state | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/FlawedSemaphore.java:15:14:15:18 | state | this expression |
+| examples/FlawedSemaphore.java:18:7:18:11 | state | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/FlawedSemaphore.java:18:7:18:11 | state | this expression |
+| examples/LockExample.java:18:15:18:20 | length | This field is not properly synchronized in that no single monitor covers all accesses, but the class $@ is annotated as @ThreadSafe. | examples/LockExample.java:14:14:14:24 | LockExample | LockExample |
+| examples/LockExample.java:19:15:19:31 | notRelatedToOther | This field is not properly synchronized in that no single monitor covers all accesses, but the class $@ is annotated as @ThreadSafe. | examples/LockExample.java:14:14:14:24 | LockExample | LockExample |
+| examples/LockExample.java:20:17:20:23 | content | This field is not properly synchronized in that no single monitor covers all accesses, but the class $@ is annotated as @ThreadSafe. | examples/LockExample.java:14:14:14:24 | LockExample | LockExample |
+| examples/LockExample.java:44:5:44:10 | length | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:44:5:44:10 | length | this expression |
+| examples/LockExample.java:45:5:45:11 | content | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:45:5:45:11 | content | this expression |
+| examples/LockExample.java:45:13:45:18 | length | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:45:13:45:18 | length | this expression |
+| examples/LockExample.java:49:5:49:10 | length | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:49:5:49:10 | length | this expression |
+| examples/LockExample.java:62:5:62:10 | length | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:62:5:62:10 | length | this expression |
+| examples/LockExample.java:65:5:65:11 | content | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:65:5:65:11 | content | this expression |
+| examples/LockExample.java:65:13:65:18 | length | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:65:13:65:18 | length | this expression |
+| examples/LockExample.java:69:5:69:10 | length | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:69:5:69:10 | length | this expression |
+| examples/LockExample.java:71:5:71:11 | content | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:71:5:71:11 | content | this expression |
+| examples/LockExample.java:71:13:71:18 | length | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:71:13:71:18 | length | this expression |
+| examples/LockExample.java:76:5:76:10 | length | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:76:5:76:10 | length | this expression |
+| examples/LockExample.java:79:5:79:11 | content | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:79:5:79:11 | content | this expression |
+| examples/LockExample.java:79:13:79:18 | length | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:79:13:79:18 | length | this expression |
+| examples/LockExample.java:112:5:112:21 | notRelatedToOther | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:112:5:112:21 | notRelatedToOther | this expression |
+| examples/LockExample.java:119:5:119:21 | notRelatedToOther | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:119:5:119:21 | notRelatedToOther | this expression |
+| examples/LockExample.java:124:5:124:21 | notRelatedToOther | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:124:5:124:21 | notRelatedToOther | this expression |
+| examples/LockExample.java:145:5:145:21 | notRelatedToOther | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:145:5:145:21 | notRelatedToOther | this expression |
+| examples/LockExample.java:153:5:153:21 | notRelatedToOther | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/LockExample.java:153:5:153:21 | notRelatedToOther | this expression |
+| examples/ManyLocks.java:8:17:8:17 | y | This field is not properly synchronized in that no single monitor covers all accesses, but the class $@ is annotated as @ThreadSafe. | examples/ManyLocks.java:7:14:7:22 | ManyLocks | ManyLocks |
+| examples/SyncLstExample.java:45:5:45:7 | lst | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/SyncLstExample.java:45:5:45:7 | lst | this expression |
+| examples/SyncStackExample.java:37:5:37:7 | stc | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/SyncStackExample.java:37:5:37:7 | stc | this expression |
+| examples/SynchronizedAndLock.java:10:17:10:22 | length | This field is not properly synchronized in that no single monitor covers all accesses, but the class $@ is annotated as @ThreadSafe. | examples/SynchronizedAndLock.java:7:14:7:32 | SynchronizedAndLock | SynchronizedAndLock |
+| examples/Test.java:52:5:52:10 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/Test.java:24:5:24:18 | setYPrivate(...) | this expression |
+| examples/Test.java:60:5:60:10 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/Test.java:60:5:60:10 | this.y | this expression |
+| examples/Test.java:74:5:74:10 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/Test.java:74:5:74:10 | this.y | this expression |
+| examples/Test.java:74:14:74:14 | y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/Test.java:74:14:74:14 | y | this expression |

java/ql/test/query-tests/ThreadSafe/ThreadSafe.qlref

@@ -0,0 +1,2 @@
+query: Likely Bugs/Concurrency/ThreadSafe.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

java/ql/test/query-tests/ThreadSafe/examples/Alias.java

@@ -0,0 +1,21 @@
+package examples;
+
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class Alias {
+    private int y;
+
+    private final ReentrantLock lock = new ReentrantLock();
+
+    public void notMismatch() {
+        final ReentrantLock lock = this.lock;
+        lock.lock();
+        try {
+            y = 42;
+        } finally {
+            this.lock.unlock();
+        }
+    }
+}

java/ql/test/query-tests/ThreadSafe/examples/App.java

@@ -0,0 +1,14 @@
+/*
+ * This Java source file was generated by the Gradle 'init' task.
+ */
+package examples;
+
+public class App {
+    public String getGreeting() {
+        return "Hello World!";
+    }
+
+    public static void main(String[] args) {
+        System.out.println(new App().getGreeting());
+    }
+}

java/ql/test/query-tests/ThreadSafe/examples/C.java

@@ -0,0 +1,72 @@
+package examples;
+
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class C {
+
+    private int y;
+    private Lock lock = new ReentrantLock();
+    private Lock lock2 = new ReentrantLock();
+
+    public void m() {
+        this.y = 0; // $ Alert
+        this.y += 1; // $ Alert
+        this.y = this.y - 1; // $ Alert
+    }
+
+    public void n4() {
+        this.y = 0; // $ Alert
+        this.y += 1; // $ Alert
+        this.y = this.y - 1; // $ Alert
+    }
+
+    public void setY(int y) {
+        this.y = y; // $ Alert
+    }
+
+    public void test() {
+        if (y == 0) { // $ Alert
+            lock.lock();
+        }
+        y = 0; // $ Alert
+        lock.unlock();
+    }
+
+    public void n() {
+        this.lock.lock();
+        this.y = 0;
+        this.y += 1;
+        this.y = this.y - 1;
+        this.lock.unlock();
+    }
+
+    public void callTestLock2() {
+        lock2.lock();
+        setY(1);
+        lock2.unlock();
+    }
+
+    public void n2() {
+        lock.lock();
+        this.y = 0;
+        this.y += 1;
+        this.y = this.y - 1;
+        lock.unlock();
+    }
+
+    public void n3() {
+        lock.lock();
+        y = 0;
+        y += 1;
+        y = y - 1;
+        lock.unlock(); 
+    }
+
+    public void callTest() {
+        lock.lock();
+        setY(1);
+        lock.unlock();
+    }
+}

java/ql/test/query-tests/ThreadSafe/examples/DeepPaths.java

@@ -0,0 +1,61 @@
+package examples;
+
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class DeepPaths {
+    private int y; // $ Alert
+
+    private final Lock lock1 = new ReentrantLock();
+    private final Lock lock2 = new ReentrantLock();
+    private final Lock lock3 = new ReentrantLock();
+
+    public void layer1Locked() {
+        lock1.lock();
+        this.layer2Locked();
+        lock1.unlock();
+    }
+
+    private void layer2Locked() {
+        lock2.lock();
+        this.layer3Unlocked();
+        lock2.unlock();
+    }
+
+    private void layer3Locked() {
+        lock3.lock();
+        y++;
+        lock3.unlock();
+    }
+
+    public void layer1Skip() {
+        lock2.lock();
+        this.layer3Locked();
+        lock2.unlock();
+    }
+
+    public void layer1Indirect() {
+        this.layer2();
+    }
+
+    private void layer2() {
+        this.layer2Locked();
+    }
+
+    public void layer1Unlocked() {
+        this.layer2Unlocked();
+    }
+
+    private void layer2Unlocked() {
+        this.layer3();
+    }
+
+    private void layer3() {
+        this.layer3Locked();
+    }
+
+    private void layer3Unlocked() {
+        y++;
+    }
+}

java/ql/test/query-tests/ThreadSafe/examples/FaultyTurnstileExample.java

@@ -0,0 +1,40 @@
+package examples;
+
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+class FaultyTurnstileExample {
+  private Lock lock = new ReentrantLock();
+  private int count = 0;
+
+  public void inc() {
+    lock.lock();
+    count++; // $ MISSING: Alert
+    lock.unlock();
+  }
+
+  public void dec() {
+    count--; // $ Alert
+  }
+}
+
+@ThreadSafe
+class FaultyTurnstileExample2 {
+  private Lock lock1 = new ReentrantLock();
+  private Lock lock2 = new ReentrantLock();
+  private int count = 0; // $ Alert
+
+  public void inc() {
+    lock1.lock();
+    count++;
+    lock1.unlock();
+  }
+
+  public void dec() {
+    lock2.lock();
+    count--;
+    lock2.unlock();
+  }
+}
+

java/ql/test/query-tests/ThreadSafe/examples/FlawedSemaphore.java

@@ -0,0 +1,30 @@
+package examples;
+
+@ThreadSafe
+public class FlawedSemaphore {
+  private final int capacity;
+  private int state;
+
+  public FlawedSemaphore(int c) {
+    capacity = c;
+    state = 0;
+  }
+
+  public void acquire() {
+    try {
+      while (state == capacity) { // $ Alert
+        this.wait();
+      }
+      state++; // $ Alert
+    } catch (InterruptedException e) {
+      e.printStackTrace();
+    }
+  }
+
+  public void release() {
+    synchronized (this) {
+      state--; // State can become negative
+      this.notifyAll();
+    }
+  }
+}

java/ql/test/query-tests/ThreadSafe/examples/LockCorrect.java

@@ -0,0 +1,51 @@
+package examples;
+
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class LockCorrect {
+  private Lock lock1 = new ReentrantLock();
+
+  private int length = 0;
+  private int notRelatedToOther = 10;
+  private int[] content  = new int[10];
+  private int thisSynchronized = 0;
+
+  public void add(int value) {
+    lock1.lock();
+    length++;
+    content[length] = value;
+    lock1.unlock();
+  }
+
+  public void removeCorrect() {
+    lock1.lock();
+    content[length] = 0;
+    length--;
+    lock1.unlock();
+  }
+
+  public void synchronizedOnLock1() {
+    synchronized(lock1) {
+      notRelatedToOther++;
+    }
+  }
+
+  public void synchronizedOnLock12() {
+    synchronized(lock1) {
+      notRelatedToOther++;
+    }
+  }
+
+  public synchronized void x() {
+    thisSynchronized++;
+  }
+
+  public void x1() {
+    synchronized(this) {
+      thisSynchronized++;
+    }
+  }
+
+}

java/ql/test/query-tests/ThreadSafe/examples/LockExample.java

@@ -0,0 +1,156 @@
+// This example shows that we only get one alert "per concurrency problem":
+// For each problematic variable, we get one alert at the earliest conflicting write.
+// If the variable is involved in several different monitors, we get an alert for each monitor that
+// is not correctly used.
+// A single alert can have many related locations, since each conflicting access which is not
+// properly synchronized is a related location.
+// This leads to many lines in the .expected file.
+package examples;
+
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class LockExample {
+  private Lock lock1 = new ReentrantLock();
+  private Lock lock2 = new ReentrantLock();
+
+  private int length = 0; // $ Alert
+  private int notRelatedToOther = 10; // $ Alert
+  private int[] content  = new int[10]; // $ Alert
+
+  public void add(int value) {
+    lock1.lock();
+    length++;
+    content[length] = value;
+    lock1.unlock();
+  }
+
+  public void removeCorrect() {
+    lock1.lock();
+    length--;
+    content[length] = 0;
+    lock1.unlock();
+  }
+
+  public void notTheSameLockAsAdd() { // use locks, but different ones
+    lock2.lock();
+    length--;
+    content[length] = 0;
+    lock2.unlock();
+  }
+
+  public void noLock() { // no locks
+    length--; // $ Alert
+    content[length] = 0; // $ Alert
+  }
+
+  public void fieldUpdatedOutsideOfLock() { // adjusts length without lock
+    length--; // $ Alert
+
+    lock1.lock();
+    content[length] = 0;
+    lock1.unlock();
+  }
+
+  public synchronized void synchronizedLock() { // no locks, but with synchronized
+    length--;
+    content[length] = 0;
+  }
+
+  public void onlyLocked() { // never unlocked, only locked
+    length--; // $ Alert
+
+    lock1.lock();
+    content[length] = 0; // $ Alert
+  }
+
+  public void onlyUnlocked() { // never locked, only unlocked
+    length--; // $ Alert
+
+    content[length] = 0; // $ Alert
+    lock1.unlock();
+  }
+
+  public void notSameLock() {
+    length--; // $ Alert
+
+    lock2.lock();// Not the same lock
+    content[length] = 0; // $ Alert
+    lock1.unlock();
+  }
+
+  public void updateCount() {
+    lock2.lock();
+    notRelatedToOther++;
+    lock2.unlock();
+  }
+
+  public void updateCountTwiceCorrect() {
+    lock2.lock();
+    notRelatedToOther++;
+    lock2.unlock();
+    lock1.lock();
+    notRelatedToOther++;
+    lock1.unlock();
+  }
+
+  public void updateCountTwiceDifferentLocks() {
+    lock2.lock();
+    notRelatedToOther++;
+    lock2.unlock();
+    lock1.lock();
+    notRelatedToOther++;
+    lock2.unlock();
+  }
+
+  public void updateCountTwiceLock() {
+    lock2.lock();
+    notRelatedToOther++;
+    lock2.unlock();
+    lock1.lock();
+    notRelatedToOther++; // $ Alert
+  }
+
+  public void updateCountTwiceUnLock() {
+    lock2.lock();
+    notRelatedToOther++;
+    lock2.unlock();
+    notRelatedToOther++; // $ Alert
+    lock1.unlock();
+  }
+
+  public void synchronizedNonRelatedOutside() {
+    notRelatedToOther++; // $ Alert
+
+    synchronized(this) {
+      length++;
+    }
+  }
+
+  public void synchronizedNonRelatedOutside2() {
+    int x = 0;
+    x++;
+
+    synchronized(this) {
+      length++;
+    }
+  }
+
+  public void synchronizedNonRelatedOutside3() {
+    synchronized(this) {
+      length++;
+    }
+
+    notRelatedToOther = 1; // $ Alert
+  }
+
+  public void synchronizedNonRelatedOutside4() {
+    synchronized(lock1) {
+      length++;
+    }
+
+    notRelatedToOther = 1; // $ Alert
+  }
+
+}

java/ql/test/query-tests/ThreadSafe/examples/LoopyCallGraph.java

@@ -0,0 +1,33 @@
+package examples;
+
+import java.util.Random;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class LoopyCallGraph {
+  private Lock lock = new ReentrantLock();
+  private int count = 0;
+  private Random random = new Random();
+
+  public void entry() {
+    if (random.nextBoolean()) {
+      increase();  // this could look like an unprotected path to a call to dec()
+    } else {
+      lock.lock();
+      dec();
+      lock.unlock();
+    }
+  }
+
+  private void increase() {
+    lock.lock();
+    count = 10;
+    lock.unlock();
+    entry();  // this could look like an unprotected path to a call to dec()
+  }
+
+  private void dec() {
+    count--;
+  }
+}

java/ql/test/query-tests/ThreadSafe/examples/ManyLocks.java

@@ -0,0 +1,37 @@
+package examples;
+
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class ManyLocks {
+    private int y; // $ Alert
+
+    private final Lock lock1 = new ReentrantLock();
+    private final Lock lock2 = new ReentrantLock();
+    private final Lock lock3 = new ReentrantLock();
+
+    public void inc() {
+        lock1.lock();
+        lock2.lock();
+        y++;
+        lock2.unlock();
+        lock1.unlock();
+    }
+
+    public void dec() {
+        lock2.lock();
+        lock3.lock();
+        y--;
+        lock3.unlock();
+        lock2.unlock();
+    }
+
+    public void reset() {
+        lock1.lock();
+        lock3.lock();
+        y = 0;
+        lock3.unlock();
+        lock1.unlock();
+    }
+}

java/ql/test/query-tests/ThreadSafe/examples/SyncLstExample.java

@@ -0,0 +1,47 @@
+package examples;
+
+import java.util.List;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class SyncLstExample<T> {
+  private Lock lock = new ReentrantLock();
+  private List<T> lst;
+
+  public SyncLstExample(List<T> lst) {
+    this.lst = lst;
+  }
+
+  public void add(T item) {
+    lock.lock();
+    lst.add(item);
+    lock.unlock();
+  }
+
+  public void remove(int i) {
+    lock.lock();
+    lst.remove(i);
+    lock.unlock();
+  }
+}
+
+@ThreadSafe
+class FaultySyncLstExample<T> {
+  private Lock lock = new ReentrantLock();
+  private List<T> lst;
+
+  public FaultySyncLstExample(List<T> lst) {
+    this.lst = lst;
+  }
+
+  public void add(T item) {
+    lock.lock();
+    lst.add(item);
+    lock.unlock();
+  }
+
+  public void remove(int i) {
+    lst.remove(i); // $ Alert
+  }
+}

java/ql/test/query-tests/ThreadSafe/examples/SyncStackExample.java

@@ -0,0 +1,39 @@
+package examples;
+
+import java.util.Stack;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class SyncStackExample<T> {
+  private Lock lock = new ReentrantLock();
+  private Stack<T> stc = new Stack<T>();
+
+  public void push(T item) {
+    lock.lock();
+    stc.push(item);
+    lock.unlock();
+  }
+
+  public void pop() {
+    lock.lock();
+    stc.pop();
+    lock.unlock();
+  }
+}
+
+@ThreadSafe
+class FaultySyncStackExample<T> {
+  private Lock lock = new ReentrantLock();
+  private Stack<T> stc = new Stack<T>();
+
+  public void push(T item) {
+    lock.lock();
+    stc.push(item);
+    lock.unlock();
+  }
+
+  public void pop() {
+    stc.pop(); // $ Alert
+  }
+}

java/ql/test/query-tests/ThreadSafe/examples/SynchronizedAndLock.java

@@ -0,0 +1,21 @@
+package examples;
+
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class SynchronizedAndLock {
+    private Lock lock = new ReentrantLock();
+
+    private int length = 0; // $ Alert
+
+    public void add(int value) {
+        lock.lock();
+        length++;
+        lock.unlock();
+    }
+
+    public synchronized void subtract() {
+        length--;
+    }
+}

java/ql/test/query-tests/ThreadSafe/examples/Test.java

@@ -0,0 +1,76 @@
+package examples;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class Test {
+  /**
+   * Escaping field due to public visibility.
+   */
+  int publicField;
+  
+  private int y;
+  final int immutableField = 1;
+
+  // As of the below examples with synchronized as well. Except the incorrectly placed lock.
+
+  private Lock lock = new ReentrantLock();
+
+  /**
+   * Calls the a method where y field escapes.
+   * @param y
+   */
+  public void setYAgainInCorrect(int t) {
+    setYPrivate(t);
+  }
+
+  /**
+   * Locks the method where y field escapes.
+   * @param y
+   */
+  public void setYAgainCorrect(int y) {
+    lock.lock();
+    setYPrivate(y);
+    lock.unlock();
+  }
+
+  /**
+   * No escaping y field. Locks the y before assignment.
+   * @param y
+   */
+  public void setYCorrect(int y) {
+    lock.lock();
+    this.y = y;
+    lock.unlock();
+  }
+
+  /**
+   * No direct escaping, since it method is private. Only escaping if another public method uses this.
+   * @param y
+   */
+  private void setYPrivate(int y) {
+    this.y = y; // $ Alert
+  }
+
+  /**
+   * Incorrectly locks y.
+   * @param y
+   */
+  public void setYWrongLock(int y) {
+    this.y = y; // $ Alert
+    lock.lock();
+    lock.unlock();
+  }
+
+  public synchronized int getImmutableField() {
+    return immutableField;
+  }
+
+  public synchronized int getImmutableField2() {
+    return immutableField;
+  }
+
+  public void testMethod() {
+    this.y = y + 2; // $ Alert
+  }
+}

java/ql/test/query-tests/ThreadSafe/examples/Test2.java

@@ -0,0 +1,22 @@
+package examples;
+
+import java.util.ArrayList;
+
+// Note: Not marked as thread-safe
+// We inherit from this in Test3Super.java
+public class Test2 {
+  int x;
+  protected ArrayList<String> lst = new ArrayList<>();
+
+  public Test2() {
+    this.x = 0;
+  }
+
+  public void changeX() {
+    this.x = x + 1;
+  }
+
+  public void changeLst() {
+    lst.add("Hello");
+  }
+}

java/ql/test/query-tests/ThreadSafe/examples/Test3Super.java

@@ -0,0 +1,17 @@
+package examples;
+
+@ThreadSafe
+public class Test3Super extends Test2 {  // We might want an alert here for the inherited unsafe methods.
+
+  public Test3Super() {
+    super.x = 0;
+  }
+
+  public void y() {
+    super.x = 0;  //$ MISSING: Alert
+  }
+
+  public void yLst() {
+    super.lst.add("Hello!");  //$ MISSING: Alert
+  }
+}

java/ql/test/query-tests/ThreadSafe/examples/ThreadSafe.java

@@ -0,0 +1,4 @@
+package examples;
+
+public @interface ThreadSafe {
+}

java/ql/test/query-tests/ThreadSafe/examples/TurnstileExample.java

@@ -0,0 +1,23 @@
+package examples;
+
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+@ThreadSafe
+public class TurnstileExample {
+  private Lock lock = new ReentrantLock();
+  private int count = 0;
+
+  public void inc() {
+    Lock l = lock;
+    l.lock();
+    count++;
+    l.unlock();
+  }
+
+  public void dec() {
+    lock.lock();
+    count--;
+    lock.unlock();
+  }
+}

javascript/ql/lib/semmle/javascript/security/dataflow/CommandInjectionQuery.qll

@@ -34,8 +34,9 @@ module CommandInjectionConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(DataFlow::Node node |
+    exists(DataFlow::Node node | isSinkWithHighlight(sink, node) |
-      isSinkWithHighlight(sink, node) and
+      result = sink.getLocation()
+      or
       result = node.getLocation()
     )
   }

javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionQuery.qll

@@ -30,8 +30,9 @@ module IndirectCommandInjectionConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
-    exists(DataFlow::Node node |
+    exists(DataFlow::Node node | isSinkWithHighlight(sink, node) |
-      isSinkWithHighlight(sink, node) and
+      result = sink.getLocation()
+      or
       result = node.getLocation()
     )
   }

javascript/ql/lib/semmle/javascript/security/dataflow/ShellCommandInjectionFromEnvironmentQuery.qll

@@ -33,7 +33,7 @@ module ShellCommandInjectionFromEnvironmentConfig implements DataFlow::ConfigSig
   Location getASelectedSinkLocation(DataFlow::Node sink) {
     exists(DataFlow::Node node |
       isSinkWithHighlight(sink, node) and
-      result = node.getLocation()
+      result = [node.getLocation(), sink.getLocation()]
     )
   }
 }

javascript/ql/lib/semmle/javascript/security/regexp/PolynomialReDoSQuery.qll

@@ -29,6 +29,8 @@ module PolynomialReDoSConfig implements DataFlow::ConfigSig {
   predicate observeDiffInformedIncrementalMode() { any() }
 
   Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.(Sink).getLocation()
+    or
     result = sink.(Sink).getHighlight().getLocation()
     or
     result = sink.(Sink).getRegExp().getLocation()