Repository: qir-alliance/pyqir (language: python)

Issue:
Parser should throw appropriate exception when loading invalid bitcode
The parser currently crashes python when loading non-bitcode files. The API should validate that a bitcode file is being loaded and throw an appropriate Python error.


Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index eedb7330..9b50b43d 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -477,6 +477,7 @@ dependencies = [
 name = "pyqir-parser"
 version = "0.5.0-alpha"
 dependencies = [
+ "inkwell",
  "llvm-ir",
  "pyo3",
 ]
diff --git a/Changelog.md b/Changelog.md
index df37371a..ce149f65 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -10,6 +10,7 @@
 - Set LLVM 13 as the default by @idavis in https://github.com/qir-alliance/pyqir/pull/131
 - Fix type hinting errors by @LaurentAjdnik in https://github.com/qir-alliance/pyqir/pull/133
 - Create CODEOWNERS by @samarsha in https://github.com/qir-alliance/pyqir/pull/134
+- Parser should throw appropriate exception when loading invalid bitcode by @idavis in https://github.com/qir-alliance/pyqir/pull/136
 
 ## [0.4.2a1] - 2022-06-03
 
diff --git a/eng/utils.ps1 b/eng/utils.ps1
index 55c42570..cbc699dd 100644
--- a/eng/utils.ps1
+++ b/eng/utils.ps1
@@ -268,10 +268,18 @@ function Build-PyQIR([string]$project) {
             $build_extra_args = "--skip-auditwheel"
         }
         Invoke-LoggedCommand {
-            maturin build --release $build_extra_args --cargo-extra-args="$($env:CARGO_EXTRA_ARGS)"
-            maturin develop --release --cargo-extra-args="$($env:CARGO_EXTRA_ARGS)"
-            & $python -m pip install -r requirements-dev.txt
-            & $python -m pytest
+            exec {
+                maturin build --release $build_extra_args --cargo-extra-args="$($env:CARGO_EXTRA_ARGS)"
+            }
+            exec {
+                maturin develop --release --cargo-extra-args="$($env:CARGO_EXTRA_ARGS)"
+            }
+            exec {
+                & $python -m pip install -r requirements-dev.txt
+            }
+            exec {
+                & $python -m pytest
+            }
         }
     }
 }
diff --git a/pyqir-parser/Cargo.toml b/pyqir-parser/Cargo.toml
index 994edd65..eadac4bb 100644
--- a/pyqir-parser/Cargo.toml
+++ b/pyqir-parser/Cargo.toml
@@ -13,15 +13,16 @@ repository = "https://github.com/qir-alliance/pyqir"
 
 [dependencies]
 llvm-ir = { version = "0.8.1" }
+inkwell = { git = "https://github.com/TheDan64/inkwell", branch = "master", default-features = false, features = ["target-x86"] }
 pyo3 = { version="0.15.2", optional = true }
 
 [features]
 extension-module = ["pyo3/abi3-py36", "pyo3/extension-module"]
 python-bindings = []
-llvm11-0 = ["llvm-ir/llvm-11"]
-llvm12-0 = ["llvm-ir/llvm-12"]
-llvm13-0 = ["llvm-ir/llvm-13"]
-#llvm14-0 = ["llvm-ir/llvm-14"]
+llvm11-0 = ["llvm-ir/llvm-11", "inkwell/llvm11-0"]
+llvm12-0 = ["llvm-ir/llvm-12", "inkwell/llvm12-0"]
+llvm13-0 = ["llvm-ir/llvm-13", "inkwell/llvm13-0"]
+#llvm14-0 = ["llvm-ir/llvm-14", "inkwell/llvm14-0"]
 default = ["extension-module", "python-bindings"]
 
 [lib]
diff --git a/pyqir-parser/src/parse.rs b/pyqir-parser/src/parse.rs
index 4ad20190..4d9a8277 100644
--- a/pyqir-parser/src/parse.rs
+++ b/pyqir-parser/src/parse.rs
@@ -3,6 +3,7 @@
 
 use std::convert::{TryFrom, TryInto};
 use std::num::ParseIntError;
+use std::path::Path;
 
 use llvm_ir;
 
@@ -339,3 +340,10 @@ impl NameExt for llvm_ir::Name {
         }
     }
 }
+
+pub(crate) fn verify_module_can_be_loaded(path: impl AsRef<Path>) -> Result<(), String> {
+    let context = inkwell::context::Context::create();
+    let _droppable = inkwell::module::Module::parse_bitcode_from_path(path, &context)
+        .map_err(|e| e.to_string())?;
+    Ok(())
+}
diff --git a/pyqir-parser/src/python.rs b/pyqir-parser/src/python.rs
index 954c2f21..aa812bee 100644
--- a/pyqir-parser/src/python.rs
+++ b/pyqir-parser/src/python.rs
@@ -9,6 +9,8 @@
 // from within rust, and wrappers for each class and function will be added to __init__.py so that the
 // parser API can have full python doc comments for usability.
 
+use crate::parse::verify_module_can_be_loaded;
+
 use super::parse::{
     BasicBlockExt, CallExt, ConstantExt, FunctionExt, IntructionExt, ModuleExt, NameExt, PhiExt,
     TypeExt,
@@ -32,6 +34,7 @@ fn native_module(_py: Python, m: &PyModule) -> PyResult<()> {
 
     #[pyfn(m)]
     fn module_from_bitcode(bc_path: PathBuf) -> PyResult<PyQirModule> {
+        verify_module_can_be_loaded(&bc_path).map_err(PyRuntimeError::new_err)?;
         llvm_ir::Module::from_bc_path(bc_path)
             .map(|module| PyQirModule { module })
             .map_err(PyRuntimeError::new_err)


Test patch:
diff --git a/pyqir-parser/tests/test_api.py b/pyqir-parser/tests/test_api.py
index a188e21c..131054af 100644
--- a/pyqir-parser/tests/test_api.py
+++ b/pyqir-parser/tests/test_api.py
@@ -3,6 +3,7 @@
 
 from pyqir.parser import *
 
+import pytest
 
 def test_parser():
     mod = QirModule("tests/teleportchain.baseprofile.bc")
@@ -98,6 +99,20 @@ def test_parser_zext_support():
     assert instr.target_operands[0].type.width == 1
 
 
+def test_loading_invalid_bitcode():
+    path = "tests/teleportchain.ll.reference"
+    with pytest.raises(RuntimeError) as exc_info:
+        _ = module_from_bitcode(path)
+    assert str(exc_info.value).lower() == "invalid bitcode signature"
+
+
+def test_loading_bad_bitcode_file_path():
+    path = "tests/does_not_exist.bc"
+    with pytest.raises(RuntimeError) as exc_info:
+        _ = module_from_bitcode(path)
+    assert str(exc_info.value).lower() == "no such file or directory"
+
+
 def test_parser_internals():
     mod = module_from_bitcode("tests/teleportchain.baseprofile.bc")
     func_name = "TeleportChain__DemonstrateTeleportationUsingPresharedEntanglement__Interop"