Merge pull request rust-lang#99 from wasmerio/stringradix

Improve safety of const_int_from_string function by only allowing valid radixes.

Author: TheDan64

Cargo.toml

@@ -28,6 +28,7 @@ enum-methods = "0.0.8"
 inkwell_internal_macros = { path = "./internal_macros", version = "0.1.0" }
 libc = "0.2"
 llvm-sys = "80.1"
+regex = "1"
 
 [badges]
 travis-ci = { repository = "TheDan64/inkwell" }

src/types/int_type.rs

@@ -9,6 +9,47 @@ use crate::types::traits::AsTypeRef;
 use crate::types::{Type, ArrayType, BasicTypeEnum, VectorType, PointerType, FunctionType};
 use crate::values::{AsValueRef, ArrayValue, GenericValue, IntValue};
 
+use regex::Regex;
+
+/// How to interpret a string or digits used to construct an integer constant.
+#[derive(Clone, Copy, Debug, EnumAsGetters, EnumIntoGetters, EnumToGetters, Eq, Hash, PartialEq)]
+pub enum StringRadix {
+    /// Binary 0 or 1
+    Binary = 2,
+    /// Octal 0-7
+    Octal = 8,
+    /// Decimal 0-9
+    Decimal = 10,
+    /// Hexadecimal with upper or lowercase letters up to F.
+    Hexadecimal = 16,
+    /// Alphanumeric, 0-9 and all 26 letters in upper or lowercase.
+    Alphanumeric = 36,
+}
+impl StringRadix {
+    /// Create a radix from a base.
+    pub fn from_u8(value: u8) -> Option<StringRadix> {
+        match value {
+            2 => Some(StringRadix::Binary),
+            8 => Some(StringRadix::Octal),
+            10 => Some(StringRadix::Decimal),
+            16 => Some(StringRadix::Hexadecimal),
+            36 => Some(StringRadix::Alphanumeric),
+            _ => None
+        }
+    }
+
+    /// Create a Regex that matches valid strings for the given radix.
+    pub fn to_regex(&self) -> Regex {
+        match self {
+            StringRadix::Binary => Regex::new(r"^[-+]?[01]+$").unwrap(),
+            StringRadix::Octal => Regex::new(r"^[-+]?[0-7]+$").unwrap(),
+            StringRadix::Decimal => Regex::new(r"^[-+]?[0-9]+$").unwrap(),
+            StringRadix::Hexadecimal => Regex::new(r"^[-+]?[0-9abcdefABCDEF]+$").unwrap(),
+            StringRadix::Alphanumeric => Regex::new(r"^[-+]?[0-9[:alpha:]]+$").unwrap(),
+        }
+    }
+}
+
 /// An `IntType` is the type of an integer constant or variable.
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub struct IntType {
@@ -209,32 +250,32 @@ impl IntType {
     ///
     /// ```no_run
     /// use inkwell::context::Context;
+    /// use inkwell::types::StringRadix;
     ///
     /// let context = Context::create();
     /// let i8_type = context.i8_type();
-    /// let i8_val = i8_type.const_int_from_string("0121", 10);
+    /// let i8_val = i8_type.const_int_from_string("0121", StringRadix::Decimal).unwrap();
     ///
     /// assert_eq!(i8_val.print_to_string().to_string(), "i8 121");
     ///
-    /// let i8_val = i8_type.const_int_from_string("0121", 3);
+    /// let i8_val = i8_type.const_int_from_string("0121", StringRadix::from_u8(10).unwrap()).unwrap();
     ///
     /// assert_eq!(i8_val.print_to_string().to_string(), "i8 16");
     ///
-    /// // Unexpected outputs:
-    /// let i8_val = i8_type.const_int_from_string("0121", 2);
-    ///
-    /// assert_eq!(i8_val.print_to_string().to_string(), "i8 3");
-    ///
-    /// let i8_val = i8_type.const_int_from_string("ABCD", 2);
+    /// let i8_val = i8_type.const_int_from_string("0121", StringRadix::Binary);
+    /// assert!(i8_val.is_none());
     ///
-    /// assert_eq!(i8_val.print_to_string().to_string(), "i8 -15");
+    /// let i8_val = i8_type.const_int_from_string("ABCD", StringRadix::Binary);
+    /// assert!(i8_val.is_none());
     /// ```
-    pub fn const_int_from_string(&self, slice: &str, radix: u8) -> IntValue {
+    pub fn const_int_from_string(&self, slice: &str, radix: StringRadix) -> Option<IntValue> {
+        if !radix.to_regex().is_match(slice) {
+            return None
+        }
         let value = unsafe {
-            LLVMConstIntOfStringAndSize(self.as_type_ref(), slice.as_ptr() as *const i8, slice.len() as u32, radix)
+            LLVMConstIntOfStringAndSize(self.as_type_ref(), slice.as_ptr() as *const i8, slice.len() as u32, radix as u8)
         };
-
-        IntValue::new(value)
+        Some(IntValue::new(value))
     }
 
     /// Create a constant `IntValue` of arbitrary precision.

src/types/mod.rs

@@ -24,7 +24,7 @@ pub use crate::types::array_type::ArrayType;
 pub use crate::types::enums::{AnyTypeEnum, BasicTypeEnum};
 pub use crate::types::float_type::FloatType;
 pub use crate::types::fn_type::FunctionType;
-pub use crate::types::int_type::IntType;
+pub use crate::types::int_type::{IntType, StringRadix};
 pub use crate::types::ptr_type::PointerType;
 pub use crate::types::struct_type::StructType;
 pub use crate::types::traits::{AnyType, BasicType, IntMathType, FloatMathType, PointerMathType};

tests/all/test_values.rs

@@ -3,7 +3,7 @@ extern crate inkwell;
 use self::inkwell::{DLLStorageClass, FloatPredicate, GlobalVisibility, ThreadLocalMode, AddressSpace};
 use self::inkwell::context::Context;
 use self::inkwell::module::Linkage::*;
-use self::inkwell::types::{StructType, VectorType};
+use self::inkwell::types::{StringRadix, StructType, VectorType};
 use self::inkwell::values::{InstructionOpcode::*, MetadataValue, FIRST_CUSTOM_METADATA_KIND_ID, VectorValue};
 #[llvm_versions(7.0..=latest)]
 use self::inkwell::comdat::ComdatSelectionKind;
@@ -711,23 +711,19 @@ fn test_function_value_no_params() {
 fn test_value_from_string() {
     let context = Context::create();
     let i8_type = context.i8_type();
-    let i8_val = i8_type.const_int_from_string("0121", 10);
+    let i8_val = i8_type.const_int_from_string("0121", StringRadix::Decimal).unwrap();
 
     assert_eq!(*i8_val.print_to_string(), *CString::new("i8 121").unwrap());
 
-    let i8_val = i8_type.const_int_from_string("0121", 3);
+    let i8_val = i8_type.const_int_from_string("0121", StringRadix::from_u8(10).unwrap()).unwrap();
 
-    assert_eq!(*i8_val.print_to_string(), *CString::new("i8 16").unwrap());
+    assert_eq!(i8_val.print_to_string().to_string(), "i8 121");
 
-    // LLVM will not throw an error, just parse until it can parse no more (and
-    // possibly spit out something completely unexpected):
-    let i8_val = i8_type.const_int_from_string("0121", 2);
-
-    assert_eq!(*i8_val.print_to_string(), *CString::new("i8 3").unwrap());
-
-    let i8_val = i8_type.const_int_from_string("ABCD", 2);
-
-    assert_eq!(*i8_val.print_to_string(), *CString::new("i8 -15").unwrap());
+    assert_eq!(i8_type.const_int_from_string("", StringRadix::Binary), None);
+    assert_eq!(i8_type.const_int_from_string("-", StringRadix::Binary), None);
+    assert_eq!(i8_type.const_int_from_string("--1", StringRadix::Binary), None);
+    assert_eq!(i8_type.const_int_from_string("2", StringRadix::Binary), None);
+    assert_eq!(i8_type.const_int_from_string("2", StringRadix::Binary), None);
 
     // Floats
     let f64_type = context.f64_type();