Fix a bug of $ROOT path when using path dependency - #348
Open
DiyouS wants to merge 1 commit into
Open
Conversation
…d triggering some corner case bugs.
Author
|
The CI failed because we added a |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix
$ROOTvariable collision in generated build scripts(vsim/vcs/riviera/synopsys/genus/formality/vivado)
Problem
bender script <fmt>emits generated build scripts (Tcl forvsim/riviera/synopsys/genus/formality/vivado, shell for vcs) that shorten
absolute source paths by replacing the invoking package's root directory
with a script variable, e.g.
$ROOT. When a dependency is apath-typedependency that resolves outside the invoking package's own directory tree
(e.g.
{ path: "../some_sibling_pkg" }), and that sibling directory's namehappens to share the root directory's name as a prefix (e.g. root is
.../foo, sibling is.../foo_bar), the generated variable referencesilently corrupts:
set ROOT "/path/to/foo"
...
vlog ... "$ROOT_bar/src/some_file.sv" \
Both Tcl and POSIX shell parse
$ROOT_baras a reference to a variablenamed
ROOT_bar, not$ROOTfollowed by literal_bar. SinceROOT_baris never defined, this raises an "undefined variable" error. Because every
vlog/vcominvocation in these templates is wrapped inif {[catch { ... }]} {return 1}(or the shell equivalent), the error is swallowed and theentire script silently aborts at that point — no error is printed, and
everything after it (including the dependency's own sources) is never
compiled. The only visible symptom is that compilation output stops partway
through with no error message.
Root cause
The templates build these paths with Tera's
replacefilter:{{ file.file | replace(from=root, to='$ROOT') }}
replaceperforms a plain substring replacement (like Rust'sstr::replace), with no awareness of path boundaries. Ifroot(e.g./scratch/.../foo) occurs as a literal prefix substring of a longer,unrelated path (e.g.
/scratch/.../foo_bar/src/x.sv), it gets replacedanyway, and the leftover suffix (
_bar/src/x.sv) is concatenated directlyonto
$ROOTwith no boundary — producing$ROOT_bar/src/x.sv. Both Tcl andshell greedily consume
[A-Za-z0-9_]characters after an unbraced$whenresolving a variable reference, so
$ROOT_baris read as one identifier,not
$ROOT+ literal text.Fix
Use the braced form
${ROOT}instead of the bare$ROOTwherever it'semitted into generated scripts.
${ROOT}is valid, standard syntax in bothTcl and POSIX shell for explicitly delimiting a variable name, and correctly
disambiguates
${ROOT}_baras "value of ROOT" + literal_bar, matchingthe intended behavior in all cases (including the common case where no
collision occurs).
Applied to every
to='$ROOT'occurrence, and the two literal"$ROOT{{ ... }}"occurrences ingenus_tcl.tera/formality_tcl.tera, across:vsim_tcl.teravcs_sh.terariviera_tcl.terasynopsys_tcl.teragenus_tcl.teraformality_tcl.teravivado_tcl.tera(
verilator_sh.tera,flist.tera,flist-plus.tera, andprecision_tcl.teradon't use this substitution pattern and areunaffected.)
Repro
foo(root) has apathdependency{ path: "../foo_bar" }(orany sibling directory whose name is prefixed by the root directory's own
name).
bender script vsim(or vcs/riviera/synopsys/genus/formality/vivado)emits a source block for files under
foo_barreferencing$ROOT_bar/...,which is undefined.
reported error, then silently stops.