Skip to content

Commit 694bf74

Browse files
authored
chore(query): Introduce VecExt and SliceExt to check unsafe bounds in debug mode (#16747)
* merge version * merge version * add unsafe comments * add quick tests * fix tests
1 parent 1388625 commit 694bf74

File tree

17 files changed

+258
-114
lines changed

17 files changed

+258
-114
lines changed

Cargo.lock

Lines changed: 32 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,7 @@ databend-driver = { version = "0.22" }
542542
databend-driver-core = { version = "0.22" }
543543
msql-srv = "0.11.0"
544544
mysql_common = "0.32.4"
545+
quickcheck = "1.0"
545546
sqllogictest = "0.21.0"
546547
sqlparser = "0.50.0"
547548

src/binaries/query/entry.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ use databend_common_base::runtime::GLOBAL_MEM_STAT;
2121
use databend_common_config::Commands;
2222
use databend_common_config::InnerConfig;
2323
use databend_common_config::DATABEND_COMMIT_VERSION;
24-
use databend_common_config::QUERY_GIT_SEMVER;
25-
use databend_common_config::QUERY_GIT_SHA;
26-
use databend_common_config::QUERY_SEMVER;
24+
use databend_common_config::DATABEND_GIT_SEMVER;
25+
use databend_common_config::DATABEND_GIT_SHA;
26+
use databend_common_config::DATABEND_SEMVER;
2727
use databend_common_exception::ErrorCode;
2828
use databend_common_exception::Result;
2929
use databend_common_exception::ResultExt;
@@ -55,7 +55,7 @@ pub async fn run_cmd(conf: &InnerConfig) -> Result<bool, MainError> {
5555
match &conf.subcommand {
5656
None => return Ok(false),
5757
Some(Commands::Ver) => {
58-
println!("version: {}", *QUERY_SEMVER);
58+
println!("version: {}", *DATABEND_SEMVER);
5959
println!("min-compatible-metasrv-version: {}", MIN_METASRV_SEMVER);
6060
}
6161
Some(Commands::Local {
@@ -215,7 +215,11 @@ pub async fn start_services(conf: &InnerConfig) -> Result<(), MainError> {
215215

216216
// Metric API service.
217217
{
218-
set_system_version("query", QUERY_GIT_SEMVER.as_str(), QUERY_GIT_SHA.as_str());
218+
set_system_version(
219+
"query",
220+
DATABEND_GIT_SEMVER.as_str(),
221+
DATABEND_GIT_SHA.as_str(),
222+
);
219223
let address = conf.query.metric_api_address.clone();
220224
let mut srv = MetricService::create();
221225
let listening = srv

src/common/base/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ procfs = { workspace = true }
6666
[dev-dependencies]
6767
anyerror = { workspace = true }
6868
anyhow = { workspace = true }
69+
quickcheck = { workspace = true }
6970
rand = { workspace = true }
7071
serde_test = { workspace = true }
7172

src/common/base/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ pub mod http_client;
3535
pub mod mem_allocator;
3636
pub mod rangemap;
3737
pub mod runtime;
38+
pub mod slice_ext;
3839
pub mod vec_ext;
3940
pub mod version;
4041

src/common/base/src/slice_ext.rs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Copyright 2021 Datafuse Labs
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
use std::slice::SliceIndex;
16+
17+
pub trait GetSaferUnchecked<T> {
18+
/// # Safety
19+
///
20+
/// Calling this method with an out-of-bounds index is *[undefined behavior]*
21+
/// even if the resulting reference is not used.
22+
unsafe fn get_unchecked_release<I>(&self, index: I) -> &<I as SliceIndex<[T]>>::Output
23+
where I: SliceIndex<[T]>;
24+
25+
/// # Safety
26+
///
27+
/// Calling this method with an out-of-bounds index is *[undefined behavior]*
28+
/// even if the resulting reference is not used.
29+
unsafe fn get_unchecked_release_mut<I>(
30+
&mut self,
31+
index: I,
32+
) -> &mut <I as SliceIndex<[T]>>::Output
33+
where
34+
I: SliceIndex<[T]>;
35+
}
36+
37+
impl<T> GetSaferUnchecked<T> for [T] {
38+
#[inline(always)]
39+
unsafe fn get_unchecked_release<I>(&self, index: I) -> &<I as SliceIndex<[T]>>::Output
40+
where I: SliceIndex<[T]> {
41+
if cfg!(debug_assertions) {
42+
&self[index]
43+
} else {
44+
unsafe { self.get_unchecked(index) }
45+
}
46+
}
47+
48+
#[inline(always)]
49+
unsafe fn get_unchecked_release_mut<I>(
50+
&mut self,
51+
index: I,
52+
) -> &mut <I as SliceIndex<[T]>>::Output
53+
where
54+
I: SliceIndex<[T]>,
55+
{
56+
if cfg!(debug_assertions) {
57+
&mut self[index]
58+
} else {
59+
unsafe { self.get_unchecked_mut(index) }
60+
}
61+
}
62+
}

src/common/base/src/vec_ext.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@ pub trait VecExt<T> {
1616
/// Remove the first element that is equal to the given item.
1717
fn remove_first(&mut self, item: &T) -> Option<T>
1818
where T: PartialEq;
19+
20+
/// Will push an item and not check if there is enough capacity
21+
///
22+
/// # Safety
23+
/// Caller must ensure the array has enough capacity to hold `T`.
24+
unsafe fn push_unchecked(&mut self, value: T);
25+
26+
/// # Safety
27+
/// Caller must ensure the array has enough capacity to hold `&[T]`.
28+
unsafe fn extend_from_slice_unchecked(&mut self, val: &[T]);
1929
}
2030

2131
impl<T> VecExt<T> for Vec<T> {
@@ -24,4 +34,34 @@ impl<T> VecExt<T> for Vec<T> {
2434
let pos = self.iter().position(|x| x == item)?;
2535
Some(self.remove(pos))
2636
}
37+
38+
#[inline]
39+
unsafe fn push_unchecked(&mut self, value: T) {
40+
debug_assert!(self.capacity() > self.len());
41+
let end = self.as_mut_ptr().add(self.len());
42+
std::ptr::write(end, value);
43+
self.set_len(self.len() + 1);
44+
}
45+
46+
unsafe fn extend_from_slice_unchecked(&mut self, val: &[T]) {
47+
debug_assert!(self.capacity() >= self.len() + val.len());
48+
let end = self.as_mut_ptr().add(self.len());
49+
std::ptr::copy_nonoverlapping(val.as_ptr(), end, val.len());
50+
self.set_len(self.len() + val.len());
51+
}
52+
}
53+
54+
pub trait VecU8Ext {
55+
/// # Safety
56+
/// Caller must ensure the array has enough capacity to hold `V`.
57+
unsafe fn store_value_uncheckd<V>(&mut self, val: &V);
58+
}
59+
60+
impl VecU8Ext for Vec<u8> {
61+
unsafe fn store_value_uncheckd<T>(&mut self, val: &T) {
62+
debug_assert!(self.capacity() >= self.len() + std::mem::size_of::<T>());
63+
let end = self.as_mut_ptr().add(self.len());
64+
std::ptr::copy_nonoverlapping(val as *const T as *const u8, end, std::mem::size_of::<T>());
65+
self.set_len(self.len() + std::mem::size_of::<T>());
66+
}
2767
}

src/common/base/tests/it/ext.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright 2021 Datafuse Labs
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
use databend_common_base::vec_ext::VecExt;
16+
use databend_common_base::vec_ext::VecU8Ext;
17+
use quickcheck::quickcheck;
18+
19+
#[test]
20+
fn test_remove_first() {
21+
fn prop(vec: Vec<i32>, item: i32) -> bool {
22+
let mut vec_clone = vec.clone();
23+
let result = vec_clone.remove_first(&item);
24+
if let Some(pos) = vec.iter().position(|x| x == &item) {
25+
assert_eq!(result, Some(vec[pos]));
26+
let mut v = vec.clone();
27+
v.remove(pos);
28+
vec_clone == v
29+
} else {
30+
assert_eq!(result, None);
31+
vec_clone == vec
32+
}
33+
}
34+
quickcheck(prop as fn(Vec<i32>, i32) -> bool);
35+
}
36+
37+
#[test]
38+
fn test_push_unchecked() {
39+
fn prop(values: Vec<i32>) -> bool {
40+
let mut vec: Vec<i32> = Vec::with_capacity(values.len());
41+
unsafe {
42+
for &value in &values {
43+
vec.push_unchecked(value);
44+
}
45+
}
46+
vec.len() == values.len() && vec.iter().zip(values.iter()).all(|(a, b)| a == b)
47+
}
48+
quickcheck(prop as fn(Vec<i32>) -> bool);
49+
}
50+
51+
#[test]
52+
fn test_extend_from_slice_unchecked() {
53+
fn prop(values: Vec<i32>) -> bool {
54+
let mut vec: Vec<i32> = Vec::with_capacity(values.len() + 1);
55+
unsafe {
56+
vec.push_unchecked(1); // Ensure there's at least one element
57+
vec.extend_from_slice_unchecked(&values);
58+
}
59+
vec.len() == 1 + values.len() && vec[1..] == values[..]
60+
}
61+
quickcheck(prop as fn(Vec<i32>) -> bool);
62+
}
63+
64+
#[test]
65+
fn test_store_value_uncheckd() {
66+
fn prop(value: i32) -> bool {
67+
let mut vec: Vec<u8> = Vec::with_capacity(std::mem::size_of::<i32>());
68+
unsafe {
69+
vec.store_value_uncheckd(&value);
70+
}
71+
vec.len() == std::mem::size_of::<i32>() && vec[0..4] == value.to_le_bytes()
72+
}
73+
quickcheck(prop as fn(i32) -> bool);
74+
}

src/common/base/tests/it/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
use databend_common_base::mem_allocator::GlobalAllocator;
1616

17+
mod ext;
1718
mod fixed_heap;
1819
mod memory;
1920
mod metrics;

src/query/config/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ databend-common-meta-app = { workspace = true }
2828
databend-common-storage = { workspace = true }
2929
databend-common-tracing = { workspace = true }
3030
log = { workspace = true }
31-
semver = { workspace = true }
3231
serde = { workspace = true }
3332
serde_with = { workspace = true }
3433
serfig = { workspace = true }

0 commit comments

Comments
 (0)