Parser should throw appropriate exception when loading invalid bitcode (

#137)

* Fixing build failure detection for subcommands
* Using inkwell to check if the module can be loaded. It has better error handling than llvm_ir.

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
 

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
+            }
         }
     }
 }

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]

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(())
+}

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)

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"