Skip to content

Commit

Permalink
builtin,cgen: fix issues found with the stricter sanitizers in clang-18
Browse files Browse the repository at this point in the history
  • Loading branch information
spytheman committed Feb 13, 2025
1 parent e9c2314 commit f460ae8
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 13 deletions.
12 changes: 7 additions & 5 deletions vlib/builtin/array.v
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
25 changes: 22 additions & 3 deletions vlib/builtin/builtin.c.v
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 abort_on_free_null ? {
C.abort()

Check failure on line 686 in vlib/builtin/builtin.c.v

View workflow job for this annotation

GitHub Actions / build-vc

unknown function: C.abort

Check failure on line 686 in vlib/builtin/builtin.c.v

View workflow job for this annotation

GitHub Actions / bootstrap-v (ubuntu-latest)

unknown function: C.abort

Check failure on line 686 in vlib/builtin/builtin.c.v

View workflow job for this annotation

GitHub Actions / bootstrap-v (macos-14)

unknown function: C.abort

Check failure on line 686 in vlib/builtin/builtin.c.v

View workflow job for this annotation

GitHub Actions / linux-cross

unknown function: C.abort
}
return
}
$if prealloc {
return
} $else $if gcboehm ? {
Expand Down
28 changes: 28 additions & 0 deletions vlib/builtin/cfns_wrapper.c.v
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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))
}
8 changes: 4 additions & 4 deletions vlib/v/gen/c/auto_str_methods.v
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {')
Expand Down
2 changes: 1 addition & 1 deletion vlib/v/gen/c/dumpexpr.v
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit f460ae8

Please sign in to comment.