Fix two nasty bugs (#40)

novafacing · web-flow · commit 47daa651dd50 · 2023-12-11T11:51:28.000-08:00
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -16,7 +16,6 @@ env:
   PUBLIC_SIMICS_PACKAGE_VERSION_1000: "6.0.169"
   MINGW_URL: "https://github.com/brechtsanders/winlibs_mingw/releases/download/13.2.0-16.0.6-11.0.0-ucrt-r1/winlibs-x86_64-posix-seh-gcc-13.2.0-llvm-16.0.6-mingw-w64ucrt-11.0.0-r1.7z"
   MINGW_VERSION: "13.2.0-16.0.6-11.0.0-ucrt-r1"
-  TSFFS_SIMICS_PKG_VERSION: "6.0.1"
 
 jobs:
   build_test_artifacts:
@@ -166,7 +165,7 @@ jobs:
           bin/project-setup --force
           ./build.rs
           ispm packages \
-            -i linux64/packages/simics-pkg-31337-${{ env.TSFFS_SIMICS_PKG_VERSION }}-linux64.ispm \
+            -i linux64/packages/simics-pkg-31337-*-linux64.ispm \
             --non-interactive --trust-insecure-packages
 
       - name: Build Project
@@ -271,13 +270,12 @@ jobs:
           ispm.exe projects $(pwd) --create --non-interactive --ignore-existing-files
           ./bin/project-setup.bat --mingw-dir C:\MinGW\ --ignore-existing-files --force
           cargo -Zscript build.rs
-          ispm.exe packages -i win64/packages/simics-pkg-31337-${{ env.TSFFS_SIMICS_PKG_VERSION }}-win64.ispm --non-interactive --trust-insecure-packages
 
       - name: Upload Distribution Package
         uses: actions/upload-artifact@v3
         with:
-          name: simics-pkg-31337-${{ env.TSFFS_SIMICS_PKG_VERSION }}-win64.ispm
-          path: win64/packages/simics-pkg-31337-${{ env.TSFFS_SIMICS_PKG_VERSION }}-win64.ispm
+          name: simics-pkg-31337-win64
+          path: win64/packages/simics-pkg-31337-*-win64.ispm
 
   build_dist:
     name: Build for Distribution
@@ -307,14 +305,14 @@ jobs:
         run: |
           docker build -t tsffs-dist -f .github/builder/Dockerfile .
           docker create --name tsffs-dist tsffs-dist bash
-          docker cp tsffs-dist:/tsffs/linux64/packages/simics-pkg-31337-${{ env.TSFFS_SIMICS_PKG_VERSION }}-linux64.ispm simics-pkg-31337-${{ env.TSFFS_SIMICS_PKG_VERSION }}-linux64.ispm
+          docker cp tsffs-dist:/tsffs/linux64/packages/ packages/
           docker rm -f tsffs-dist
 
       - name: Upload Distribution Package
         uses: actions/upload-artifact@v3
         with:
-          name: simics-pkg-31337-${{ env.TSFFS_SIMICS_PKG_VERSION }}-linux64.ispm
-          path: simics-pkg-31337-${{ env.TSFFS_SIMICS_PKG_VERSION }}-linux64.ispm
+          name: simics-pkg-31337-linux64
+          path: packages/simics-pkg-31337-*-linux64.ispm
 
   fuzz_tests:
     name: Run Fuzz Tests
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -57,7 +57,7 @@ Then you just add a line to every git commit message:
     Signed-off-by: Joe Smith <joe.smith@email.com>
 ```
 
-Use your real name (sorry, no pseudonyms or anonymous contributions.)
+Use a known identity (sorry, no anonymous contributions.)
 
 If you set your `user.name` and `user.email` git configs, you can sign your
 commit automatically with `git commit -s`.
diff --git a/Cargo.toml b/Cargo.toml
@@ -16,11 +16,11 @@ license = "Apache-2.0"
 publish = false
 readme = "README.md"
 repository = "https://github.com/intel/tsffs"
-version = "0.2.0"
+version = "0.2.1"
 
 [workspace.metadata]
-build-id = 2
-version = "6.0.1"
+build-id = 3
+version = "6.0.2"
 
 [workspace]
 resolver = "2"
diff --git a/Dockerfile b/Dockerfile
@@ -95,7 +95,7 @@ RUN ispm projects /workspace/tsffs/ --create --ignore-existing-files --non-inter
     bin/project-setup --force && \
     ./build.rs && \
     ispm packages \
-        -i /workspace/tsffs/linux64/packages/simics-pkg-31337-6.0.1-linux64.ispm \
+        -i /workspace/tsffs/linux64/packages/*-linux64.ispm \
         --non-interactive --trust-insecure-packages && \
     make clobber
 
diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md
@@ -49,4 +49,4 @@
   - [Build Internals](developer/build.md)
   - [Refreshing Build Environment](developer/refresh.md)
   - [Building Against A Specific SIMICS Version](developer/specific-simics-version.md)
-
+  - [Debugging TSFFS](developer/debugging.md)
diff --git a/docs/src/config/installing-in-projects.md b/docs/src/config/installing-in-projects.md
@@ -31,7 +31,7 @@ relative (from the project's SIMICS base package root) paths to packages that sh
 be included in the project.
 
 If all SIMICS packages are installed into an `install-dir` together, the TSFFS package
-can be added by adding a line like:
+can be added by adding a line like (if your installed package version is `6.0.1`):
 
 ```txt
 ../simics-tsffs-6.0.1/
@@ -43,4 +43,10 @@ If your SIMICS packages are not all installed together, the path can be absolute
 
 ```txt
 /absolute/path/to/installed/simics-tsffs-6.0.1/
+```
+
+You can obtain your latest installed version with:
+
+```sh
+ispm packages --list-installed --json | jq -r '[ .installedPackages[] | select(.pkgNumber == 31337) ] | ([ .[].version ] | max_by(split(".") | map(tonumber))) as $m | first(first(.[]|select(.version == $m)).paths[0])'
 ```
diff --git a/docs/src/developer/README.md b/docs/src/developer/README.md
@@ -2,4 +2,5 @@
 
 - [Build Internals](build.md)
 - [Refreshing Build Environment](refresh.md)
-- [Building Against A Specific SIMICS Version](specific-simics-version.md)
+- [Building Against A Specific SIMICS Version](specific-simics-version.md)
+- [Debugging TSFFS](debugging.md)
diff --git a/docs/src/developer/debugging.md b/docs/src/developer/debugging.md
@@ -0,0 +1,43 @@
+# Debugging
+
+Hopefully not very often, but once in a while you may need to debug the TSFFS module.
+
+The easiest way to do this is by loading and using it in a script that does what you
+want. For example, early in development there was a bug when calling the interface
+API.
+
+```txt
+@tsffs.iface.tsffs.set_corpus_directory("%simics%/corpus")
+```
+
+So this script was used to help debug:
+
+```txt
+load-module tsffs
+@tsffs = SIM_create_object(SIM_get_class("tsffs"), "tsffs", [])
+tsffs.log-level 4
+@import time
+@print("Sleeping")
+@time.sleep(30)
+@tsffs.iface.tsffs.set_corpus_directory("%simics%/corpus")
+```
+
+ALl this script does is sleep for 30 seconds, then call the API we care about. The 30
+second sleep gives you enough time to run this script, find the PID of the simics
+process, and attach it with GDB. Once in GDB, just break all threads on the place you
+want to debug.
+
+
+```sh
+$ ./simics -no-gui --no-win ./test.simics
+$ ps aux | grep simics | grep -v grep | awk '{print $2}'
+134284
+$ gdb -q attach 134284
+gdb> thread apply all break set_corpus_directory
+gdb> continue
+...
+```
+
+In general, most bugs will happen in FFI code, so breakpointing should be relatively
+straightforward. However, in complex cases demangling may be necessary. For this,
+a new version of GDB including rustfilt is suggested.
diff --git a/docs/src/setup/linux.md b/docs/src/setup/linux.md
@@ -159,14 +159,14 @@ steps for us:
 ./build.rs
 ```
 
-This will produce a file `linux64/packages/simics-pkg-31337-6.0.1-linux64.ispm`. We can
+This will produce a file `linux64/packages/simics-pkg-31337-VERSION-linux64.ispm`. We can
 then install this package into our local SIMICS installation. This in turn allows us to
 add the TSFFS package to our SIMICS projects for use. Note the
 `--trust-insecure-packages` flag is required because this package is not built and
 signed by the SIMICS team, but by ourselves.
 
 ```sh
-ispm packages -i linux64/packages/simics-pkg-31337-6.0.1-linux64.ispm \
+ispm packages -i linux64/packages/*-linux64.ispm \
     --non-interactive --trust-insecure-packages
 ```
 
diff --git a/docs/src/setup/windows.md b/docs/src/setup/windows.md
@@ -130,7 +130,7 @@ cargo -Zscript build.rs
 Once built, install TSFFS to your SIMICS installation with:
 
 ```powershell
-ispm.exe packages -i win64/packages/simics-pkg-31337-6.0.1-win64.ispm --non-interactive --trust-insecure-packages
+ispm.exe packages -i win64/packages/*-win64.ispm --non-interactive --trust-insecure-packages
 ```
 
 ## Test TSFFS
diff --git a/examples/manual-example/Dockerfile b/examples/manual-example/Dockerfile
@@ -94,7 +94,7 @@ RUN ispm projects /workspace/tsffs/ --create --ignore-existing-files --non-inter
     bin/project-setup --force && \
     ./build.rs && \
     ispm packages \
-        -i /workspace/tsffs/linux64/packages/simics-pkg-31337-6.0.1-linux64.ispm \
+        -i /workspace/tsffs/linux64/packages/*-linux64.ispm \
         --non-interactive --trust-insecure-packages
 
 WORKDIR /workspace/projects/example/
diff --git a/examples/tests/x86_64-uefi-edk2/test-call-all-apis.simics b/examples/tests/x86_64-uefi-edk2/test-call-all-apis.simics
@@ -0,0 +1,51 @@
+load-module tsffs
+
+@tsffs = SIM_create_object(SIM_get_class("tsffs"), "tsffs", [])
+tsffs.log-level 4
+@tsffs.iface.tsffs.set_start_on_harness(True)
+@tsffs.iface.tsffs.set_start_magic_number(1)
+@tsffs.iface.tsffs.set_stop_on_harness(True)
+@tsffs.iface.tsffs.set_stop_magic_number(2)
+@tsffs.iface.tsffs.set_timeout(3.0)
+@tsffs.iface.tsffs.add_exception_solution(14)
+@tsffs.iface.tsffs.add_exception_solution(6)
+@tsffs.iface.tsffs.remove_exception_solution(14)
+@tsffs.iface.tsffs.remove_exception_solution(6)
+@tsffs.iface.tsffs.set_all_exceptions_are_solutions(True)
+@tsffs.iface.tsffs.set_all_exceptions_are_solutions(False)
+@tsffs.iface.tsffs.add_breakpoint_solution(1)
+@tsffs.iface.tsffs.remove_breakpoint_solution(1)
+@tsffs.iface.tsffs.set_all_breakpoints_are_solutions(True)
+@tsffs.iface.tsffs.set_all_breakpoints_are_solutions(False)
+@tsffs.iface.tsffs.set_cmplog_enabled(False)
+@tsffs.iface.tsffs.set_cmplog_enabled(True)
+@tsffs.iface.tsffs.set_generate_random_corpus(True)
+@import time
+@print("Sleeping")
+@time.sleep(30)
+@tsffs.iface.tsffs.set_corpus_directory("%simics%/corpus")
+@tsffs.iface.tsffs.set_solutions_directory("%simics%/solutions")
+@tsffs.iface.tsffs.set_iterations(1000)
+@tsffs.iface.tsffs.set_use_snapshots(True)
+@tsffs.iface.tsffs.tokenize_executable("%simics%/test.efi")
+
+load-target "qsp-x86/uefi-shell" namespace = qsp machine:hardware:storage:disk0:image = "minimal_boot_disk.craff"
+
+script-branch {
+    bp.time.wait-for seconds = 15
+    qsp.serconsole.con.input "\n"
+    bp.time.wait-for seconds = .5
+    qsp.serconsole.con.input "FS0:\n"
+    bp.time.wait-for seconds = .5
+    local $manager = (start-agent-manager)
+    qsp.serconsole.con.input ("SimicsAgent.efi --download " + (lookup-file "%simics%/test.efi") + "\n")
+    bp.time.wait-for seconds = .5
+    qsp.serconsole.con.input "test.efi\n"
+}
+
+script-branch {
+    bp.time.wait-for seconds = 240
+    quit 1
+}
+
+run
diff --git a/modules/tsffs/src/simics/simics-macro/src/lib.rs b/modules/tsffs/src/simics/simics-macro/src/lib.rs
@@ -1092,6 +1092,7 @@ pub fn interface_impl(args: TokenStream, input: TokenStream) -> TokenStream {
         }
 
         #[derive(Debug)]
+        #[repr(C)]
         pub struct #internal_interface_name {
             #(#internal_interface_fields),*
         }
diff --git a/modules/tsffs/src/simics/simics/src/api/logging/mod.rs b/modules/tsffs/src/simics/simics/src/api/logging/mod.rs
@@ -37,6 +37,14 @@ pub enum LogLevel {
     Trace = 4,
 }
 
+/// Sanitize a string for logging (i.e. as if with printf)
+fn sanitize<S>(s: S) -> String
+where
+    S: AsRef<str>,
+{
+    s.as_ref().replace('%', "%%")
+}
+
 #[simics_exception]
 /// Log an info-level message through the SIMICS logging functions
 ///
@@ -93,7 +101,7 @@ pub fn log_error<S>(device: *mut ConfObject, msg: S) -> Result<()>
 where
     S: AsRef<str>,
 {
-    let msg_cstring = CString::new(msg.as_ref())?;
+    let msg_cstring = CString::new(sanitize(msg.as_ref()))?;
 
     #[cfg(not(simics_deprecated_api_sim_log))]
     unsafe {
@@ -127,7 +135,7 @@ pub fn log_critical<S>(device: *mut ConfObject, msg: S) -> Result<()>
 where
     S: AsRef<str>,
 {
-    let msg_cstring = CString::new(msg.as_ref())?;
+    let msg_cstring = CString::new(sanitize(msg.as_ref()))?;
 
     #[cfg(not(simics_deprecated_api_sim_log))]
     unsafe {
@@ -149,7 +157,7 @@ where
 ///
 /// All Contexts
 pub fn log_spec_violation(level: LogLevel, device: *mut ConfObject, msg: String) -> Result<()> {
-    let msg_cstring = CString::new(msg)?;
+    let msg_cstring = CString::new(sanitize(msg))?;
 
     #[cfg(not(simics_deprecated_api_sim_log))]
     unsafe {
@@ -171,7 +179,7 @@ pub fn log_spec_violation(level: LogLevel, device: *mut ConfObject, msg: String)
 ///
 /// All Contexts
 pub fn log_unimplemented(level: LogLevel, device: *mut ConfObject, msg: String) -> Result<()> {
-    let msg_cstring = CString::new(msg)?;
+    let msg_cstring = CString::new(sanitize(msg))?;
 
     #[cfg(not(simics_deprecated_api_sim_log))]
     unsafe {
diff --git a/modules/tsffs/src/simics/simics/src/api/simulator/configuration.rs b/modules/tsffs/src/simics/simics/src/api/simulator/configuration.rs
@@ -14,7 +14,6 @@ use crate::{
     },
     Error, Result,
 };
-use anyhow::anyhow;
 use raw_cstr::raw_cstr;
 use simics_macro::simics_exception;
 use std::{
@@ -35,14 +34,7 @@ pub fn read_configuration<P>(file: P) -> Result<()>
 where
     P: AsRef<Path>,
 {
-    unsafe {
-        SIM_read_configuration(raw_cstr(file.as_ref().to_str().ok_or_else(|| {
-            anyhow!(
-                "Could not convert file path {} to string",
-                file.as_ref().display()
-            )
-        })?)?)
-    };
+    unsafe { SIM_read_configuration(raw_cstr(file.as_ref().to_str().ok_or(Error::ToString)?)?) };
     Ok(())
 }
 
@@ -141,12 +133,7 @@ where
     unsafe {
         SIM_add_configuration(
             object_list,
-            raw_cstr(file.as_ref().to_str().ok_or_else(|| {
-                anyhow!(
-                    "Could not convert file path {} to string",
-                    file.as_ref().display()
-                )
-            })?)?,
+            raw_cstr(file.as_ref().to_str().ok_or(Error::ToString)?)?,
         )
     };
     Ok(())
@@ -195,12 +182,7 @@ where
 {
     unsafe {
         SIM_write_configuration_to_file(
-            raw_cstr(file.as_ref().to_str().ok_or_else(|| {
-                anyhow!(
-                    "Could not convert file path {} to string",
-                    file.as_ref().display()
-                )
-            })?)?,
+            raw_cstr(file.as_ref().to_str().ok_or(Error::ToString)?)?,
             flags,
         )
     };
diff --git a/modules/tsffs/src/simics/simics/src/api/simulator/memory.rs b/modules/tsffs/src/simics/simics/src/api/simulator/memory.rs
@@ -14,7 +14,6 @@ use crate::{
     },
     Error, Result,
 };
-use anyhow::anyhow;
 use raw_cstr::raw_cstr;
 use simics_macro::simics_exception;
 use std::path::Path;
@@ -94,12 +93,7 @@ where
     Ok(unsafe {
         SIM_load_binary(
             obj,
-            raw_cstr(file.as_ref().to_str().ok_or_else(|| {
-                anyhow!(
-                    "Could not convert file name {} to string",
-                    file.as_ref().display()
-                )
-            })?)?,
+            raw_cstr(file.as_ref().to_str().ok_or(Error::ToString)?)?,
             offset,
             use_pa,
             verbose,
@@ -121,12 +115,7 @@ where
     unsafe {
         SIM_load_file(
             obj,
-            raw_cstr(file.as_ref().to_str().ok_or_else(|| {
-                anyhow!(
-                    "Could not convert file name {} to string",
-                    file.as_ref().display()
-                )
-            })?)?,
+            raw_cstr(file.as_ref().to_str().ok_or(Error::ToString)?)?,
             paddr,
             verbose,
         )
diff --git a/modules/tsffs/src/simics/simics/src/api/simulator/paths.rs b/modules/tsffs/src/simics/simics/src/api/simulator/paths.rs
diff --git a/modules/tsffs/src/simics/simics/src/error/mod.rs b/modules/tsffs/src/simics/simics/src/error/mod.rs
diff --git a/modules/tsffs/src/tsffs/src/interface/mod.rs b/modules/tsffs/src/tsffs/src/interface/mod.rs
diff --git a/modules/tsffs/tests/Cargo.toml b/modules/tsffs/tests/Cargo.toml
diff --git a/modules/tsffs/tests/src/lib.rs b/modules/tsffs/tests/src/lib.rs
diff --git a/modules/tsffs/tests/tests/x86_64.rs b/modules/tsffs/tests/tests/x86_64.rs
