From adf084eeed16dbf4b6348d750c92394406d9d8e0 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Thu, 14 Jan 2021 04:55:30 +0200 Subject: [PATCH] cgen: fix address violations for `return error(abc)`, reduce leaks in `os` --- vlib/os/os.v | 23 +++++++++++++++++------ vlib/os/os_c.v | 28 ++++++++++++++++++++++------ vlib/os/os_nix.c.v | 11 ++++++++--- vlib/v/gen/cgen.v | 5 ++++- 4 files changed, 51 insertions(+), 16 deletions(-) diff --git a/vlib/os/os.v b/vlib/os/os.v index 90e473f040..7084067126 100644 --- a/vlib/os/os.v +++ b/vlib/os/os.v @@ -152,10 +152,11 @@ pub fn rmdir_all(path string) ? { mut ret_err := '' items := ls(path) ? for item in items { - if is_dir(join_path(path, item)) { - rmdir_all(join_path(path, item)) + fullpath := join_path(path, item) + if is_dir(fullpath) { + rmdir_all(fullpath) } - rm(join_path(path, item)) or { ret_err = err } + rm(fullpath) or { ret_err = err } } rmdir(path) or { ret_err = err } if ret_err.len > 0 { @@ -402,13 +403,16 @@ pub fn is_abs_path(path string) bool { } // join_path returns a path as string from input string parameter(s). +[manualfree] pub fn join_path(base string, dirs ...string) string { mut result := []string{} result << base.trim_right('\\/') for d in dirs { result << d } - return result.join(path_separator) + res := result.join(path_separator) + unsafe { result.free() } + return res } // walk_ext returns a recursive list of all files in `path` ending with `ext`. @@ -555,13 +559,20 @@ pub fn vmodules_paths() []string { // See https://discordapp.com/channels/592103645835821068/592294828432424960/630806741373943808 // It gives a convenient way to access program resources like images, fonts, sounds and so on, // *no matter* how the program was started, and what is the current working directory. +[manualfree] pub fn resource_abs_path(path string) string { - mut base_path := real_path(dir(executable())) + exe := executable() + dexe := dir(exe) + mut base_path := real_path(dexe) vresource := getenv('V_RESOURCE_PATH') if vresource.len != 0 { base_path = vresource } - return real_path(join_path(base_path, path)) + fp := join_path(base_path, path) + res := real_path(fp) + fp.free() + base_path.free() + return res } pub struct Uname { diff --git a/vlib/os/os_c.v b/vlib/os/os_c.v index 8a212d430c..369971617c 100644 --- a/vlib/os/os_c.v +++ b/vlib/os/os_c.v @@ -46,6 +46,7 @@ struct C.dirent { } // read_bytes returns all bytes read from file in `path`. +[manualfree] pub fn read_bytes(path string) ?[]byte { mut fp := vfopen(path, 'rb') ? cseek := C.fseek(fp, 0, C.SEEK_END) @@ -63,7 +64,9 @@ pub fn read_bytes(path string) ?[]byte { return error('fread failed') } C.fclose(fp) - return res[0..nr_read_elements * fsize] + fres := res[0..nr_read_elements * fsize].clone() + unsafe { res.free() } + return fres } // read_file reads the file in `path` and returns the contents. @@ -580,15 +583,18 @@ pub fn on_segfault(f voidptr) { // executable returns the path name of the executable that started the current // process. +[manualfree] pub fn executable() string { $if linux { - mut result := vcalloc(max_path_len) - count := C.readlink('/proc/self/exe', charptr(result), max_path_len) + mut xresult := vcalloc(max_path_len) + count := C.readlink('/proc/self/exe', charptr(xresult), max_path_len) if count < 0 { eprintln('os.executable() failed at reading /proc/self/exe to get exe path') return executable_fallback() } - return unsafe { result.vstring() } + res := unsafe { xresult.vstring() }.clone() + unsafe { free(xresult) } + return res } $if windows { max := 512 @@ -713,15 +719,19 @@ pub fn getwd() string { max := 512 // max_path_len * sizeof(wchar_t) buf := &u16(vcalloc(max * 2)) if C._wgetcwd(buf, max) == 0 { + free(buf) return '' } return string_from_wide(buf) } $else { buf := vcalloc(512) if C.getcwd(charptr(buf), 512) == 0 { + free(buf) return '' } - return unsafe { buf.vstring() } + res := unsafe { buf.vstring() }.clone() + free(buf) + return res } } @@ -730,8 +740,12 @@ pub fn getwd() string { // Also https://insanecoding.blogspot.com/2007/11/pathmax-simply-isnt.html // and https://insanecoding.blogspot.com/2007/11/implementing-realpath-in-c.html // NB: this particular rabbit hole is *deep* ... +[manualfree] pub fn real_path(fpath string) string { mut fullpath := vcalloc(max_path_len) + defer { + unsafe { free(fullpath) } + } mut ret := charptr(0) $if windows { ret = charptr(C._fullpath(charptr(fullpath), charptr(fpath.str), max_path_len)) @@ -745,7 +759,9 @@ pub fn real_path(fpath string) string { } } res := unsafe { fullpath.vstring() } - return normalize_drive_letter(res) + nres := normalize_drive_letter(res) + cres := nres.clone() + return cres } fn normalize_drive_letter(path string) string { diff --git a/vlib/os/os_nix.c.v b/vlib/os/os_nix.c.v index 17d46f7917..929384fc2e 100644 --- a/vlib/os/os_nix.c.v +++ b/vlib/os/os_nix.c.v @@ -90,10 +90,15 @@ pub fn ls(path string) ?[]string { if isnil(ent) { break } - name := tos_clone(byteptr(ent.d_name)) - if name != '.' && name != '..' && name != '' { - res << name + bptr := byteptr(ent.d_name) + unsafe { + if bptr[0] == 0 || + (bptr[0] == `.` && bptr[1] == 0) || + (bptr[0] == `.` && bptr[1] == `.` && bptr[2] == 0) { + continue + } } + res << tos_clone(bptr) } C.closedir(dir) return res diff --git a/vlib/v/gen/cgen.v b/vlib/v/gen/cgen.v index 7d50b153dc..f05214367a 100644 --- a/vlib/v/gen/cgen.v +++ b/vlib/v/gen/cgen.v @@ -4160,7 +4160,10 @@ fn (mut g Gen) return_statement(node ast.Return) { g.expr_with_cast(node.exprs[0], node.types[0], g.fn_decl.return_type) g.writeln(';') styp := g.typ(g.fn_decl.return_type) - g.writeln('return *($styp*)&$tmp;') + err_obj := g.new_tmp_var() + g.writeln('$styp $err_obj;') + g.writeln('memcpy(&$err_obj, &$tmp, sizeof(Option));') + g.writeln('return $err_obj;') return } }