authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2023-06-02 00:33:28-07:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2023-06-10 20:47:59-07:00
logf1c900c72e0d941fb2ab87197485c710fc95450b
tree60c39eb03de5d0a54a5a33c871169678a2ba940c
parent69b7b910929e84248671377e1743477757e66837

compiler: avoid use of undefined memory

InternPool is nice in some ways but it also comes with its own set of footguns. This commit fixes 5 instances. I see quite a few Valgrind warnings remaining when running the behavior tests. Perhaps the solution is to have stringToSlice return a struct with start and length as indexes, which has a format function?

2 files changed, 31 insertions(+), 7 deletions(-)

src/Module.zig+27-6
...@@ -576,7 +576,6 @@ pub const Decl = struct {...@@ -576,7 +576,6 @@ pub const Decl = struct {
576 }576 }
577 mod.destroyFunc(func);577 mod.destroyFunc(func);
578 }578 }
579 _ = mod.memoized_decls.remove(decl.val.ip_index);
580 }579 }
581580
582 /// This name is relative to the containing namespace of the decl.581 /// This name is relative to the containing namespace of the decl.
...@@ -690,10 +689,30 @@ pub const Decl = struct {...@@ -690,10 +689,30 @@ pub const Decl = struct {
690 }689 }
691690
692 pub fn getFullyQualifiedName(decl: Decl, mod: *Module) !InternPool.NullTerminatedString {691 pub fn getFullyQualifiedName(decl: Decl, mod: *Module) !InternPool.NullTerminatedString {
693 const gpa = mod.gpa;692 if (decl.name_fully_qualified) return decl.name;
693
694 const ip = &mod.intern_pool;694 const ip = &mod.intern_pool;
695 const count = count: {
696 var count: usize = ip.stringToSlice(decl.name).len + 1;
697 var ns: Namespace.Index = decl.src_namespace;
698 while (true) {
699 const namespace = mod.namespacePtr(ns);
700 const ns_decl_index = namespace.getDeclIndex(mod);
701 const ns_decl = mod.declPtr(ns_decl_index);
702 count += ip.stringToSlice(ns_decl.name).len + 1;
703 ns = namespace.parent.unwrap() orelse {
704 count += namespace.file_scope.sub_file_path.len;
705 break :count count;
706 };
707 }
708 };
709
710 const gpa = mod.gpa;
695 const start = ip.string_bytes.items.len;711 const start = ip.string_bytes.items.len;
696 try decl.renderFullyQualifiedName(mod, ip.string_bytes.writer(gpa));712 // Protects reads of interned strings from being reallocated during the call to
713 // renderFullyQualifiedName.
714 try ip.string_bytes.ensureUnusedCapacity(gpa, count);
715 decl.renderFullyQualifiedName(mod, ip.string_bytes.writer(gpa)) catch unreachable;
697716
698 // Sanitize the name for nvptx which is more restrictive.717 // Sanitize the name for nvptx which is more restrictive.
699 // TODO This should be handled by the backend, not the frontend. Have a718 // TODO This should be handled by the backend, not the frontend. Have a
...@@ -4018,7 +4037,7 @@ pub fn ensureDeclAnalyzed(mod: *Module, decl_index: Decl.Index) SemaError!void {...@@ -4018,7 +4037,7 @@ pub fn ensureDeclAnalyzed(mod: *Module, decl_index: Decl.Index) SemaError!void {
4018 .unreferenced => false,4037 .unreferenced => false,
4019 };4038 };
40204039
4021 var decl_prog_node = mod.sema_prog_node.start(mod.intern_pool.stringToSlice(decl.name), 0);4040 var decl_prog_node = mod.sema_prog_node.start("", 0);
4022 decl_prog_node.activate();4041 decl_prog_node.activate();
4023 defer decl_prog_node.end();4042 defer decl_prog_node.end();
40244043
...@@ -5774,9 +5793,11 @@ pub fn createAnonymousDeclFromDecl(...@@ -5774,9 +5793,11 @@ pub fn createAnonymousDeclFromDecl(
5774 const new_decl_index = try mod.allocateNewDecl(namespace, src_decl.src_node, src_scope);5793 const new_decl_index = try mod.allocateNewDecl(namespace, src_decl.src_node, src_scope);
5775 errdefer mod.destroyDecl(new_decl_index);5794 errdefer mod.destroyDecl(new_decl_index);
5776 const ip = &mod.intern_pool;5795 const ip = &mod.intern_pool;
5777 const name = try ip.getOrPutStringFmt(mod.gpa, "{s}__anon_{d}", .{5796 // This protects the getOrPutStringFmt from reallocating src decl name while reading it.
5797 try ip.string_bytes.ensureUnusedCapacity(mod.gpa, ip.stringToSlice(src_decl.name).len + 20);
5798 const name = ip.getOrPutStringFmt(mod.gpa, "{s}__anon_{d}", .{
5778 ip.stringToSlice(src_decl.name), @enumToInt(new_decl_index),5799 ip.stringToSlice(src_decl.name), @enumToInt(new_decl_index),
5779 });5800 }) catch unreachable;
5780 try mod.initNewAnonDecl(new_decl_index, src_decl.src_line, namespace, tv, name);5801 try mod.initNewAnonDecl(new_decl_index, src_decl.src_line, namespace, tv, name);
5781 return new_decl_index;5802 return new_decl_index;
5782}5803}
src/Sema.zig+4-1
...@@ -17065,6 +17065,7 @@ fn typeInfoNamespaceDecls(...@@ -17065,6 +17065,7 @@ fn typeInfoNamespaceDecls(
17065 seen_namespaces: *std.AutoHashMap(*Namespace, void),17065 seen_namespaces: *std.AutoHashMap(*Namespace, void),
17066) !void {17066) !void {
17067 const mod = sema.mod;17067 const mod = sema.mod;
17068 const ip = &mod.intern_pool;
17068 const gop = try seen_namespaces.getOrPut(namespace);17069 const gop = try seen_namespaces.getOrPut(namespace);
17069 if (gop.found_existing) return;17070 if (gop.found_existing) return;
17070 const decls = namespace.decls.keys();17071 const decls = namespace.decls.keys();
...@@ -17081,7 +17082,9 @@ fn typeInfoNamespaceDecls(...@@ -17081,7 +17082,9 @@ fn typeInfoNamespaceDecls(
17081 const name_val = v: {17082 const name_val = v: {
17082 var anon_decl = try block.startAnonDecl();17083 var anon_decl = try block.startAnonDecl();
17083 defer anon_decl.deinit();17084 defer anon_decl.deinit();
17084 const name = mod.intern_pool.stringToSlice(decl.name);17085 // Protects the decl name slice from being invalidated at the call to intern().
17086 try ip.string_bytes.ensureUnusedCapacity(sema.gpa, ip.stringToSlice(decl.name).len + 1);
17087 const name = ip.stringToSlice(decl.name);
17085 const new_decl_ty = try mod.arrayType(.{17088 const new_decl_ty = try mod.arrayType(.{
17086 .len = name.len,17089 .len = name.len,
17087 .child = .u8_type,17090 .child = .u8_type,