Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework verification feature selection #198

Merged
merged 4 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
36 changes: 36 additions & 0 deletions crates/objc2-encode/src/encoding_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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, "");
}
}
4 changes: 4 additions & 0 deletions crates/objc2-encode/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
11 changes: 10 additions & 1 deletion crates/objc2/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 2 additions & 3 deletions crates/objc2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
#
Expand Down
24 changes: 12 additions & 12 deletions crates/objc2/src/__macro_helpers.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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] };
Expand All @@ -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] };
Expand All @@ -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] };
Expand Down
Loading