From 3271212187ef4051111a146ceeda255f3c5af3e7 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Wed, 9 Aug 2023 19:01:11 +0200 Subject: [PATCH 1/3] Fix potential UB in ByteString.hashCode It has a benign JVM data-race on the hashCode field, but on K/N LLVM has no guarantees it will read default or written value (e.g. OoTA is one of the possibilities). The recommendation from K/N folks is to use @Volatile here Fixes #190 --- bytestring/common/src/ByteString.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bytestring/common/src/ByteString.kt b/bytestring/common/src/ByteString.kt index 6066c9313..494a38d97 100644 --- a/bytestring/common/src/ByteString.kt +++ b/bytestring/common/src/ByteString.kt @@ -21,6 +21,7 @@ package kotlinx.io.bytestring +import kotlin.concurrent.Volatile import kotlin.math.max import kotlin.math.min @@ -69,6 +70,7 @@ public class ByteString private constructor( public constructor(data: ByteArray, startIndex: Int = 0, endIndex: Int = data.size) : this(data.copyOfRange(startIndex, endIndex), null) + @Volatile private var hashCode: Int = 0 public companion object { From 1888932964dc71dad2ed85ff33338c397ff925ee Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Thu, 10 Aug 2023 12:06:25 +0200 Subject: [PATCH 2/3] Introduce BeningDataRace --- bytestring/common/src/-Platform.kt | 14 ++++++++++++++ bytestring/common/src/ByteString.kt | 3 +-- bytestring/native/src/-PlatformNative.kt | 10 ++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 bytestring/common/src/-Platform.kt create mode 100644 bytestring/native/src/-PlatformNative.kt diff --git a/bytestring/common/src/-Platform.kt b/bytestring/common/src/-Platform.kt new file mode 100644 index 000000000..78294eee2 --- /dev/null +++ b/bytestring/common/src/-Platform.kt @@ -0,0 +1,14 @@ +/* +* Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. +* Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file. +*/ +package kotlinx.io.bytestring + +/** + * Annotation indicating that the marked property is the subjectof benign data race. + * LLVM does not support this notion, so on K/N platforms we alias it into `@Volatile` to prevent potential OoTA. + */ +@OptionalExpectation +@Target(AnnotationTarget.FIELD) +@OptIn(ExperimentalMultiplatform::class) +internal expect annotation class BenignDataRace() diff --git a/bytestring/common/src/ByteString.kt b/bytestring/common/src/ByteString.kt index 494a38d97..19ecef0d8 100644 --- a/bytestring/common/src/ByteString.kt +++ b/bytestring/common/src/ByteString.kt @@ -21,7 +21,6 @@ package kotlinx.io.bytestring -import kotlin.concurrent.Volatile import kotlin.math.max import kotlin.math.min @@ -70,7 +69,7 @@ public class ByteString private constructor( public constructor(data: ByteArray, startIndex: Int = 0, endIndex: Int = data.size) : this(data.copyOfRange(startIndex, endIndex), null) - @Volatile + @BenignDataRace private var hashCode: Int = 0 public companion object { diff --git a/bytestring/native/src/-PlatformNative.kt b/bytestring/native/src/-PlatformNative.kt new file mode 100644 index 000000000..515c94010 --- /dev/null +++ b/bytestring/native/src/-PlatformNative.kt @@ -0,0 +1,10 @@ +/* +* Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. +* Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file. +*/ +package kotlinx.io.bytestring + +import kotlin.concurrent.* + +@Suppress("ACTUAL_WITHOUT_EXPECT") // This suppress can be removed in 2.0: KT-59355 +internal actual typealias BenignDataRace = Volatile From 2c352ed2cd626b7c77a5c9ae9507db85d7ce418d Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Thu, 10 Aug 2023 13:53:18 +0200 Subject: [PATCH 3/3] Update bytestring/common/src/-Platform.kt Co-authored-by: Filipp Zhinkin --- bytestring/common/src/-Platform.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bytestring/common/src/-Platform.kt b/bytestring/common/src/-Platform.kt index 78294eee2..dfaae6840 100644 --- a/bytestring/common/src/-Platform.kt +++ b/bytestring/common/src/-Platform.kt @@ -5,7 +5,7 @@ package kotlinx.io.bytestring /** - * Annotation indicating that the marked property is the subjectof benign data race. + * Annotation indicating that the marked property is the subject of benign data race. * LLVM does not support this notion, so on K/N platforms we alias it into `@Volatile` to prevent potential OoTA. */ @OptionalExpectation