Commit a038ef3570

LemonBoy <thatlemon@gmail.com>
2019-05-11 21:06:31
Assemble asm files using CC
Stuffing all the files together and compiling the resulting blob with the main program is a terrible idea. Some files, namely the .S ones, must be run trough the C preprocessor before assembling them (#2437). Beside that the aggregate may be mis-compiled due to the presence of some flags that affect the following code. For example let's consider two files, a.s and b.s a.s ``` fn1: ret .data data1: .word 0 ``` b.s ``` fn2: ret ``` Now, fn1 and fn2 will be both placed in the .text section as intended if the two files are compiled separately. But if we merge them the `.data` flag ends up placing fn2 in the wrong section! This fixes a nasty crash where musl's memset ended up in the non-executable data segment, leading to too many hours of head-scratching.
1 parent 6cf7fb1
src/cache_hash.cpp
@@ -470,9 +470,6 @@ Error cache_add_dep_file(CacheHash *ch, Buf *dep_file_path, bool verbose) {
     Error err;
     Buf *contents = buf_alloc();
     if ((err = os_fetch_file_path(dep_file_path, contents))) {
-        if (verbose) {
-            fprintf(stderr, "unable to read .d file: %s\n", err_str(err));
-        }
         return ErrorReadingDepFile;
     }
     SplitIterator it = memSplit(buf_to_slice(contents), str("\r\n"));
src/codegen.cpp
@@ -8334,8 +8334,6 @@ void add_cc_args(CodeGen *g, ZigList<const char *> &args, const char *out_dep_pa
     for (size_t arg_i = 0; arg_i < g->clang_argv_len; arg_i += 1) {
         args.append(g->clang_argv[arg_i]);
     }
-
-
 }
 
 void codegen_translate_c(CodeGen *g, Buf *full_path, FILE *out_file, bool use_userland_implementation) {
@@ -8582,24 +8580,6 @@ static void gen_root_source(CodeGen *g) {
 
 }
 
-void codegen_add_assembly(CodeGen *g, Buf *path) {
-    g->assembly_files.append(path);
-}
-
-static void gen_global_asm(CodeGen *g) {
-    Buf contents = BUF_INIT;
-    Error err;
-    for (size_t i = 0; i < g->assembly_files.length; i += 1) {
-        Buf *asm_file = g->assembly_files.at(i);
-        // No need to use the caching system for these fetches because they
-        // are handled separately.
-        if ((err = os_fetch_file_path(asm_file, &contents))) {
-            zig_panic("Unable to read %s: %s", buf_ptr(asm_file), err_str(err));
-        }
-        buf_append_buf(&g->global_asm, &contents);
-    }
-}
-
 static void print_zig_cc_cmd(const char *zig_exe, ZigList<const char *> *args) {
     fprintf(stderr, "%s", zig_exe);
     for (size_t arg_i = 0; arg_i < args->length; arg_i += 1) {
@@ -8750,8 +8730,12 @@ static void gen_c_object(CodeGen *g, Buf *self_exe_path, CFile *c_file) {
 
         // add the files depended on to the cache system
         if ((err = cache_add_dep_file(cache_hash, out_dep_path, true))) {
-            fprintf(stderr, "Failed to add C source dependencies to cache: %s\n", err_str(err));
-            exit(1);
+            // Don't treat the absence of the .d file as a fatal error, the
+            // compiler may not produce one eg. when compiling .s files
+            if (err != ErrorReadingDepFile) {
+                fprintf(stderr, "Failed to add C source dependencies to cache: %s\n", err_str(err));
+                exit(1);
+            }
         }
         os_delete_file(out_dep_path);
 
@@ -9330,8 +9314,6 @@ static Error check_cache(CodeGen *g, Buf *manifest_dir, Buf *digest) {
     cache_list_of_buf(ch, g->darwin_frameworks.items, g->darwin_frameworks.length);
     cache_list_of_buf(ch, g->rpath_list.items, g->rpath_list.length);
     cache_list_of_buf(ch, g->forbidden_libs.items, g->forbidden_libs.length);
-    cache_list_of_file(ch, g->assembly_files.items, g->assembly_files.length);
-    cache_int(ch, g->emit_file_type);
     cache_int(ch, g->build_mode);
     cache_int(ch, g->out_type);
     cache_bool(ch, g->zig_target->is_native);
@@ -9392,7 +9374,7 @@ static Error check_cache(CodeGen *g, Buf *manifest_dir, Buf *digest) {
 }
 
 static bool need_llvm_module(CodeGen *g) {
-    return g->assembly_files.length != 0 || buf_len(&g->root_package->root_src_path) != 0;
+    return buf_len(&g->root_package->root_src_path) != 0;
 }
 
 static void resolve_out_paths(CodeGen *g) {
@@ -9499,7 +9481,6 @@ void codegen_build_and_link(CodeGen *g) {
 
             codegen_add_time_event(g, "Semantic Analysis");
 
-            gen_global_asm(g);
             gen_root_source(g);
 
         }
src/link.cpp
@@ -59,14 +59,6 @@ static const char *build_libc_object(CodeGen *parent_gen, const char *name, CFil
     return buf_ptr(&child_gen->output_file_path);
 }
 
-static const char *build_asm_object(CodeGen *parent_gen, const char *name, Buf *file) {
-    CodeGen *child_gen = create_child_codegen(parent_gen, nullptr, OutTypeObj, nullptr);
-    codegen_set_out_name(child_gen, buf_create_from_str(name));
-    codegen_add_assembly(child_gen, file);
-    codegen_build_and_link(child_gen);
-    return buf_ptr(&child_gen->output_file_path);
-}
-
 static const char *path_from_zig_lib(CodeGen *g, const char *dir, const char *subpath) {
     Buf *dir1 = buf_alloc();
     os_path_join(g->zig_lib_dir, buf_create_from_str(dir), dir1);
@@ -140,6 +132,7 @@ static const char *build_libunwind(CodeGen *parent) {
         c_file->args.append(path_from_libunwind(parent, "include"));
         c_file->args.append("-fPIC");
         c_file->args.append("-D_LIBUNWIND_DISABLE_VISIBILITY_ANNOTATIONS");
+        c_file->args.append("-Wa,--noexecstack");
         if (parent->zig_target->is_native) {
             c_file->args.append("-D_LIBUNWIND_IS_NATIVE_ONLY");
         }
@@ -406,12 +399,13 @@ static const char *musl_arch_name(const ZigTarget *target) {
     }
 }
 
-static Buf *musl_start_asm_path(CodeGen *parent, const char *file) {
-    return buf_sprintf("%s" OS_SEP "libc" OS_SEP "musl" OS_SEP "crt" OS_SEP "%s" OS_SEP "%s",
-        buf_ptr(parent->zig_lib_dir), musl_arch_name(parent->zig_target), file);
+static const char *musl_start_asm_path(CodeGen *parent, const char *file) {
+    Buf *result = buf_sprintf("%s" OS_SEP "libc" OS_SEP "musl" OS_SEP "crt" OS_SEP "%s" OS_SEP "%s",
+                   buf_ptr(parent->zig_lib_dir), musl_arch_name(parent->zig_target), file);
+    return buf_ptr(result);
 }
 
-static void musl_add_cc_args(CodeGen *parent, CFile *c_file) {
+static void musl_add_cc_args(CodeGen *parent, CFile *c_file, bool want_O3) {
     c_file->args.append("-std=c99");
     c_file->args.append("-ffreestanding");
     // Musl adds these args to builds with gcc but clang does not support them. 
@@ -448,7 +442,11 @@ static void musl_add_cc_args(CodeGen *parent, CFile *c_file) {
     c_file->args.append(buf_ptr(buf_sprintf("%s" OS_SEP "libc" OS_SEP "include" OS_SEP "generic-musl",
             buf_ptr(parent->zig_lib_dir))));
 
-    c_file->args.append("-Os");
+    if (want_O3)
+        c_file->args.append("-O3");
+    else
+        c_file->args.append("-Os");
+
     c_file->args.append("-fomit-frame-pointer");
     c_file->args.append("-fno-unwind-tables");
     c_file->args.append("-fno-asynchronous-unwind-tables");
@@ -574,19 +572,14 @@ static const char *build_musl(CodeGen *parent) {
         Buf *full_path = buf_sprintf("%s" OS_SEP "libc" OS_SEP "%s",
                 buf_ptr(parent->zig_lib_dir), buf_ptr(src_file));
 
-        if (src_kind == MuslSrcAsm) {
-            codegen_add_assembly(child_gen, full_path);
-        } else {
-            CFile *c_file = allocate<CFile>(1);
-            c_file->source_path = buf_ptr(full_path);
-            musl_add_cc_args(parent, c_file);
-            c_file->args.append("-Qunused-arguments");
-            c_file->args.append("-w"); // disable all warnings
-            if (src_kind == MuslSrcO3) {
-                c_file->args.append("-O3");
-            }
-            c_source_files.append(c_file);
-        }
+        CFile *c_file = allocate<CFile>(1);
+        c_file->source_path = buf_ptr(full_path);
+
+        musl_add_cc_args(parent, c_file, src_kind == MuslSrcO3);
+        c_file->args.append("-Qunused-arguments");
+        c_file->args.append("-w"); // disable all warnings
+
+        c_source_files.append(c_file);
     }
 
     child_gen->c_source_files = c_source_files;
@@ -744,20 +737,28 @@ static const char *get_libc_crt_file(CodeGen *parent, const char *file) {
         }
     } else if (parent->libc == nullptr && target_is_musl(parent->zig_target)) {
         if (strcmp(file, "crti.o") == 0) {
-            return build_asm_object(parent, "crti", musl_start_asm_path(parent, "crti.s"));
+            CFile *c_file = allocate<CFile>(1);
+            c_file->source_path = musl_start_asm_path(parent, "crti.s");
+            musl_add_cc_args(parent, c_file, false);
+            c_file->args.append("-Qunused-arguments");
+            return build_libc_object(parent, "crti", c_file);
         } else if (strcmp(file, "crtn.o") == 0) {
-            return build_asm_object(parent, "crtn", musl_start_asm_path(parent, "crtn.s"));
+            CFile *c_file = allocate<CFile>(1);
+            c_file->source_path = musl_start_asm_path(parent, "crtn.s");
+            c_file->args.append("-Qunused-arguments");
+            musl_add_cc_args(parent, c_file, false);
+            return build_libc_object(parent, "crtn", c_file);
         } else if (strcmp(file, "crt1.o") == 0) {
             CFile *c_file = allocate<CFile>(1);
             c_file->source_path = path_from_libc(parent, "musl" OS_SEP "crt" OS_SEP "crt1.c");
-            musl_add_cc_args(parent, c_file);
+            musl_add_cc_args(parent, c_file, false);
             c_file->args.append("-fno-stack-protector");
             c_file->args.append("-DCRT");
             return build_libc_object(parent, "crt1", c_file);
         } else if (strcmp(file, "Scrt1.o") == 0) {
             CFile *c_file = allocate<CFile>(1);
             c_file->source_path = path_from_libc(parent, "musl" OS_SEP "crt" OS_SEP "Scrt1.c");
-            musl_add_cc_args(parent, c_file);
+            musl_add_cc_args(parent, c_file, false);
             c_file->args.append("-fPIC");
             c_file->args.append("-fno-stack-protector");
             c_file->args.append("-DCRT");
src/main.cpp
@@ -48,7 +48,6 @@ static int print_full_usage(const char *arg0, FILE *file, int return_code) {
         "  zen                          print zen of zig and exit\n"
         "\n"
         "Compile Options:\n"
-        "  --assembly [source]          add assembly file to build\n"
         "  --c-source [options] [file]  compile C source code\n"
         "  --cache-dir [path]           override the local cache directory\n"
         "  --cache [auto|off|on]        build in cache, print output path to stdout\n"
@@ -428,7 +427,6 @@ int main(int argc, char **argv) {
     bool each_lib_rpath = false;
     ZigList<const char *> objects = {0};
     ZigList<CFile *> c_source_files = {0};
-    ZigList<const char *> asm_files = {0};
     const char *test_filter = nullptr;
     const char *test_name_prefix = nullptr;
     size_t ver_major = 0;
@@ -774,8 +772,6 @@ int main(int argc, char **argv) {
                             break;
                         }
                     }
-                } else if (strcmp(arg, "--assembly") == 0) {
-                    asm_files.append(argv[i]);
                 } else if (strcmp(arg, "--cache-dir") == 0) {
                     cache_dir = argv[i];
                 } else if (strcmp(arg, "-target") == 0) {
@@ -971,14 +967,13 @@ int main(int argc, char **argv) {
     case CmdTranslateCUserland:
     case CmdTest:
         {
-            if (cmd == CmdBuild && !in_file && objects.length == 0 && asm_files.length == 0 &&
+            if (cmd == CmdBuild && !in_file && objects.length == 0 &&
                     c_source_files.length == 0)
             {
                 fprintf(stderr,
                     "Expected at least one of these things:\n"
                     " * Zig root source file argument\n"
                     " * --object argument\n"
-                    " * --assembly argument\n"
                     " * --c-source argument\n");
                 return print_error_usage(arg0);
             } else if ((cmd == CmdTranslateC || cmd == CmdTranslateCUserland ||
@@ -1130,9 +1125,6 @@ int main(int argc, char **argv) {
                 for (size_t i = 0; i < objects.length; i += 1) {
                     codegen_add_object(g, buf_create_from_str(objects.at(i)));
                 }
-                for (size_t i = 0; i < asm_files.length; i += 1) {
-                    codegen_add_assembly(g, buf_create_from_str(asm_files.at(i)));
-                }
             }
 
 
std/build.zig
@@ -1376,7 +1376,7 @@ pub const LibExeObjStep = struct {
                     try zig_args.append(name);
                 },
                 LinkObject.AssemblyFile => |asm_file| {
-                    try zig_args.append("--assembly");
+                    try zig_args.append("--c-source");
                     try zig_args.append(builder.pathFromRoot(asm_file));
                 },
                 LinkObject.CSourceFile => |c_source_file| {