A newline in an opts value silently corrupts the command
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 13
- Forks
- 21
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 8
Description
A config can put a newline into a `*_opts` value and nothing rejects it. It reaches the flow makefile intact and splits the recipe there, so the tool runs with part of its command line missing.
Wrapping an over-long value with an hjson multi-line string is enough to trigger it. In the `rom_ctrl` entry of OpenTitan's `hw/top_darjeeling/lint/top_darjeeling_dv_lint_cfgs.hjson`:
```hjson
additional_fusesoc_argument:
'''
--mapping=lowrisc:systems:top_darjeeling:0.1
--mapping=lowrisc:dv:rom_ctrl_bkdr_util_hier:0.1
'''
```
`dvsim --tool veriblelint --select-cfgs rom_ctrl -n` loads this without complaint, and `make -n` expands `do_build` into two commands:
```
cd && fusesoc ... --mapping=lowrisc:systems:top_darjeeling:0.1
mapping=lowrisc:dv:rom_ctrl_bkdr_util_hier:0.1 lowrisc:dv:rom_ctrl_sim 2>&1 | tee
```
FuseSoC loses the second mapping and the core name. The remainder runs in a fresh shell without the `cd`, its leading `--` eaten as make's recipe prefix characters, and the `tee` of the build log attaches to that half. Here FuseSoC errors out, so the run fails, if confusingly; when the first half happens to be a complete command, the `-` prefix on the recipe line swallows the second half and the arguments after the newline are dropped with the run reporting success.
Nothing about this is specific to lint or to that key. `Deploy._construct_cmd()` (`job/deploy.py:331-348`) `shlex.quote`s the value into a make variable, `runtime/local.py:194-195` runs `shlex.split(cmd)` with no shell so the newline survives, and every flow makefile expands its opts variables inline in a recipe line (`sim.mk`, `lint.mk`, `formal.mk`, `syn.mk`, `cdc.mk`, `rdc.mk` in OpenTitan). List-typed keys are no protection either, since the elements are joined with spaces.
The practical cost is that a long opts value cannot be wrapped in any flow, which leaves lines such as a 245-character `additional_fusesoc_argument` in OpenTitan's `hw/top_earlgrey/lint/top_earlgrey_dv_lint_cfgs.hjson` (lowRISC/opentitan#31185).
## Scope
- Replace newlines with spaces in the `is str` branch of `_construct_cmd()`, before `shlex.quote`. That one branch covers list-valued and scalar attributes, and every flow.
- Replace newlines only, rather than collapsing all whitespace as dvsim does for the dvplan commands (`job/deploy.py:1128`), so runs of spaces inside a quoted tool argument survive.
- Rejecting a newline at config load would do just as well, and would match the clean `RuntimeError` a config already gets for setting a list where a scalar is declared. Either beats corrupting the command.
## Done when
- The reproducer yields a single-line recipe, with a test to keep it that way.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with job/deploy.py:331-348 and trace how _construct_cmd() quotes scalar and list-valued opts. Reproduce the newline case with the OpenTitan rom_ctrl configuration and inspect the generated make recipe; add or update a focused test so the recipe remains single-line while runs of spaces are preserved.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, shell
- Domain
- build-system, tooling
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 84/100