fix: segfault in positional arg completion

Adding the inputPath as a positional feature uncovered this bug.
As positional argument forms were discarded from the `expectedArgs`
list, their closures were not. When the `.completer` closure was then
called, part of the surrounding object did not exist anymore.

This didn't cause an issue before, but with the new call to
`getEvalState()` in the "inputs" completer in nix/flake.cc, a segfault
was triggered reproducibly on invalid memory access to the `this`
pointer, which was always 0.

The solution of splicing the argument forms into a new list to extend
their lifetime is a bit of a hack, but I was unable to get the "nicer"
iterator-based solution to work.
This commit is contained in:
Felix Uhl 2023-10-24 22:22:05 +02:00 committed by Théophane Hufschmitt
parent c7dcdb8325
commit f282ef5a56
3 changed files with 29 additions and 2 deletions

View file

@ -255,7 +255,18 @@ bool Args::processArgs(const Strings & args, bool finish)
}
if (!anyCompleted)
exp.handler.fun(ss);
expectedArgs.pop_front();
/* Move the list element to the processedArgs. This is almost the same as
`processedArgs.push_back(expectedArgs.front()); expectedArgs.pop_front()`,
except that it will only adjust the next and prev pointers of the list
elements, meaning the actual contents don't move in memory. This is
critical to prevent invalidating internal pointers! */
processedArgs.splice(
processedArgs.end(),
expectedArgs,
expectedArgs.begin(),
++expectedArgs.begin());
res = true;
}

View file

@ -200,13 +200,25 @@ protected:
/**
* Queue of expected positional argument forms.
*
* Positional arugment descriptions are inserted on the back.
* Positional argument descriptions are inserted on the back.
*
* As positional arguments are passed, these are popped from the
* front, until there are hopefully none left as all args that were
* expected in fact were passed.
*/
std::list<ExpectedArg> expectedArgs;
/**
* List of processed positional argument forms.
*
* All items removed from `expectedArgs` are added here. After all
* arguments were processed, this list should be exactly the same as
* `expectedArgs` was before.
*
* This list is used to extend the lifetime of the argument forms.
* If this is not done, some closures that reference the command
* itself will segfault.
*/
std::list<ExpectedArg> processedArgs;
/**
* Process some positional arugments

View file

@ -44,6 +44,10 @@ EOF
# Input override completion
[[ "$(NIX_GET_COMPLETIONS=4 nix build ./foo --override-input '')" == $'normal\na\t' ]]
[[ "$(NIX_GET_COMPLETIONS=5 nix flake show ./foo --override-input '')" == $'normal\na\t' ]]
cd ./foo
[[ "$(NIX_GET_COMPLETIONS=3 nix flake update '')" == $'normal\na\t' ]]
cd ..
[[ "$(NIX_GET_COMPLETIONS=5 nix flake update --flake './foo' '')" == $'normal\na\t' ]]
## With multiple input flakes
[[ "$(NIX_GET_COMPLETIONS=5 nix build ./foo ./bar --override-input '')" == $'normal\na\t\nb\t' ]]
## With tilde expansion