builtin,cgen: fix issues found with the stricter sanitizers in clang-18 on Ubuntu 24.04 (#23710)

spytheman · web-flow · commit 2f700c04b7bd · 2025-02-13T21:27:39.000+02:00
diff --git a/vlib/builtin/array.v b/vlib/builtin/array.v
@@ -658,24 +658,26 @@ pub fn (a &array) clone() array {
 // recursively clone given array - `unsafe` when called directly because depth is not checked
 @[unsafe]
 pub fn (a &array) clone_to_depth(depth int) array {
+	source_capacity_in_bytes := u64(a.cap) * u64(a.element_size)
 	mut arr := array{
 		element_size: a.element_size
-		data:         vcalloc(u64(a.cap) * u64(a.element_size))
+		data:         vcalloc(source_capacity_in_bytes)
 		len:          a.len
 		cap:          a.cap
 	}
 	// Recursively clone-generated elements if array element is array type
 	if depth > 0 && a.element_size == sizeof(array) && a.len >= 0 && a.cap >= a.len {
+		ar := array{}
+		asize := int(sizeof(array))
 		for i in 0 .. a.len {
-			ar := array{}
-			unsafe { vmemcpy(&ar, a.get_unsafe(i), int(sizeof(array))) }
+			unsafe { vmemcpy(&ar, a.get_unsafe(i), asize) }
 			ar_clone := unsafe { ar.clone_to_depth(depth - 1) }
 			unsafe { arr.set_unsafe(i, &ar_clone) }
 		}
 		return arr
 	} else {
-		if a.data != 0 {
-			unsafe { vmemcpy(&u8(arr.data), a.data, u64(a.cap) * u64(a.element_size)) }
+		if a.data != 0 && source_capacity_in_bytes > 0 {
+			unsafe { vmemcpy(&u8(arr.data), a.data, source_capacity_in_bytes) }
 		}
 		return arr
 	}
diff --git a/vlib/builtin/builtin.c.v b/vlib/builtin/builtin.c.v
@@ -433,6 +433,8 @@ pub fn malloc(n isize) &u8 {
 	vplayground_mlimit(n)
 	if n < 0 {
 		_memory_panic(@FN, n)
+	} else if n == 0 {
+		return &u8(0)
 	}
 	mut res := &u8(0)
 	$if prealloc {
@@ -639,6 +641,7 @@ pub fn vcalloc(n isize) &u8 {
 	} $else {
 		return unsafe { C.calloc(1, n) }
 	}
+	return &u8(0) // not reached, TODO: remove when V's checker is improved
 }
 
 // special versions of the above that allocate memory which is not scanned
@@ -655,19 +658,35 @@ pub fn vcalloc_noscan(n isize) &u8 {
 		if n < 0 {
 			_memory_panic(@FN, n)
 		}
-		return $if gcboehm_opt ? {
-			unsafe { &u8(C.memset(C.GC_MALLOC_ATOMIC(n), 0, n)) }
+		$if gcboehm_opt ? {
+			res := unsafe { C.GC_MALLOC_ATOMIC(n) }
+			unsafe { C.memset(res, 0, n) }
+			return &u8(res)
 		} $else {
-			unsafe { &u8(C.GC_MALLOC(n)) }
+			res := unsafe { C.GC_MALLOC(n) }
+			return &u8(res)
 		}
 	} $else {
 		return unsafe { vcalloc(n) }
 	}
+	return &u8(0) // not reached, TODO: remove when V's checker is improved
 }
 
 // free allows for manually freeing memory allocated at the address `ptr`.
 @[unsafe]
 pub fn free(ptr voidptr) {
+	$if trace_free ? {
+		C.fprintf(C.stderr, c'free ptr: %p\n', ptr)
+	}
+	if ptr == unsafe { 0 } {
+		$if trace_free_nulls ? {
+			C.fprintf(C.stderr, c'free null ptr\n', ptr)
+		}
+		$if trace_free_nulls_break ? {
+			break_if_debugger_attached()
+		}
+		return
+	}
 	$if prealloc {
 		return
 	} $else $if gcboehm ? {
diff --git a/vlib/builtin/cfns_wrapper.c.v b/vlib/builtin/cfns_wrapper.c.v
@@ -22,6 +22,13 @@ pub fn vmemcpy(dest voidptr, const_src voidptr, n isize) voidptr {
 	$if trace_vmemcpy ? {
 		C.fprintf(C.stderr, c'vmemcpy dest: %p src: %p n: %6ld\n', dest, const_src, n)
 	}
+	$if trace_vmemcpy_nulls ? {
+		if dest == unsafe { 0 } || const_src == unsafe { 0 } {
+			C.fprintf(C.stderr, c'vmemcpy null pointers passed, dest: %p src: %p n: %6ld\n',
+				dest, const_src, n)
+			print_backtrace()
+		}
+	}
 	unsafe {
 		return C.memcpy(dest, const_src, n)
 	}
@@ -71,6 +78,13 @@ pub fn vmemset(s voidptr, c int, n isize) voidptr {
 	$if trace_vmemset ? {
 		C.fprintf(C.stderr, c'vmemset s: %p c: %d n: %6ld\n', s, c, n)
 	}
+	$if trace_vmemset_nulls ? {
+		if s == unsafe { 0 } {
+			C.fprintf(C.stderr, c'vmemset null pointers passed s: %p, c: %6d, n: %6ld\n',
+				s, c, n)
+			print_backtrace()
+		}
+	}
 	unsafe {
 		return C.memset(s, c, n)
 	}
@@ -80,5 +94,19 @@ type FnSortCB = fn (const_a voidptr, const_b voidptr) int
 
 @[inline; unsafe]
 fn vqsort(base voidptr, nmemb usize, size usize, sort_cb FnSortCB) {
+	$if trace_vqsort ? {
+		C.fprintf(C.stderr, c'vqsort base: %p, nmemb: %6uld, size: %6uld, sort_cb: %p\n',
+			base, nmemb, size, sort_cb)
+	}
+	if nmemb == 0 {
+		return
+	}
+	$if trace_vqsort_nulls ? {
+		if base == unsafe { 0 } || u64(sort_cb) == 0 {
+			C.fprintf(C.stderr, c'vqsort null pointers passed base: %p, nmemb: %6uld, size: %6uld, sort_cb: %p\n',
+				base, nmemb, size, sort_cb)
+			print_backtrace()
+		}
+	}
 	C.qsort(base, nmemb, size, voidptr(sort_cb))
 }
diff --git a/vlib/v/gen/c/auto_str_methods.v b/vlib/v/gen/c/auto_str_methods.v
@@ -310,7 +310,7 @@ fn (mut g Gen) gen_str_for_multi_return(info ast.MultiReturn, styp string, str_f
 	g.definitions.writeln('${g.static_non_parallel}string ${str_fn_name}(${styp} a); // auto')
 	mut fn_builder := strings.new_builder(512)
 	fn_builder.writeln('${g.static_non_parallel}string ${str_fn_name}(${styp} a) {')
-	fn_builder.writeln('\tstrings__Builder sb = strings__new_builder(${info.types.len} * 10);')
+	fn_builder.writeln('\tstrings__Builder sb = strings__new_builder(2 + ${info.types.len} * 10);')
 	fn_builder.writeln('\tstrings__Builder_write_string(&sb, _SLIT("("));')
 	for i, typ in info.types {
 		sym := g.table.sym(typ)
@@ -612,7 +612,7 @@ fn (mut g Gen) gen_str_for_array(info ast.Array, styp string, str_fn_name string
 	g.auto_str_funcs.writeln('${g.static_non_parallel}string ${str_fn_name}(${styp} a) { return indent_${str_fn_name}(a, 0);}')
 	g.definitions.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${styp} a, int indent_count); // auto')
 	g.auto_str_funcs.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${styp} a, int indent_count) {')
-	g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(a.len * 10);')
+	g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(2 + a.len * 10);')
 	g.auto_str_funcs.writeln('\tstrings__Builder_write_string(&sb, _SLIT("["));')
 	g.auto_str_funcs.writeln('\tfor (int i = 0; i < a.len; ++i) {')
 	if sym.kind == .function {
@@ -719,7 +719,7 @@ fn (mut g Gen) gen_str_for_array_fixed(info ast.ArrayFixed, styp string, str_fn_
 	g.auto_str_funcs.writeln('${g.static_non_parallel}string ${str_fn_name}(${def_arg}) { return indent_${str_fn_name}(a, 0);}')
 	g.definitions.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${def_arg}, int indent_count); // auto')
 	g.auto_str_funcs.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${def_arg}, int indent_count) {')
-	g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(${info.size} * 10);')
+	g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(2 + ${info.size} * 10);')
 	g.auto_str_funcs.writeln('\tstrings__Builder_write_string(&sb, _SLIT("["));')
 	g.auto_str_funcs.writeln('\tfor (int i = 0; i < ${info.size}; ++i) {')
 	if sym.kind == .function {
@@ -815,7 +815,7 @@ fn (mut g Gen) gen_str_for_map(info ast.Map, styp string, str_fn_name string) {
 	g.auto_str_funcs.writeln('${g.static_non_parallel}string ${str_fn_name}(${styp} m) { return indent_${str_fn_name}(m, 0);}')
 	g.definitions.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${styp} m, int indent_count); // auto')
 	g.auto_str_funcs.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${styp} m, int indent_count) { /* gen_str_for_map */')
-	g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(m.key_values.len*10);')
+	g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(2 + m.key_values.len * 10);')
 	g.auto_str_funcs.writeln('\tstrings__Builder_write_string(&sb, _SLIT("{"));')
 	g.auto_str_funcs.writeln('\tbool is_first = true;')
 	g.auto_str_funcs.writeln('\tfor (int i = 0; i < m.key_values.len; ++i) {')
diff --git a/vlib/v/gen/c/dumpexpr.v b/vlib/v/gen/c/dumpexpr.v
@@ -207,7 +207,7 @@ fn (mut g Gen) dump_expr_definitions() {
 				'\tstring_free(&value);')
 		}
 		surrounder.add('
-	strings__Builder sb = strings__new_builder(256);
+	strings__Builder sb = strings__new_builder(64);
 ', '
 	string res;
 	res = strings__Builder_str(&sb);
diff --git a/vlib/v/tests/builtin_arrays/array_sort_with_compare_sanitize_test.v b/vlib/v/tests/builtin_arrays/array_sort_with_compare_sanitize_test.v
@@ -0,0 +1,26 @@
+fn test_sort_with_compare_works_even_in_sanitized_modes() {
+	mut a := []int{}
+	dump(a.data)
+	dump(a.len)
+	dump(a.cap)
+	a.sort()
+	dump(a)
+	a.sort_with_compare(fn (sa &string, sb &string) int {
+		assert false, 'this should not be called, the array len is 0'
+		return (*sa).len - (*sb).len
+	})
+	dump(a)
+	assert a.len == 0
+
+	mut cloned_a := a.clone()
+	dump(cloned_a.data)
+	dump(cloned_a.len)
+	dump(cloned_a.cap)
+	cloned_a.sort()
+	cloned_a.sort_with_compare(fn (sa &string, sb &string) int {
+		assert false, 'this should not be called, the array len is 0, even after .clone()'
+		return (*sa).len - (*sb).len
+	})
+	dump(cloned_a)
+	assert cloned_a.len == 0
+}

Original file line number	Diff line number	Diff line change
`@@ -207,7 +207,7 @@ fn (mut g Gen) dump_expr_definitions() {`
`207`	`207`	`'\tstring_free(&value);')`
`208`	`208`	`}`
`209`	`209`	`surrounder.add('`
`210`		`- strings__Builder sb = strings__new_builder(256);`
	`210`	`+ strings__Builder sb = strings__new_builder(64);`
`211`	`211`	`', '`
`212`	`212`	`string res;`
`213`	`213`	`res = strings__Builder_str(&sb);`