A newline in an opts value silently corrupts the command

[andreaskurth]

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:

additional_fusesoc_argument:
  '''
  --mapping=lowrisc:systems:top_darjeeling:0.1
  --mapping=lowrisc:dv:rom_ctrl_bkdr_util_hier:0.1
  '''

dvsim <cfg> --tool veriblelint --select-cfgs rom_ctrl -n loads this without complaint, and make -n expands do_build into two commands:

cd <build_dir> && 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 <build_log>

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.quotes 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.

[rswarbrick]

My preference for a "short term fix" is the third option (spit out an error if there is an \n in the command). It will be trivial to implement and I'm not sure we need any extra functionality. I like the "obvious for the user" option.