lowRISC / lowRISC/dvsim

Eval wildcard system

Open
#17 0 comments 0 reactions 5 assignees View on GitHub

@timothytrippel is already working on this.

Since Sep 22, 2025.

Dominant language
Python
Stars
13
Forks
21
Avg merge
1d 11h
Merged PRs (30d)
8

Description

The `eval_cmd` feature is questionable to begin with as it encourages mixing config and behavioural logic. Having good separation between logic and config is generally considered to result in a more robust and maintainable long term solution. At some point it might be good to consider removing that feature and replacing it with a plug-in system that gives the same functionality without the issues associated with blindly executing arbitrary strings in configuration when loading the config file.

There are not many uses of this in OT, so it's feasible to remove, with some effort.

## Implementation issues

In the short term, the main issue with the implementation of this feature is that `eval_cmd` only has a start delimiter. Which means that if the wildcards are substituted in a different order then arbitrary text will be executed. This is a slightly contrived example, as DVSim has direct support for using environment variables as wildcards. However conceptually this demonstration shows real a real bug that I've seen.

```
{
"a": "{eval_cmd} env | grep PWD",
"b": "{a}/sub/dir"
}
```

If `a` is resolved before `b`, then `b` might contain what you expect:

```
~/opentitan/sub/dir
```

However if for some reason `b` is resolved before `a`, then we get:

```
{
"a": "{eval_cmd} env | grep PWD",
"b": "{eval_cmd} env | grep PWD/sub/dir"
}
```

Which in a second pass becomes:

```
{
"a": "~/opentitan",
"b": ""
}
```

There are two issues here:
1. The operations are not commutative - the order matters
2. The eval command should only be run once and the result then propagated. Right now if the wildcard is substituted before it's evaluated, then the shell commands are run multiple times for no reason.

## Proposal

Short term, fix the wildcard substitution so that it is commutative. Potential solutions are:
- Do not use the value of a field anywhere else until it's been evaluated? This may cause issues in terms of dependency loops, and complicate the algorithm?
- Add an end delimiter before substituting

The delimiter idea could be implemented by moving the arguments within the enclosing brackets. For example use the format `{eval_cmd arguments ...}` as a replacement for `{eval_cmd} arguments ...`. As part of the migration step the old form could be automatically converted. Which means that if the string contains `{eval_cmd}` then attempt to evaluate it, if that's not possible right now as it contains unresolved wildcards, then convert it to the new format.

Long term, ideally we find a way of implementing the required functionality without the `{eval_cmd}` feature.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.