From 8d24a975f1d300f43485aa33e72187a016d23a49 Mon Sep 17 00:00:00 2001 From: Griffin Smith Date: Wed, 18 Nov 2020 09:02:25 -0500 Subject: fix(tvix): Use copy constructor to add strings to protos Passing a string directly to add_paths like this causes the proto class to take ownership over the string, meaning when it is destructed it will *explicitly* free the string. When the string's actual owner (the derivation struct) then goes out of scope it'll get freed again, causing a double-free. This fixes that to instead use the copy constructor to assign to a pointer to a new path, and covers the whole to_proto method with a rapidcheck test. Fixes: b/64 Change-Id: I84235bed9104ff430a0acf686d4a96f1e2e9a897 Reviewed-on: https://cl.tvl.fyi/c/depot/+/2106 Reviewed-by: tazjin Tested-by: BuildkiteCI --- third_party/nix/src/libstore/derivations.cc | 2 +- third_party/nix/src/tests/derivations_test.cc | 28 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) (limited to 'third_party/nix/src') diff --git a/third_party/nix/src/libstore/derivations.cc b/third_party/nix/src/libstore/derivations.cc index 0b7f5d092c43..ed184b6d9de4 100644 --- a/third_party/nix/src/libstore/derivations.cc +++ b/third_party/nix/src/libstore/derivations.cc @@ -72,7 +72,7 @@ nix::proto::Derivation BasicDerivation::to_proto() const { result.mutable_outputs()->insert({key, output.to_proto()}); } for (const auto& input_src : inputSrcs) { - result.mutable_input_sources()->add_paths(input_src); + *result.mutable_input_sources()->add_paths() = input_src; } result.set_platform(platform); result.mutable_builder()->set_path(builder); diff --git a/third_party/nix/src/tests/derivations_test.cc b/third_party/nix/src/tests/derivations_test.cc index 63e2c3070e3e..50a540e84aac 100644 --- a/third_party/nix/src/tests/derivations_test.cc +++ b/third_party/nix/src/tests/derivations_test.cc @@ -114,6 +114,34 @@ RC_GTEST_FIXTURE_PROP(DerivationsTest, UnparseParseRoundTrip, AssertDerivationsEqual(drv, parsed); } +// NOLINTNEXTLINE +RC_GTEST_FIXTURE_PROP(DerivationsTest, ToProtoPreservesInput, + (Derivation && drv)) { + auto proto = drv.to_proto(); + + RC_ASSERT(proto.outputs_size() == drv.outputs.size()); + RC_ASSERT(proto.input_sources().paths_size() == drv.inputSrcs.size()); + auto paths = proto.input_sources().paths(); + for (const auto& input_src : drv.inputSrcs) { + RC_ASSERT(std::find(paths.begin(), paths.end(), input_src) != paths.end()); + } + + RC_ASSERT(proto.platform() == drv.platform); + RC_ASSERT(proto.builder().path() == drv.builder); + + RC_ASSERT(proto.args_size() == drv.args.size()); + auto args = proto.args(); + for (const auto& arg : drv.args) { + RC_ASSERT(std::find(args.begin(), args.end(), arg) != args.end()); + } + + RC_ASSERT(proto.env_size() == drv.env.size()); + auto env = proto.env(); + for (const auto& [key, value] : drv.env) { + RC_ASSERT(env.at(key) == value); + } +} + class ParseDrvPathWithOutputsTest : public DerivationsTest {}; TEST(ParseDrvPathWithOutputsTest, ParseDrvPathWithOutputs) { -- cgit 1.4.1