Skip to content
This repository has been archived by the owner on Aug 19, 2024. It is now read-only.

Bugfix: loadMemoryFromFile support absolute paths #693

Merged
merged 3 commits into from
Nov 16, 2023

Conversation

nbfalcon
Copy link
Contributor

Previously, specifying an absolute path in loadMemoryFromFileInline would cause an error when running with treadle, because the path was absolutized using os.path.RelPath. The latter function throws an exception for absolute paths.

Now both absolute and relative paths work (matching the behaviour of Verilator).

Note that this has no security implications, since the previous "relative-path" based implementation also allowed "../../" paths; existing relative paths should not be affected.

Previously, specifying an absolute path in loadMemoryFromFileInline
would cause an error when running with treadle, because the path was
absolutized using os.path.RelPath. The latter function throws an
exception for absolute paths.

Now both absolute and relative paths work (matching the behaviour of
Verilator).

Note that this has no security implications, since the previous
"relative-path" based implementation also allowed "../../" paths;
existing relative paths should not be affected.
@CLAassistant
Copy link

CLAassistant commented Nov 16, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@ekiwi ekiwi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Just one small thing I would like you to change. Please also sign the CLA and let me know if you have any questions or issues with it.

@ekiwi
Copy link
Collaborator

ekiwi commented Nov 16, 2023

Please run sbt scalafmtAll and commit + push the changes. Thanks!

@ekiwi ekiwi merged commit 5b9c0b1 into ucb-bar:main Nov 16, 2023
26 checks passed
@ekiwi
Copy link
Collaborator

ekiwi commented Nov 16, 2023

Thank you!

Your bug-fix will soon be available as part of the 6.0-SNAPSHOT release of chiseltest which is compatible with Chisel6. Please let me know if you need to use this fix with an older version of Chisel and I can help guide you on how to do a backport.

@nbfalcon
Copy link
Contributor Author

Perfect, thank you! It's fine as is, I use the latest version of Chisel (6.0.0-M3 master) with locally built chiseltest/firrtl2, so a "regular release schedule" works for me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants