From 296ed6cea4cac698fd9a7185f6f883172b4f6d05 Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Thu, 16 Nov 2023 16:57:17 +0100 Subject: [PATCH 1/3] Bugfix: loadMemoryFromFile support absolute paths 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. --- .../scala/treadle2/executable/Memory.scala | 2 +- .../examples/LoadMemoryFromFileSpec.scala | 22 ++++++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/main/scala/treadle2/executable/Memory.scala b/src/main/scala/treadle2/executable/Memory.scala index 12a4a3435..bfbd0a892 100644 --- a/src/main/scala/treadle2/executable/Memory.scala +++ b/src/main/scala/treadle2/executable/Memory.scala @@ -653,7 +653,7 @@ class MemoryInitializer(engine: ExecutionEngine) extends LazyLogging { private object MemoryFileParser { def parse(filename: String, base: Int): Seq[BigInt] = { require(base == 16 || base == 2) - val fullName = os.pwd / os.RelPath(filename) + val fullName = os.Path(filename, base=os.pwd) os.read.lines(fullName).flatMap(l => parseLine(l.trim, base)) } private def parseLine(line: String, base: Int): Seq[BigInt] = { diff --git a/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala b/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala index 44be1f930..e73d60611 100644 --- a/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala +++ b/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala @@ -10,7 +10,7 @@ import chiseltest.simulator.RequiresVerilator import firrtl2.options.TargetDirAnnotation import org.scalatest.freespec.AnyFreeSpec -class UsesMem(memoryDepth: Int, memoryType: Bits) extends Module { +class UsesMem(memoryDepth: Int, memoryType: Bits, fileName: String) extends Module { val io = IO(new Bundle { val address = Input(UInt(memoryType.getWidth.W)) val value = Output(memoryType) @@ -19,7 +19,7 @@ class UsesMem(memoryDepth: Int, memoryType: Bits) extends Module { val memory = Mem(memoryDepth, memoryType) - loadMemoryFromFileInline(memory, "src/test/resources/iotesters/mem1.txt") + loadMemoryFromFileInline(memory, fileName) io.value := memory(io.address) @@ -28,6 +28,9 @@ class UsesMem(memoryDepth: Int, memoryType: Bits) extends Module { low.io.address := io.address io.value2 := low.io.value } +object UsesMem { + val MEM1 = "src/test/resources/iotesters/mem1.txt" +} class UsesMemLow(memoryDepth: Int, memoryType: Data) extends Module { val io = IO(new Bundle { @@ -56,8 +59,21 @@ class LoadMemoryFromFileSpec extends AnyFreeSpec with ChiselScalatestTester { "Users can specify a source file to load memory from" taggedAs RequiresVerilator in { val targetDir = TargetDirAnnotation("test_run_dir/load_mem_test") - test(new UsesMem(memoryDepth = 8, memoryType = UInt(16.W))) + test(new UsesMem(memoryDepth = 8, memoryType = UInt(16.W), "src/test/resources/iotesters/mem1.txt")) .withAnnotations(Seq(VerilatorBackendAnnotation, targetDir)) .runPeekPoke(new LoadMemoryFromFileTester(_)) } + + "Treadle supports loadFromFileInline using absolute paths" in { + // An absolute path + val path: os.Path = os.pwd / os.RelPath(UsesMem.MEM1) + test(new UsesMem(memoryDepth = 8, memoryType = UInt(16.W), path.toString())) + .runPeekPoke(new LoadMemoryFromFileTester(_)) + } + + "Treadle also supports loadFromFileInline using relative paths" in { + // An absolute path + test(new UsesMem(memoryDepth = 8, memoryType = UInt(16.W), UsesMem.MEM1)) + .runPeekPoke(new LoadMemoryFromFileTester(_)) + } } From 0a8d602ff5ffb559a5dc4619f0c122072adae954 Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Thu, 16 Nov 2023 17:14:50 +0100 Subject: [PATCH 2/3] Review: style: MEM1 -> Mem1 --- .../iotesters/examples/LoadMemoryFromFileSpec.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala b/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala index e73d60611..ab200bf17 100644 --- a/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala +++ b/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala @@ -29,7 +29,7 @@ class UsesMem(memoryDepth: Int, memoryType: Bits, fileName: String) extends Modu io.value2 := low.io.value } object UsesMem { - val MEM1 = "src/test/resources/iotesters/mem1.txt" + val Mem1 = "src/test/resources/iotesters/mem1.txt" } class UsesMemLow(memoryDepth: Int, memoryType: Data) extends Module { @@ -66,14 +66,14 @@ class LoadMemoryFromFileSpec extends AnyFreeSpec with ChiselScalatestTester { "Treadle supports loadFromFileInline using absolute paths" in { // An absolute path - val path: os.Path = os.pwd / os.RelPath(UsesMem.MEM1) + val path: os.Path = os.pwd / os.RelPath(UsesMem.Mem1) test(new UsesMem(memoryDepth = 8, memoryType = UInt(16.W), path.toString())) .runPeekPoke(new LoadMemoryFromFileTester(_)) } "Treadle also supports loadFromFileInline using relative paths" in { // An absolute path - test(new UsesMem(memoryDepth = 8, memoryType = UInt(16.W), UsesMem.MEM1)) + test(new UsesMem(memoryDepth = 8, memoryType = UInt(16.W), UsesMem.Mem1)) .runPeekPoke(new LoadMemoryFromFileTester(_)) } } From ab13cc60405741d862fe96b210adb4c4fa4c5ef3 Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Thu, 16 Nov 2023 17:21:10 +0100 Subject: [PATCH 3/3] scalafmt --- src/main/scala/treadle2/executable/Memory.scala | 2 +- .../chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/scala/treadle2/executable/Memory.scala b/src/main/scala/treadle2/executable/Memory.scala index bfbd0a892..3576d1011 100644 --- a/src/main/scala/treadle2/executable/Memory.scala +++ b/src/main/scala/treadle2/executable/Memory.scala @@ -653,7 +653,7 @@ class MemoryInitializer(engine: ExecutionEngine) extends LazyLogging { private object MemoryFileParser { def parse(filename: String, base: Int): Seq[BigInt] = { require(base == 16 || base == 2) - val fullName = os.Path(filename, base=os.pwd) + val fullName = os.Path(filename, base = os.pwd) os.read.lines(fullName).flatMap(l => parseLine(l.trim, base)) } private def parseLine(line: String, base: Int): Seq[BigInt] = { diff --git a/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala b/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala index ab200bf17..7da1aa627 100644 --- a/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala +++ b/src/test/scala/chiseltest/iotesters/examples/LoadMemoryFromFileSpec.scala @@ -3,9 +3,9 @@ package chiseltest.iotesters.examples import chisel3._ -import chiseltest.iotesters._ import chisel3.util.experimental.loadMemoryFromFileInline import chiseltest._ +import chiseltest.iotesters._ import chiseltest.simulator.RequiresVerilator import firrtl2.options.TargetDirAnnotation import org.scalatest.freespec.AnyFreeSpec