Skip to content

Commit 2f700c0

Browse files
authored
builtin,cgen: fix issues found with the stricter sanitizers in clang-18 on Ubuntu 24.04 (#23710)
1 parent e9c2314 commit 2f700c0

File tree

6 files changed

+88
-13
lines changed

6 files changed

+88
-13
lines changed

vlib/builtin/array.v

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -658,24 +658,26 @@ pub fn (a &array) clone() array {
658658
// recursively clone given array - `unsafe` when called directly because depth is not checked
659659
@[unsafe]
660660
pub fn (a &array) clone_to_depth(depth int) array {
661+
source_capacity_in_bytes := u64(a.cap) * u64(a.element_size)
661662
mut arr := array{
662663
element_size: a.element_size
663-
data: vcalloc(u64(a.cap) * u64(a.element_size))
664+
data: vcalloc(source_capacity_in_bytes)
664665
len: a.len
665666
cap: a.cap
666667
}
667668
// Recursively clone-generated elements if array element is array type
668669
if depth > 0 && a.element_size == sizeof(array) && a.len >= 0 && a.cap >= a.len {
670+
ar := array{}
671+
asize := int(sizeof(array))
669672
for i in 0 .. a.len {
670-
ar := array{}
671-
unsafe { vmemcpy(&ar, a.get_unsafe(i), int(sizeof(array))) }
673+
unsafe { vmemcpy(&ar, a.get_unsafe(i), asize) }
672674
ar_clone := unsafe { ar.clone_to_depth(depth - 1) }
673675
unsafe { arr.set_unsafe(i, &ar_clone) }
674676
}
675677
return arr
676678
} else {
677-
if a.data != 0 {
678-
unsafe { vmemcpy(&u8(arr.data), a.data, u64(a.cap) * u64(a.element_size)) }
679+
if a.data != 0 && source_capacity_in_bytes > 0 {
680+
unsafe { vmemcpy(&u8(arr.data), a.data, source_capacity_in_bytes) }
679681
}
680682
return arr
681683
}

vlib/builtin/builtin.c.v

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,8 @@ pub fn malloc(n isize) &u8 {
433433
vplayground_mlimit(n)
434434
if n < 0 {
435435
_memory_panic(@FN, n)
436+
} else if n == 0 {
437+
return &u8(0)
436438
}
437439
mut res := &u8(0)
438440
$if prealloc {
@@ -639,6 +641,7 @@ pub fn vcalloc(n isize) &u8 {
639641
} $else {
640642
return unsafe { C.calloc(1, n) }
641643
}
644+
return &u8(0) // not reached, TODO: remove when V's checker is improved
642645
}
643646

644647
// special versions of the above that allocate memory which is not scanned
@@ -655,19 +658,35 @@ pub fn vcalloc_noscan(n isize) &u8 {
655658
if n < 0 {
656659
_memory_panic(@FN, n)
657660
}
658-
return $if gcboehm_opt ? {
659-
unsafe { &u8(C.memset(C.GC_MALLOC_ATOMIC(n), 0, n)) }
661+
$if gcboehm_opt ? {
662+
res := unsafe { C.GC_MALLOC_ATOMIC(n) }
663+
unsafe { C.memset(res, 0, n) }
664+
return &u8(res)
660665
} $else {
661-
unsafe { &u8(C.GC_MALLOC(n)) }
666+
res := unsafe { C.GC_MALLOC(n) }
667+
return &u8(res)
662668
}
663669
} $else {
664670
return unsafe { vcalloc(n) }
665671
}
672+
return &u8(0) // not reached, TODO: remove when V's checker is improved
666673
}
667674

668675
// free allows for manually freeing memory allocated at the address `ptr`.
669676
@[unsafe]
670677
pub fn free(ptr voidptr) {
678+
$if trace_free ? {
679+
C.fprintf(C.stderr, c'free ptr: %p\n', ptr)
680+
}
681+
if ptr == unsafe { 0 } {
682+
$if trace_free_nulls ? {
683+
C.fprintf(C.stderr, c'free null ptr\n', ptr)
684+
}
685+
$if trace_free_nulls_break ? {
686+
break_if_debugger_attached()
687+
}
688+
return
689+
}
671690
$if prealloc {
672691
return
673692
} $else $if gcboehm ? {

vlib/builtin/cfns_wrapper.c.v

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@ pub fn vmemcpy(dest voidptr, const_src voidptr, n isize) voidptr {
2222
$if trace_vmemcpy ? {
2323
C.fprintf(C.stderr, c'vmemcpy dest: %p src: %p n: %6ld\n', dest, const_src, n)
2424
}
25+
$if trace_vmemcpy_nulls ? {
26+
if dest == unsafe { 0 } || const_src == unsafe { 0 } {
27+
C.fprintf(C.stderr, c'vmemcpy null pointers passed, dest: %p src: %p n: %6ld\n',
28+
dest, const_src, n)
29+
print_backtrace()
30+
}
31+
}
2532
unsafe {
2633
return C.memcpy(dest, const_src, n)
2734
}
@@ -71,6 +78,13 @@ pub fn vmemset(s voidptr, c int, n isize) voidptr {
7178
$if trace_vmemset ? {
7279
C.fprintf(C.stderr, c'vmemset s: %p c: %d n: %6ld\n', s, c, n)
7380
}
81+
$if trace_vmemset_nulls ? {
82+
if s == unsafe { 0 } {
83+
C.fprintf(C.stderr, c'vmemset null pointers passed s: %p, c: %6d, n: %6ld\n',
84+
s, c, n)
85+
print_backtrace()
86+
}
87+
}
7488
unsafe {
7589
return C.memset(s, c, n)
7690
}
@@ -80,5 +94,19 @@ type FnSortCB = fn (const_a voidptr, const_b voidptr) int
8094

8195
@[inline; unsafe]
8296
fn vqsort(base voidptr, nmemb usize, size usize, sort_cb FnSortCB) {
97+
$if trace_vqsort ? {
98+
C.fprintf(C.stderr, c'vqsort base: %p, nmemb: %6uld, size: %6uld, sort_cb: %p\n',
99+
base, nmemb, size, sort_cb)
100+
}
101+
if nmemb == 0 {
102+
return
103+
}
104+
$if trace_vqsort_nulls ? {
105+
if base == unsafe { 0 } || u64(sort_cb) == 0 {
106+
C.fprintf(C.stderr, c'vqsort null pointers passed base: %p, nmemb: %6uld, size: %6uld, sort_cb: %p\n',
107+
base, nmemb, size, sort_cb)
108+
print_backtrace()
109+
}
110+
}
83111
C.qsort(base, nmemb, size, voidptr(sort_cb))
84112
}

vlib/v/gen/c/auto_str_methods.v

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ fn (mut g Gen) gen_str_for_multi_return(info ast.MultiReturn, styp string, str_f
310310
g.definitions.writeln('${g.static_non_parallel}string ${str_fn_name}(${styp} a); // auto')
311311
mut fn_builder := strings.new_builder(512)
312312
fn_builder.writeln('${g.static_non_parallel}string ${str_fn_name}(${styp} a) {')
313-
fn_builder.writeln('\tstrings__Builder sb = strings__new_builder(${info.types.len} * 10);')
313+
fn_builder.writeln('\tstrings__Builder sb = strings__new_builder(2 + ${info.types.len} * 10);')
314314
fn_builder.writeln('\tstrings__Builder_write_string(&sb, _SLIT("("));')
315315
for i, typ in info.types {
316316
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
612612
g.auto_str_funcs.writeln('${g.static_non_parallel}string ${str_fn_name}(${styp} a) { return indent_${str_fn_name}(a, 0);}')
613613
g.definitions.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${styp} a, int indent_count); // auto')
614614
g.auto_str_funcs.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${styp} a, int indent_count) {')
615-
g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(a.len * 10);')
615+
g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(2 + a.len * 10);')
616616
g.auto_str_funcs.writeln('\tstrings__Builder_write_string(&sb, _SLIT("["));')
617617
g.auto_str_funcs.writeln('\tfor (int i = 0; i < a.len; ++i) {')
618618
if sym.kind == .function {
@@ -719,7 +719,7 @@ fn (mut g Gen) gen_str_for_array_fixed(info ast.ArrayFixed, styp string, str_fn_
719719
g.auto_str_funcs.writeln('${g.static_non_parallel}string ${str_fn_name}(${def_arg}) { return indent_${str_fn_name}(a, 0);}')
720720
g.definitions.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${def_arg}, int indent_count); // auto')
721721
g.auto_str_funcs.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${def_arg}, int indent_count) {')
722-
g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(${info.size} * 10);')
722+
g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(2 + ${info.size} * 10);')
723723
g.auto_str_funcs.writeln('\tstrings__Builder_write_string(&sb, _SLIT("["));')
724724
g.auto_str_funcs.writeln('\tfor (int i = 0; i < ${info.size}; ++i) {')
725725
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) {
815815
g.auto_str_funcs.writeln('${g.static_non_parallel}string ${str_fn_name}(${styp} m) { return indent_${str_fn_name}(m, 0);}')
816816
g.definitions.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${styp} m, int indent_count); // auto')
817817
g.auto_str_funcs.writeln('${g.static_non_parallel}string indent_${str_fn_name}(${styp} m, int indent_count) { /* gen_str_for_map */')
818-
g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(m.key_values.len*10);')
818+
g.auto_str_funcs.writeln('\tstrings__Builder sb = strings__new_builder(2 + m.key_values.len * 10);')
819819
g.auto_str_funcs.writeln('\tstrings__Builder_write_string(&sb, _SLIT("{"));')
820820
g.auto_str_funcs.writeln('\tbool is_first = true;')
821821
g.auto_str_funcs.writeln('\tfor (int i = 0; i < m.key_values.len; ++i) {')

vlib/v/gen/c/dumpexpr.v

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ fn (mut g Gen) dump_expr_definitions() {
207207
'\tstring_free(&value);')
208208
}
209209
surrounder.add('
210-
strings__Builder sb = strings__new_builder(256);
210+
strings__Builder sb = strings__new_builder(64);
211211
', '
212212
string res;
213213
res = strings__Builder_str(&sb);
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
fn test_sort_with_compare_works_even_in_sanitized_modes() {
2+
mut a := []int{}
3+
dump(a.data)
4+
dump(a.len)
5+
dump(a.cap)
6+
a.sort()
7+
dump(a)
8+
a.sort_with_compare(fn (sa &string, sb &string) int {
9+
assert false, 'this should not be called, the array len is 0'
10+
return (*sa).len - (*sb).len
11+
})
12+
dump(a)
13+
assert a.len == 0
14+
15+
mut cloned_a := a.clone()
16+
dump(cloned_a.data)
17+
dump(cloned_a.len)
18+
dump(cloned_a.cap)
19+
cloned_a.sort()
20+
cloned_a.sort_with_compare(fn (sa &string, sb &string) int {
21+
assert false, 'this should not be called, the array len is 0, even after .clone()'
22+
return (*sa).len - (*sb).len
23+
})
24+
dump(cloned_a)
25+
assert cloned_a.len == 0
26+
}

0 commit comments

Comments
 (0)