Merge pull request #198 from madsmtm/verify-under-debug-assertions

Rework verification feature selection

Author: madsmtm

.github/workflows/ci.yml

@@ -155,7 +155,7 @@ jobs:
       # Use --no-fail-fast, except with dinghy
       TESTARGS: ${{ matrix.dinghy && ' ' || '--no-fail-fast' }} ${{ matrix.test-args }}
       SOME_FEATURES: ${{ matrix.features || 'malloc,block,exception,foundation' }}
-      FEATURES: ${{ matrix.features || 'malloc,block,exception,foundation,catch-all,verify_message,uuid' }}
+      FEATURES: ${{ matrix.features || 'malloc,block,exception,foundation,catch-all,verify,uuid' }}
       UNSTABLE_FEATURES: ${{ matrix.unstable-features || 'unstable-autoreleasesafe,unstable-c-unwind' }}
       CMD: cargo
 

crates/objc2-encode/src/encoding_box.rs

@@ -105,6 +105,27 @@ impl EncodingBox {
         Encoding::ULongLong => Self::ULongLong,
         _ => unreachable!(),
     };
+
+    /// Parse and comsume an encoding from the start of a string.
+    ///
+    /// This is can be used to parse concatenated encodings, such as those
+    /// returned by `method_getTypeEncoding`.
+    ///
+    /// [`from_str`][Self::from_str] is simpler, use that instead if you can.
+    pub fn from_start_of_str(s: &mut &str) -> Result<Self, ParseError> {
+        let mut parser = Parser::new(s);
+        parser.strip_leading_qualifiers();
+
+        match parser.parse_encoding() {
+            Err(err) => Err(ParseError::new(parser, err)),
+            Ok(encoding) => {
+                let remaining = parser.remaining();
+                *s = remaining;
+
+                Ok(encoding)
+            }
+        }
+    }
 }
 
 /// Same formatting as [`Encoding`]'s `Display` implementation.
@@ -213,4 +234,19 @@ mod tests {
         assert_eq!(expected, actual);
         assert_eq!(expected.to_string(), "AA{a}");
     }
+
+    #[test]
+    fn parse_part_of_string() {
+        let mut s = "{a}c";
+
+        let expected = EncodingBox::Struct("a".into(), None);
+        let actual = EncodingBox::from_start_of_str(&mut s).unwrap();
+        assert_eq!(expected, actual);
+
+        let expected = EncodingBox::Char;
+        let actual = EncodingBox::from_start_of_str(&mut s).unwrap();
+        assert_eq!(expected, actual);
+
+        assert_eq!(s, "");
+    }
 }

crates/objc2-encode/src/parse.rs

@@ -117,6 +117,10 @@ impl<'a> Parser<'a> {
         }
     }
 
+    pub(crate) fn remaining(&self) -> &'a str {
+        &self.data[self.split_point..]
+    }
+
     fn peek(&self) -> Result<u8> {
         self.try_peek().ok_or(ErrorKind::UnexpectedEnd)
     }

crates/objc2/CHANGELOG.md

@@ -31,26 +31,35 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   in methods declared with `extern_methods!`, and let that be the `NSError**`
   parameter.
 * Added `#[method_id(...)]` attribute to `extern_methods!`.
+* Added `"verify"` feature as a replacement for the `"verify_message"`
+  feature.
 
 ### Changed
 * Allow other types than `&Class` as the receiver in `msg_send_id!` methods
   of the `new` family.
 * **BREAKING**: Changed the `Allocated` struct to be used as `Allocated<T>`
   instead of `Id<Allocated<T>, O>`.
 * Verify the message signature of overriden methods when declaring classes if
-  the `verify_message` feature is enabled.
+  the `verify` feature is enabled.
 * Verify in `declare_class!` that protocols are implemented correctly.
 * **BREAKING**: Changed the name of the attribute macro in `extern_methods`
   from `#[sel(...)]` to `#[method(...)]`.
 * **BREAKING**: Changed `extern_methods!` and `declare_class!` such that
   associated functions whoose first parameter is called `this`, is treated as
   instance methods instead of class methods.
+* **BREAKING**: Message verification is now enabled by default. Your message
+  sends might panic with `debug_assertions` enabled if they are detected to
+  be invalid. Test your code to see if that is the case!
 
 ### Fixed
 * Fixed duplicate selector extraction in `extern_methods!`.
 * Fixed using `deprecated` attributes in `declare_class!`.
 * Fixed `cfg` attributes on methods and implementations in `declare_class!`.
 
+### Removed
+* **BREAKING**: Removed `"verify_message"` feature. It has been mostly
+  replaced by `debug_assertions` and the `"verify"` feature.
+
 
 ## 0.3.0-beta.3 - 2022-09-01
 

crates/objc2/Cargo.toml

@@ -34,9 +34,8 @@ exception = ["objc-sys/unstable-exception"]
 # Wrap every `objc2::msg_send` call in a `@try/@catch` block
 catch-all = ["exception"]
 
-# Verify type encodings on every message send
-# Only intended to be used while debugging!
-verify_message = ["malloc"] # TODO: Remove malloc feature here
+# Enable all verification steps when debug assertions are enabled.
+verify = ["malloc"]
 
 # Expose features that require linking to `libc::free`.
 #

crates/objc2/src/__macro_helpers.rs

@@ -1,16 +1,16 @@
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 use alloc::vec::Vec;
 use core::ptr;
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 use std::collections::HashSet;
 
 use crate::declare::ClassBuilder;
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 use crate::declare::MethodImplementation;
 use crate::encode::Encode;
 use crate::message::__TupleExtender;
 use crate::rc::{Allocated, Id, Ownership, Shared};
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 use crate::runtime::MethodDescription;
 use crate::runtime::{Class, Object, Protocol, Sel};
 use crate::{Message, MessageArguments, MessageReceiver};
@@ -478,7 +478,7 @@ impl ModuleInfo {
 
 impl ClassBuilder {
     #[doc(hidden)]
-    #[cfg(feature = "verify_message")]
+    #[cfg(all(debug_assertions, feature = "verify"))]
     pub fn __add_protocol_methods<'a, 'b>(
         &'a mut self,
         protocol: &'b Protocol,
@@ -497,7 +497,7 @@ impl ClassBuilder {
     }
 
     #[doc(hidden)]
-    #[cfg(not(feature = "verify_message"))]
+    #[cfg(not(all(debug_assertions, feature = "verify")))]
     #[inline]
     pub fn __add_protocol_methods(&mut self, protocol: &Protocol) -> &mut Self {
         self.add_protocol(protocol);
@@ -509,7 +509,7 @@ impl ClassBuilder {
 /// - Only methods on the protocol are overriden.
 /// - TODO: The methods have the correct signature.
 /// - All required methods are overridden.
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 pub struct ClassProtocolMethodsBuilder<'a, 'b> {
     builder: &'a mut ClassBuilder,
     protocol: &'b Protocol,
@@ -521,7 +521,7 @@ pub struct ClassProtocolMethodsBuilder<'a, 'b> {
     registered_class_methods: HashSet<Sel>,
 }
 
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 impl ClassProtocolMethodsBuilder<'_, '_> {
     #[inline]
     pub unsafe fn add_method<T, F>(&mut self, sel: Sel, func: F)
@@ -579,7 +579,7 @@ impl ClassProtocolMethodsBuilder<'_, '_> {
     }
 }
 
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 impl Drop for ClassProtocolMethodsBuilder<'_, '_> {
     fn drop(&mut self) {
         for desc in &self.required_instance_methods {
@@ -778,7 +778,7 @@ mod tests {
 
     #[test]
     #[should_panic = "unexpected NULL newMethodOnInstance; receiver was NULL"]
-    #[cfg(not(feature = "verify_message"))] // Does NULL receiver checks
+    #[cfg(not(debug_assertions))] // Does NULL receiver checks
     fn test_new_any_with_null_receiver() {
         let obj: *const NSObject = ptr::null();
         let _obj: Id<Object, Shared> = unsafe { msg_send_id![obj, newMethodOnInstance] };
@@ -801,7 +801,7 @@ mod tests {
 
     #[test]
     #[should_panic = "failed allocating object"]
-    #[cfg(not(feature = "verify_message"))] // Does NULL receiver checks
+    #[cfg(not(debug_assertions))] // Does NULL receiver checks
     fn test_init_with_null_receiver() {
         let obj: Option<Allocated<RcTestObject>> = None;
         let _obj: Id<RcTestObject, Owned> = unsafe { msg_send_id![obj, init] };
@@ -823,7 +823,7 @@ mod tests {
 
     #[test]
     #[should_panic = "unexpected NULL description; receiver was NULL"]
-    #[cfg(not(feature = "verify_message"))] // Does NULL receiver checks
+    #[cfg(not(debug_assertions))] // Does NULL receiver checks
     fn test_normal_with_null_receiver() {
         let obj: *const NSObject = ptr::null();
         let _obj: Id<NSString, Shared> = unsafe { msg_send_id![obj, description] };