From 314f044c2bbb8dd8799df6c76f6d81109bf35043 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 29 Jan 2024 06:19:23 +0100 Subject: [PATCH] keep copies of parser inputs that are in-memory only the parser modifies its inputs, which means that sharing them between the error context reporting system and the parser itself can confuse the reporting system. usually this led to early truncation of error context reports which, while not dangerous, can be quite confusing. (cherry picked from commit d384ecd553aa997270b79ee98d02f7cf7e1849e6) Change-Id: I677646b5675b12b2faa787943646aa36dc6e6ee3 --- src/libexpr/eval.cc | 16 +++++++++++----- .../lang/parse-fail-dup-attrs-1.err.exp | 1 + .../lang/parse-fail-dup-attrs-2.err.exp | 1 + .../lang/parse-fail-dup-attrs-3.err.exp | 1 + .../lang/parse-fail-dup-attrs-4.err.exp | 1 + .../lang/parse-fail-dup-attrs-7.err.exp | 1 + .../lang/parse-fail-undef-var-2.err.exp | 3 ++- tests/functional/lang/parse-fail-utf8.err.exp | 3 ++- 8 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 1739a04fa..32766950d 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2745,9 +2745,12 @@ Expr * EvalState::parseExprFromFile(const SourcePath & path, std::shared_ptr & staticEnv) { - auto s = make_ref(std::move(s_)); - s->append("\0\0", 2); - return parse(s->data(), s->size(), Pos::String{.source = s}, basePath, staticEnv); + // NOTE this method (and parseStdin) must take care to *fully copy* their input + // into their respective Pos::Origin until the parser stops overwriting its input + // data. + auto s = make_ref(s_); + s_.append("\0\0", 2); + return parse(s_.data(), s_.size(), Pos::String{.source = s}, basePath, staticEnv); } @@ -2759,12 +2762,15 @@ Expr * EvalState::parseExprFromString(std::string s, const SourcePath & basePath Expr * EvalState::parseStdin() { + // NOTE this method (and parseExprFromString) must take care to *fully copy* their + // input into their respective Pos::Origin until the parser stops overwriting its + // input data. //Activity act(*logger, lvlTalkative, "parsing standard input"); auto buffer = drainFD(0); // drainFD should have left some extra space for terminators + auto s = make_ref(buffer); buffer.append("\0\0", 2); - auto s = make_ref(std::move(buffer)); - return parse(s->data(), s->size(), Pos::Stdin{.source = s}, rootPath(CanonPath::fromCwd()), staticBaseEnv); + return parse(buffer.data(), buffer.size(), Pos::Stdin{.source = s}, rootPath(CanonPath::fromCwd()), staticBaseEnv); } diff --git a/tests/functional/lang/parse-fail-dup-attrs-1.err.exp b/tests/functional/lang/parse-fail-dup-attrs-1.err.exp index 6c3a3510c..ffb5198c1 100644 --- a/tests/functional/lang/parse-fail-dup-attrs-1.err.exp +++ b/tests/functional/lang/parse-fail-dup-attrs-1.err.exp @@ -3,3 +3,4 @@ error: attribute 'x' already defined at «stdin»:1:3 2| y = 456; 3| x = 789; | ^ + 4| } diff --git a/tests/functional/lang/parse-fail-dup-attrs-2.err.exp b/tests/functional/lang/parse-fail-dup-attrs-2.err.exp index fecdece20..4607a5d59 100644 --- a/tests/functional/lang/parse-fail-dup-attrs-2.err.exp +++ b/tests/functional/lang/parse-fail-dup-attrs-2.err.exp @@ -3,3 +3,4 @@ error: attribute 'x' already defined at «stdin»:9:5 9| x = 789; 10| inherit (as) x; | ^ + 11| }; diff --git a/tests/functional/lang/parse-fail-dup-attrs-3.err.exp b/tests/functional/lang/parse-fail-dup-attrs-3.err.exp index fecdece20..4607a5d59 100644 --- a/tests/functional/lang/parse-fail-dup-attrs-3.err.exp +++ b/tests/functional/lang/parse-fail-dup-attrs-3.err.exp @@ -3,3 +3,4 @@ error: attribute 'x' already defined at «stdin»:9:5 9| x = 789; 10| inherit (as) x; | ^ + 11| }; diff --git a/tests/functional/lang/parse-fail-dup-attrs-4.err.exp b/tests/functional/lang/parse-fail-dup-attrs-4.err.exp index f85ffea51..c98a8f8d0 100644 --- a/tests/functional/lang/parse-fail-dup-attrs-4.err.exp +++ b/tests/functional/lang/parse-fail-dup-attrs-4.err.exp @@ -3,3 +3,4 @@ error: attribute 'services.ssh.port' already defined at «stdin»:2:3 2| services.ssh.port = 22; 3| services.ssh.port = 23; | ^ + 4| } diff --git a/tests/functional/lang/parse-fail-dup-attrs-7.err.exp b/tests/functional/lang/parse-fail-dup-attrs-7.err.exp index 98cea9dae..2daddf380 100644 --- a/tests/functional/lang/parse-fail-dup-attrs-7.err.exp +++ b/tests/functional/lang/parse-fail-dup-attrs-7.err.exp @@ -3,3 +3,4 @@ error: attribute 'x' already defined at «stdin»:6:12 6| inherit x; 7| inherit x; | ^ + 8| }; diff --git a/tests/functional/lang/parse-fail-undef-var-2.err.exp b/tests/functional/lang/parse-fail-undef-var-2.err.exp index a58d8dca4..393c454dd 100644 --- a/tests/functional/lang/parse-fail-undef-var-2.err.exp +++ b/tests/functional/lang/parse-fail-undef-var-2.err.exp @@ -1,5 +1,6 @@ error: syntax error, unexpected ':', expecting '}' at «stdin»:3:13: 2| - 3| f = {x, y : + 3| f = {x, y : ["baz" "bar" z "bat"]}: x + y; | ^ + 4| diff --git a/tests/functional/lang/parse-fail-utf8.err.exp b/tests/functional/lang/parse-fail-utf8.err.exp index e83abdb9e..1c83f6eb3 100644 --- a/tests/functional/lang/parse-fail-utf8.err.exp +++ b/tests/functional/lang/parse-fail-utf8.err.exp @@ -1,4 +1,5 @@ error: syntax error, unexpected invalid token, expecting end of file at «stdin»:1:5: - 1| 123 à + 1| 123 é 4 | ^ + 2|