Compiler: use safer string functions

ast/decl.c2

@@ -20,6 +20,7 @@ import attr local;
 import string_buffer;
 
 import stdio;
+import string;
 
 // TODO better order (most used first)
 public type DeclKind enum u8 (const char* const name) {
@@ -327,7 +328,7 @@ public fn const char* Decl.getFullName(const Decl* d) {
         }
         break;
     case Import:
-        stdio.snprintf(tmp, tmp_size, "%s", d.getName());
+        string.pstrcpy(tmp, tmp_size, d.getName());
         break;
     case EnumConstant:
         QualType qt = d.getType();

ast/statistics.c2

@@ -109,8 +109,10 @@ fn void Stat.dump(const Stat* ss, const char* name, StatPack* sp) {
     char[10] buf = "";
     if (count) {
         u32 avg = (u32)(size * (u64)100 / count);
-        i32 len = snprintf(buf, sizeof(buf), "%3d   ", avg / 100);
-        if (avg % 100) snprintf(buf + len - 3, sizeof(buf) - (len - 3), ".%02d", avg % 100);
+        u32 whole = avg / 100;
+        u32 frac = avg % 100;
+        if (frac) snprintf(buf, elemsof(buf), "%3d.%02d", whole, frac);
+        else snprintf(buf, elemsof(buf), "%3d   ", whole);
     }
     printf("  %20s  %6d  %7d  %s", name, count, size, buf);
     bool output = false;

bootstrap/globals.c

@@ -33,16 +33,62 @@ int pf(const char *fmt, ...) {
 void nothing(void *p) {
 }
 
+size_t pstrcpy(char *dest, size_t size, const char *src) {
+    /*@API utils.string
+       Copy the string pointed by `src` to the destination array `dest`,
+       of length `size` bytes, truncating excess bytes.
+
+       @param `dest` destination array, must be a valid pointer.
+       @param `size` length of destination array in bytes.
+       @param `src` pointer to a source string, must be a valid pointer.
+       @return the length of the resulting string or `size` if truncation
+       occurred.  `dest` is null terminated unless `size` is `0`.
+       @note: truncation can be detected reliably by comparing the return
+       value with the destination size.
+       @note: this function does what many programmers wrongly expect
+       `strncpy` to do. `strncpy` has different semantics and does not
+       null terminate the destination array in case of excess bytes.
+       **NEVER use `strncpy`**.
+       @note: use this function to concatenate strings efficiently and safely:
+       ```
+       bool make_greeting(char *dest, size_t size, const char *name) {
+           size_t pos = 0;
+           pos += pstrcpy(dest + pos, size - pos, "Hello ");
+           pos += pstrcpy(dest + pos, size - pos, name);
+           pos += pstrcpy(dest + pos, size - pos, "! The world is safe\n");
+           return pos < size;
+       }
+       ```
+     */
+    for (size_t i = 0; i + 1 < size; i++) {
+        if ((dest[i] = *src++) == '\0')
+            return i;
+    }
+    if (size) {
+        dest[size - 1] = '\0';
+    }
+    return size;
+}
+
 void preprocess_header(const char *header_name) {
     char cmd[200];
+    size_t pos;
     printf("// preprocessor output of <%s>\n{\n", header_name);
     fflush(stdout);
-    snprintf(cmd, sizeof(cmd), "echo '#include <%s>' | cc -E -", header_name);
+    pos = snprintf(cmd, sizeof(cmd), "echo '#include <%s>' | cc -E -", header_name);
+    if (pos > sizeof(cmd)) {
+        pos = sizeof(cmd);
+    }
     if (!verbose) {
-        strcat(cmd, " | grep -v '^#'");
-        strcat(cmd, " | tr -s '\n'");
+        pos += pstrcpy(cmd + pos, sizeof(cmd) - pos, " | grep -v '^#'");
+        pos += pstrcpy(cmd + pos, sizeof(cmd) - pos, " | tr -s '\n'");
+    }
+    if (pos >= sizeof(cmd)) {
+        printf("command too long: %s\n", cmd);
+    } else
+    if (system(cmd)) {
+        printf("system failed (%s): %s\n", strerror(errno), cmd);
     }
-    system(cmd);
     printf("}\n");
 }
 

common/utils.c2

@@ -16,7 +16,7 @@
 module utils;
 
 import constants;
-import file_utils;
+import file_utils local;
 
 import stdio local;
 import string local;
@@ -35,8 +35,8 @@ public fn u64 now() @(unused) {
 
 // Note: both paths may be empty or should end in a /
 public type PathInfo struct {
-    char[file_utils.Max_path] orig2root; // subdir/subdir/
-    char[file_utils.Max_path] root2orig; // ../../
+    char[Max_path] orig2root; // subdir/subdir/
+    char[Max_path] root2orig; // ../../
 }
 
 public fn bool PathInfo.hasSubdir(const PathInfo* pi) {
@@ -64,7 +64,7 @@ public fn u32 changeToProjectDir(PathInfo* info, const char* other_dir) {
     info.orig2root[len] = '/';
     info.orig2root[len+1] = 0;
 
-    strcpy(info.root2orig, other_dir);
+    pstrcpy(info.root2orig, elemsof(info.root2orig), other_dir);
     // add '/' if not present
     len = strlen(other_dir);
     if (other_dir[len-1] != '/') {
@@ -79,34 +79,36 @@ public fn u32 changeToProjectDir(PathInfo* info, const char* other_dir) {
 }
 
 // info may be nil
+// XXX: this function changes the current directory if recipe file
+//      is not present there.
 public fn bool findProjectDir(PathInfo* info) {
     if (info) {
         info.root2orig[0] = 0;
         info.orig2root[0] = 0;
     }
-    char[file_utils.Max_path] base_path;
-    char[file_utils.Max_path] rel_path;
+    char[Max_path] base_path;
+    char[Max_path] rel_path;
     base_path[0] = 0;
     rel_path[0] = 0;
 
-    char[file_utils.Max_path] buffer;
+    char[Max_path] buffer;
     while (1) {
         char* path = unistd.getcwd(buffer, elemsof(buffer)-1);  // -1 for trailing slash
         if (path == nil) {
             perror("getcwd");
             return false;
         }
         if (base_path[0] == 0) {
-            strcpy(base_path, path);
-            strcat(base_path, "/");
+            usize pos = pstrcpy(base_path, elemsof(base_path), path);
+            pstrcpy(base_path + pos, elemsof(base_path) - pos, "/");
         }
 
         if (checkFile(constants.recipe_name)) {
             char* path_prefix = base_path + strlen(path);
             if (*path_prefix == '/') path_prefix++;
             if (info) {
-                strcpy(info.orig2root, path_prefix);
-                strcpy(info.root2orig, rel_path);
+                pstrcpy(info.orig2root, elemsof(info.orig2root), path_prefix);
+                pstrcpy(info.root2orig, elemsof(info.root2orig), rel_path);
             }
             return true;
         }
@@ -115,7 +117,7 @@ public fn bool findProjectDir(PathInfo* info) {
             perror("chdir");
             return false;
         }
-        strcat(rel_path, "../");
+        pstrcat(rel_path, elemsof(rel_path), "../");
     }
 }
 

compiler/compiler.c2

@@ -354,7 +354,7 @@ fn void Compiler.build(Compiler* c,
     info.addSource = Compiler.add_source;
     info.register_attr = Compiler.register_attr;
     info.fn_arg = c;
-    string.strcpy(info.target_name, c.auxPool.idx2str(target.getNameIdx()));
+    string.pstrcpy(info.target_name, elemsof(info.target_name), c.auxPool.idx2str(target.getNameIdx()));
 
     // create output directory
     if (!file_utils.make_path(info.output_dir, elemsof(info.output_dir), output_base, c.auxPool.idx2str(target.getNameIdx()))

compiler/main.c2

@@ -574,9 +574,10 @@ fn void Context.handle_args(Context* c, i32 argc, char** argv) {
     bool have_plugin_dir = false;
     if (c.opts.build_file) {
         if (auto_found_buildfile) {
-            strcpy(c.build_filename, c.opts.build_file);
+            pstrcpy(c.build_filename, elemsof(c.build_filename), c.opts.build_file);
         } else {
-            snprintf(c.build_filename, sizeof(c.build_filename), "%s%s", c.path_info.orig2root, c.opts.build_file);
+            usize pos = pstrcpy(c.build_filename, elemsof(c.build_filename), c.path_info.orig2root);
+            pstrcpy(c.build_filename + pos, elemsof(c.build_filename) - pos, c.opts.build_file);
         }
         console.log("using build-file %s", c.build_filename);
         c.build_info = build_file_parser.parse(c.sm, c.auxPool, c.build_filename);

generator/c/c_generator_special.c2

@@ -19,13 +19,13 @@ import ast;
 import build_file;
 import build_target;
 import component;
-import file_utils;
+import file_utils local;
 import module_list;
 import string_buffer;
 import string_list;
 
 import stdio local;
-import string;
+import string local;
 
 fn void Generator.createMakefile(Generator* gen,
                                  const char* output_dir,
@@ -89,7 +89,7 @@ fn void Generator.createMakefile(Generator* gen,
         out.add("LDFLAGS+=-fsanitize=undefined\n");
     }
 
-    char[file_utils.Max_path] target_name;
+    char[Max_path] target_name;
 
     if (gen.fast_build) {
         // dont add *.c, but add (handy for switching between fast/normal build
@@ -120,7 +120,7 @@ fn void Generator.createMakefile(Generator* gen,
     switch (gen.target_kind) {
     case Image:
     case Executable:
-        string.strcpy(target_name, gen.target);
+        pstrcpy(target_name, elemsof(target_name), gen.target);
         out.print("all: ../%s\n\n", target_name);
 
         out.print("../%s: $(objects) $(headers)\n", target_name);
@@ -151,7 +151,7 @@ fn void Generator.createMakefile(Generator* gen,
 
             if (gen.targetInfo.sys == FreeBSD || gen.targetInfo.sys == OpenBSD) {
                 // ignore "dl" library on BSD systems: libc handles dlopen etc.
-                if (linkname && !string.strcmp(linkname, "dl")) continue;
+                if (linkname && !strcmp(linkname, "dl")) continue;
             }
             if (c.getKind() == ExternalStaticLib) {
                 if (c.getNameIdx() == libc_name) { // special case for static libc

generator/radix_tree/radix_tree.c2

@@ -519,7 +519,9 @@ fn void Tree.iterate(const Tree* t, u32 node_idx, Iter* iter) {
     const Node* n = t.idx2node(node_idx);
     // if we have children, dont print
     const char* word = t.idx2word(n.word_idx);
-    strcpy(iter.buf + iter.len, word);
+    usize len = strlen(word);
+    assert(iter.len + len < elemsof(iter.buf));
+    memcpy(iter.buf + iter.len, word, len + 1);
     if (n.num_children == 0) {
 #if RadixTreeValue
         iter.func(iter.arg, iter.buf, n.value);
@@ -528,7 +530,7 @@ fn void Tree.iterate(const Tree* t, u32 node_idx, Iter* iter) {
 #endif
     } else {
         u32 old_len = iter.len;
-        iter.len += (u32)strlen(word);
+        iter.len += len;
         for (u32 i=0; i<n.num_children; i++) {
             t.iterate(n.children[i], iter);
         }
@@ -544,16 +546,19 @@ public fn void Tree.find(Tree* t, const char* text, MatchFn func, void* arg) @(u
     if (node_idx == 0) return;
 
     const Node* n = t.idx2node(node_idx);
+    const char* tail = t.idx2word(n.word_idx) + cur_match;
     if (n.num_children == 0) {
 #if RadixTreeValue
-        func(arg, t.idx2word(n.word_idx) + cur_match, n.value);
+        func(arg, tail, n.value);
 #else
-        func(arg, t.idx2word(n.word_idx) + cur_match);
+        func(arg, tail);
 #endif
     } else {
         Iter iter = { func, arg, 0 }
-        strcpy(iter.buf, t.idx2word(n.word_idx) + cur_match);
-        iter.len = (u32)strlen(iter.buf);
+        u32 len = (u32)strlen(tail);
+        assert(len < elemsof(iter.buf));
+        memcpy(iter.buf, tail, len + 1);
+        iter.len = len;
         for (u32 i=0; i<n.num_children; i++) {
             u32 old_len = iter.len;
             t.iterate(n.children[i], &iter);

libs/libc/string.c2i

@@ -92,18 +92,27 @@ char* strnstr(const char* s1, const char* s2, size_t n) {
     return nil;
 }
 
+// return the length of src, but copy at most n-1 bytes
+// from src and null terminate dst.
 size_t strlcpy(char* dst, const char* src, size_t n) {
-    for (size_t i = 0; i < n; i++) {
+    size_t i = 0;
+    while (i < n) {
         if ((dst[i] = *src++) == '\0') return i;
+        i++;
     }
     if (n) dst[n - 1] = '\0';
     while (*src++) n++;
     return n;
 }
 
+// return the length of src, but copy at most n-1 bytes
+// from src and null terminate dst.
+// return the combined length of dst and src, but copy
+// at most n - strlen(dst) - 1 bytes from src and null
+// terminate dst.
 size_t strlcat(char* dst, const char* src, size_t n) {
     size_t i = 0;
-    while (i < n && dst[i]) i++;
+    while (dst[i]) i++;
     while (i < n) {
         if ((dst[i] = *src++) == '\0') return i;
         i++;

plugins/git_version_plugin.c2

@@ -42,7 +42,7 @@ fn void* load(const char* options, console.Config* config) {
     Plugin* p = calloc(1, sizeof(Plugin));
     console.init(config);
     // TODO handle options
-    strcpy(p.version, "unknown");
+    pstrcpy(p.version, elemsof(p.version), "unknown");
 
     i32 retval = process_utils.run2(".", "git", "describe --tags --always --dirty",
                                     p.version, sizeof(p.version));

tools/c2rename.c2

@@ -110,7 +110,7 @@ public fn i32 main(i32 argc, const char** argv)
             return -1;
         }
         if (count == 1) {
-            strcpy(refsname, finder.get(0));
+            pstrcpy(refsname, elemsof(refsname), finder.get(0));
             refsfile = refsname;
         } else {
             fprintf(stderr, "multiple %s files, please specify target with '-t' option\n", constants.refs_filename);

tools/tester/expect_file.c2

@@ -64,7 +64,7 @@ public type ExpectFile struct @(opaque) {
 
 public fn ExpectFile* create(const char* name, ExpectMode m) {
     ExpectFile* f = calloc(1, sizeof(ExpectFile));
-    strcpy(f.filename, name);
+    pstrcpy(f.filename, elemsof(f.filename), name);
     f.mode = m;
     f.lines = line_db.create();
     return f;