From ea5518f69edc51e8e70c2c4d4c4daa3ad9bcb242 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 13 Apr 2019 12:31:45 +0200 Subject: make os_file_close poison file handle after close This helps track down use-after-close bugs. --- src/cache_hash.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'src/cache_hash.cpp') diff --git a/src/cache_hash.cpp b/src/cache_hash.cpp index 1f25a9982e..250733ec92 100644 --- a/src/cache_hash.cpp +++ b/src/cache_hash.cpp @@ -256,10 +256,10 @@ static Error populate_file_hash(CacheHash *ch, CacheHashFile *chf, Buf *contents } if ((err = hash_file(chf->bin_digest, this_file, contents))) { - os_file_close(this_file); + os_file_close(&this_file); return err; } - os_file_close(this_file); + os_file_close(&this_file); blake2b_update(&ch->blake, chf->bin_digest, 48); @@ -300,7 +300,7 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) { Buf line_buf = BUF_INIT; buf_resize(&line_buf, 512); if ((err = os_file_read_all(ch->manifest_file, &line_buf))) { - os_file_close(ch->manifest_file); + os_file_close(&ch->manifest_file); return err; } @@ -389,14 +389,14 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) { OsFileAttr actual_attr; if ((err = os_file_open_r(chf->path, &this_file, &actual_attr))) { fprintf(stderr, "Unable to open %s\n: %s", buf_ptr(chf->path), err_str(err)); - os_file_close(ch->manifest_file); + os_file_close(&ch->manifest_file); return ErrorCacheUnavailable; } if (chf->attr.mtime.sec == actual_attr.mtime.sec && chf->attr.mtime.nsec == actual_attr.mtime.nsec && chf->attr.inode == actual_attr.inode) { - os_file_close(this_file); + os_file_close(&this_file); } else { // we have to recompute the digest. // later we'll rewrite the manifest with the new mtime/digest values @@ -411,11 +411,11 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) { uint8_t actual_digest[48]; if ((err = hash_file(actual_digest, this_file, nullptr))) { - os_file_close(this_file); - os_file_close(ch->manifest_file); + os_file_close(&this_file); + os_file_close(&ch->manifest_file); return err; } - os_file_close(this_file); + os_file_close(&this_file); if (memcmp(chf->bin_digest, actual_digest, 48) != 0) { memcpy(chf->bin_digest, actual_digest, 48); // keep going until we have the input file digests @@ -433,12 +433,12 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) { CacheHashFile *chf = &ch->files.at(file_i); if ((err = populate_file_hash(ch, chf, nullptr))) { fprintf(stderr, "Unable to hash %s: %s\n", buf_ptr(chf->path), err_str(err)); - os_file_close(ch->manifest_file); + os_file_close(&ch->manifest_file); return ErrorCacheUnavailable; } } if (return_code != ErrorNone) { - os_file_close(ch->manifest_file); + os_file_close(&ch->manifest_file); } return return_code; } @@ -453,7 +453,7 @@ Error cache_add_file_fetch(CacheHash *ch, Buf *resolved_path, Buf *contents) { CacheHashFile *chf = ch->files.add_one(); chf->path = resolved_path; if ((err = populate_file_hash(ch, chf, contents))) { - os_file_close(ch->manifest_file); + os_file_close(&ch->manifest_file); return err; } @@ -586,6 +586,6 @@ void cache_release(CacheHash *ch) { } } - os_file_close(ch->manifest_file); + os_file_close(&ch->manifest_file); } -- cgit v1.2.3 From 93e89b3b7efeaa41f4f11fbb022b962d2244dab2 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 13 Apr 2019 12:33:29 +0200 Subject: don't close cache manifest file prematurely ErrorInvalidFormat is not a fatal error so don't close the cache manifest file right away but instead let cache_final() handle it. Fixes the following (very common) warning when running the test suite: Warning: Unable to write cache file [..]: unexpected seek failure The seek failure is an lseek() system call that failed with EBADF because the file descriptor had already been closed. --- src/cache_hash.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/cache_hash.cpp') diff --git a/src/cache_hash.cpp b/src/cache_hash.cpp index 250733ec92..efb1a06b59 100644 --- a/src/cache_hash.cpp +++ b/src/cache_hash.cpp @@ -437,7 +437,7 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) { return ErrorCacheUnavailable; } } - if (return_code != ErrorNone) { + if (return_code != ErrorNone && return_code != ErrorInvalidFormat) { os_file_close(&ch->manifest_file); } return return_code; -- cgit v1.2.3 From 8ef7f6febb7132d7a1ee44199fd22006f326de5c Mon Sep 17 00:00:00 2001 From: Shawn Landden Date: Tue, 9 Apr 2019 19:16:51 -0500 Subject: remove Shebang (#!) support Closes: #2165 --- src/analyze.cpp | 2 +- src/cache_hash.cpp | 2 +- src/codegen.cpp | 6 +++--- src/libc_installation.cpp | 2 +- src/main.cpp | 4 ++-- src/os.cpp | 35 +++++------------------------------ src/os.hpp | 4 ++-- std/zig/ast.zig | 3 --- std/zig/parse.zig | 10 ---------- std/zig/parser_test.zig | 8 -------- std/zig/render.zig | 5 ----- 11 files changed, 15 insertions(+), 66 deletions(-) (limited to 'src/cache_hash.cpp') diff --git a/src/analyze.cpp b/src/analyze.cpp index 2775c05b79..64cf8f2321 100644 --- a/src/analyze.cpp +++ b/src/analyze.cpp @@ -6072,7 +6072,7 @@ Error file_fetch(CodeGen *g, Buf *resolved_path, Buf *contents) { if (g->enable_cache) { return cache_add_file_fetch(&g->cache_hash, resolved_path, contents); } else { - return os_fetch_file_path(resolved_path, contents, false); + return os_fetch_file_path(resolved_path, contents); } } diff --git a/src/cache_hash.cpp b/src/cache_hash.cpp index efb1a06b59..2a0810eced 100644 --- a/src/cache_hash.cpp +++ b/src/cache_hash.cpp @@ -469,7 +469,7 @@ Error cache_add_file(CacheHash *ch, Buf *path) { 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, false))) { + if ((err = os_fetch_file_path(dep_file_path, contents))) { if (verbose) { fprintf(stderr, "unable to read .d file: %s\n", err_str(err)); } diff --git a/src/codegen.cpp b/src/codegen.cpp index 33049abd62..6075fb6881 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -7870,7 +7870,7 @@ static Error define_builtin_compile_vars(CodeGen *g) { Buf *contents; if (hit) { contents = buf_alloc(); - if ((err = os_fetch_file_path(builtin_zig_path, contents, false))) { + if ((err = os_fetch_file_path(builtin_zig_path, contents))) { fprintf(stderr, "Unable to open '%s': %s\n", buf_ptr(builtin_zig_path), err_str(err)); exit(1); } @@ -8299,7 +8299,7 @@ static void gen_root_source(CodeGen *g) { Error err; // No need for using the caching system for this file fetch because it is handled // separately. - if ((err = os_fetch_file_path(resolved_path, source_code, true))) { + if ((err = os_fetch_file_path(resolved_path, source_code))) { fprintf(stderr, "unable to open '%s': %s\n", buf_ptr(resolved_path), err_str(err)); exit(1); } @@ -8374,7 +8374,7 @@ static void gen_global_asm(CodeGen *g) { 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, false))) { + 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); diff --git a/src/libc_installation.cpp b/src/libc_installation.cpp index 3ea17f1bdc..3e5f8b0d66 100644 --- a/src/libc_installation.cpp +++ b/src/libc_installation.cpp @@ -45,7 +45,7 @@ Error zig_libc_parse(ZigLibCInstallation *libc, Buf *libc_file, const ZigTarget bool found_keys[array_length(zig_libc_keys)] = {}; Buf *contents = buf_alloc(); - if ((err = os_fetch_file_path(libc_file, contents, false))) { + if ((err = os_fetch_file_path(libc_file, contents))) { if (err != ErrorFileNotFound && verbose) { fprintf(stderr, "Unable to read '%s': %s\n", buf_ptr(libc_file), err_str(err)); } diff --git a/src/main.cpp b/src/main.cpp index 03cf3aad68..8cbc9b0d8c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -331,7 +331,7 @@ int main(int argc, char **argv) { os_path_split(cwd, nullptr, cwd_basename); Buf *build_zig_contents = buf_alloc(); - if ((err = os_fetch_file_path(build_zig_path, build_zig_contents, false))) { + if ((err = os_fetch_file_path(build_zig_path, build_zig_contents))) { fprintf(stderr, "Unable to read %s: %s\n", buf_ptr(build_zig_path), err_str(err)); return EXIT_FAILURE; } @@ -346,7 +346,7 @@ int main(int argc, char **argv) { } Buf *main_zig_contents = buf_alloc(); - if ((err = os_fetch_file_path(main_zig_path, main_zig_contents, false))) { + if ((err = os_fetch_file_path(main_zig_path, main_zig_contents))) { fprintf(stderr, "Unable to read %s: %s\n", buf_ptr(main_zig_path), err_str(err)); return EXIT_FAILURE; } diff --git a/src/os.cpp b/src/os.cpp index 60c66908cc..49d56f2d5d 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -751,39 +751,15 @@ Buf os_path_resolve(Buf **paths_ptr, size_t paths_len) { #endif } -Error os_fetch_file(FILE *f, Buf *out_buf, bool skip_shebang) { +Error os_fetch_file(FILE *f, Buf *out_buf) { static const ssize_t buf_size = 0x2000; buf_resize(out_buf, buf_size); ssize_t actual_buf_len = 0; - bool first_read = true; - for (;;) { size_t amt_read = fread(buf_ptr(out_buf) + actual_buf_len, 1, buf_size, f); actual_buf_len += amt_read; - if (skip_shebang && first_read && buf_starts_with_str(out_buf, "#!")) { - size_t i = 0; - while (true) { - if (i > buf_len(out_buf)) { - zig_panic("shebang line exceeded %zd characters", buf_size); - } - - size_t current_pos = i; - i += 1; - - if (out_buf->list.at(current_pos) == '\n') { - break; - } - } - - ZigList *list = &out_buf->list; - memmove(list->items, list->items + i, list->length - i); - list->length -= i; - - actual_buf_len -= i; - } - if (amt_read != buf_size) { if (feof(f)) { buf_resize(out_buf, actual_buf_len); @@ -794,7 +770,6 @@ Error os_fetch_file(FILE *f, Buf *out_buf, bool skip_shebang) { } buf_resize(out_buf, actual_buf_len + buf_size); - first_read = false; } zig_unreachable(); } @@ -864,8 +839,8 @@ static Error os_exec_process_posix(const char *exe, ZigList &args, FILE *stdout_f = fdopen(stdout_pipe[0], "rb"); FILE *stderr_f = fdopen(stderr_pipe[0], "rb"); - Error err1 = os_fetch_file(stdout_f, out_stdout, false); - Error err2 = os_fetch_file(stderr_f, out_stderr, false); + Error err1 = os_fetch_file(stdout_f, out_stdout); + Error err2 = os_fetch_file(stderr_f, out_stderr); fclose(stdout_f); fclose(stderr_f); @@ -1097,7 +1072,7 @@ Error os_copy_file(Buf *src_path, Buf *dest_path) { } } -Error os_fetch_file_path(Buf *full_path, Buf *out_contents, bool skip_shebang) { +Error os_fetch_file_path(Buf *full_path, Buf *out_contents) { FILE *f = fopen(buf_ptr(full_path), "rb"); if (!f) { switch (errno) { @@ -1116,7 +1091,7 @@ Error os_fetch_file_path(Buf *full_path, Buf *out_contents, bool skip_shebang) { return ErrorFileSystem; } } - Error result = os_fetch_file(f, out_contents, skip_shebang); + Error result = os_fetch_file(f, out_contents); fclose(f); return result; } diff --git a/src/os.hpp b/src/os.hpp index b301937e83..058bb2020c 100644 --- a/src/os.hpp +++ b/src/os.hpp @@ -126,8 +126,8 @@ void os_file_close(OsFile *file); Error ATTRIBUTE_MUST_USE os_write_file(Buf *full_path, Buf *contents); Error ATTRIBUTE_MUST_USE os_copy_file(Buf *src_path, Buf *dest_path); -Error ATTRIBUTE_MUST_USE os_fetch_file(FILE *file, Buf *out_contents, bool skip_shebang); -Error ATTRIBUTE_MUST_USE os_fetch_file_path(Buf *full_path, Buf *out_contents, bool skip_shebang); +Error ATTRIBUTE_MUST_USE os_fetch_file(FILE *file, Buf *out_contents); +Error ATTRIBUTE_MUST_USE os_fetch_file_path(Buf *full_path, Buf *out_contents); Error ATTRIBUTE_MUST_USE os_get_cwd(Buf *out_cwd); diff --git a/std/zig/ast.zig b/std/zig/ast.zig index 9aba59f77c..7024f988a2 100644 --- a/std/zig/ast.zig +++ b/std/zig/ast.zig @@ -479,7 +479,6 @@ pub const Node = struct { doc_comments: ?*DocComment, decls: DeclList, eof_token: TokenIndex, - shebang: ?TokenIndex, pub const DeclList = SegmentedList(*Node, 4); @@ -491,7 +490,6 @@ pub const Node = struct { } pub fn firstToken(self: *const Root) TokenIndex { - if (self.shebang) |shebang| return shebang; return if (self.decls.len == 0) self.eof_token else (self.decls.at(0).*).firstToken(); } @@ -2235,7 +2233,6 @@ test "iterate" { .doc_comments = null, .decls = Node.Root.DeclList.init(std.debug.global_allocator), .eof_token = 0, - .shebang = null, }; var base = &root.base; testing.expect(base.iterate(0) == null); diff --git a/std/zig/parse.zig b/std/zig/parse.zig index 42fa6d8590..a461e15721 100644 --- a/std/zig/parse.zig +++ b/std/zig/parse.zig @@ -22,7 +22,6 @@ pub fn parse(allocator: *mem.Allocator, source: []const u8) !ast.Tree { .base = ast.Node{ .id = ast.Node.Id.Root }, .decls = ast.Node.Root.DeclList.init(arena), .doc_comments = null, - .shebang = null, // initialized when we get the eof token .eof_token = undefined, }; @@ -43,15 +42,6 @@ pub fn parse(allocator: *mem.Allocator, source: []const u8) !ast.Tree { } var tok_it = tree.tokens.iterator(0); - // skip over shebang line - shebang: { - const shebang_tok_index = tok_it.index; - const shebang_tok_ptr = tok_it.peek() orelse break :shebang; - if (shebang_tok_ptr.id != Token.Id.ShebangLine) break :shebang; - root_node.shebang = shebang_tok_index; - _ = tok_it.next(); - } - // skip over line comments at the top of the file while (true) { const next_tok = tok_it.peek() orelse break; diff --git a/std/zig/parser_test.zig b/std/zig/parser_test.zig index 82607dbbe9..111f0cd6ac 100644 --- a/std/zig/parser_test.zig +++ b/std/zig/parser_test.zig @@ -50,14 +50,6 @@ test "zig fmt: linksection" { ); } -test "zig fmt: shebang line" { - try testCanonical( - \\#!/usr/bin/env zig - \\pub fn main() void {} - \\ - ); -} - test "zig fmt: correctly move doc comments on struct fields" { try testTransform( \\pub const section_64 = extern struct { diff --git a/std/zig/render.zig b/std/zig/render.zig index f1fe23c2a8..74c1e2acfc 100644 --- a/std/zig/render.zig +++ b/std/zig/render.zig @@ -73,11 +73,6 @@ fn renderRoot( ) (@typeOf(stream).Child.Error || Error)!void { var tok_it = tree.tokens.iterator(0); - // render the shebang line - if (tree.root_node.shebang) |shebang| { - try stream.write(tree.tokenSlice(shebang)); - } - // render all the line comments at the beginning of the file while (tok_it.next()) |token| { if (token.id != Token.Id.LineComment) break; -- cgit v1.2.3