Create blocks with BLOCK_HAS_SIGNATURE

[goffrie]

objc2/crates/block2/src/concrete_block.rs

Line 189 in dd6c804

// TODO: Use new ABI with BLOCK_HAS_SIGNATURE

Some newer Apple APIs appear to fail in unexpected ways when called with blocks that don't have signatures. It would be useful to at least have some way of manually specifying a signature, though unsafe. Generating it automatically would be even better of course, though it's an interesting question how to actually do so.

[madsmtm]

Parts of the code for generating the encodings in a const context from a Encoding::ENCODING is done, but we're missing a few Rust features to make it a reality (see #70).

But we can definitely add a way to do it in an unsafe way for now.

Do you have an example of a few APIs that fail, so that I can add them to the test suite when implementing the feature?

[goffrie]

The ones I know of are NSFileProviderManager's reimportItemsBelowItemWithIdentifier:completionHandler: and waitForStabilizationWithCompletionHandler:, but I'm not sure how easy it would be to set up a test for those since it requires a FileProvider domain to be set up and everything.

[madsmtm]

Thanks for the examples, I haven't worked with FileProvider myself, so probably the reason why I haven't hit this particular snag myself yet.

I probably won't have the time to implement a function for creating blocks with encodings at the moment (I'm out of the country, and my focus lies elsewhere), but I will happily review and accept a PR that does it (and I could mentor an implementer as well).

[PaulDance]

I seem to have hit this limitation as well. The concerned framework is NetworkExtension, and the specific API is NEFilterDataProvider's applySettings:completionHandler.

The process crashes on the uncaught exception Block was not compiled using a compiler that inserts type information about arguments. The full error can be seen below.

Details

<NSXPCConnection: 0x600001528000> connection from pid 298 on anonymousListener or serviceListener: Warning: Exception caught during invocation of selector startFilterWithOptions:completionHandler:, dropping incoming message and invalidating the connection.
Exception: [NSXPCConnection sendInvocation]: Block was not compiled using a compiler that inserts type information about arguments. (applySettings:completionHandler:)
[NSXPCConnection sendInvocation]: Block was not compiled using a compiler that inserts type information about arguments. (applySettings:completionHandler:)
(
        0   CoreFoundation                      0x000000019f85f2ec __exceptionPreprocess + 176
        1   libobjc.A.dylib                     0x000000019f346788 objc_exception_throw + 60
        2   Foundation                          0x00000001a08dc760 -[NSXPCConnection _sendInvocation:orArguments:count:methodSignature:selector:withProxy:] + 2852
        3   Foundation                          0x00000001a08e28c4 -[NSXPCConnection _sendSelector:withProxy:arg1:arg2:] + 128
        4   Foundation                          0x00000001a08e27ec _NSXPCDistantObjectSimpleMessageSend2 + 68
        5   NetworkExtension                    0x00000001b0d17a8c -[NEFilterDataExtensionProviderContext applySettings:completionHandler:] + 84
        6   NetworkExtension                    0x00000001b0d1c910 -[NEFilterDataProvider applySettings:completionHandler:] + 168
        7   com.example.test.extension          0x00000001022cc240 _ZN64_$LT$$LP$A$C$B$RP$$u20$as$u20$objc2..encode..EncodeArguments$GT$8__invoke17h9a7d14a53f9b466aE + 120
        8   com.example.test.extension          0x00000001022ce4c4 _ZN5objc27runtime16message_receiver18msg_send_primitive4send17h2366f2fbb2f1f220E + 100
        9   com.example.test.extension          0x00000001022cf534 _ZN5objc27runtime16message_receiver15MessageReceiver12send_message17hca860006f94d41e6E + 204
        10  com.example.test.extension          0x00000001022cc104 _ZN5objc215__macro_helpers8msg_send7MsgSend12send_message17ha8ccb9f33cb3e30aE + 236
        11  com.example.test.extension          0x00000001022ccb1c _ZN23objc2_network_extension9generated22__NEFilterDataProvider20NEFilterDataProvider31applySettings_completionHandler17h5adee2dd49263920E + 108
        12  com.example.test.extension          0x00000001022c7420 start_filter_inner + 2100
        13  com.example.test.extension          0x00000001022c8c58 start_filter + 64
        14  NetworkExtension                    0x00000001b0d19b94 -[NEFilterDataExtensionProviderContext startFilterWithOptions:completionHandler:] + 568
        15  Foundation                          0x00000001a0925908 __NSXPCCONNECTION_IS_CALLING_OUT_TO_EXPORTED_OBJECT_S2__ + 16
        16  Foundation                          0x00000001a113eee0 -[NSXPCConnection _decodeAndInvokeMessageWithEvent:reply:flags:] + 1592
        17  Foundation                          0x00000001a1140558 message_handler_message + 88
        18  Foundation                          0x00000001a08e0ad4 message_handler + 152
        19  libxpc.dylib                        0x000000019f418f74 _xpc_connection_call_event_handler + 144
        20  libxpc.dylib                        0x000000019f4176e8 _xpc_connection_mach_event + 1404
        21  libdispatch.dylib                   0x000000019f55a4a8 _dispatch_client_callout4 + 20
        22  libdispatch.dylib                   0x000000019f576888 _dispatch_mach_msg_invoke + 468
        23  libdispatch.dylib                   0x000000019f561898 _dispatch_lane_serial_drain + 368
        24  libdispatch.dylib                   0x000000019f5775d8 _dispatch_mach_invoke + 444
        25  libdispatch.dylib                   0x000000019f561898 _dispatch_lane_serial_drain + 368
        26  libdispatch.dylib                   0x000000019f562578 _dispatch_lane_invoke + 432
        27  libdispatch.dylib                   0x000000019f56d2d0 _dispatch_root_queue_drain_deferred_wlh + 288
        28  libdispatch.dylib                   0x000000019f56cb44 _dispatch_workloop_worker_thread + 404
        29  libsystem_pthread.dylib             0x000000019f70700c _pthread_wqthread + 288
        30  libsystem_pthread.dylib             0x000000019f705d28 start_wqthread + 8
)

in which:

• com.example.test.extension is the PoC extension I'm trying to obtain;
• start_filter is the implementation of NEFilterDataProvider's startFilterWithCompletionHandler:;
• the extension does not call the NEFilterDataProvider implementation directly, but it is rather done by the framework through NEProvider's startSystemExtensionMode;

This is a case where it is needed to give a new block to the framework and Obj-C runtime instead of simply receiving one from it and calling it, for which there is no problem. Interestingly however, not all such cases end up triggering an error: both NEFilterManager's loadFromPreferencesWithCompletionHandler and saveToPreferencesWithCompletionHandler require and call a block given by the caller, but succeed to do so without any issue whatsoever, even though the input and output type are exactly the same as applySettings' (*mut NSError -> ()). I don't really get why some functions would check for the block's signature and not others. The only difference I see between the two cases is the value I move into the closure given to RcBlock, although I'm not sure it has an effect on anything: in (loadFrom|saveTo)Preferences...'s case, it is a Arc<dispatch::Semaphore> and in applySettings', it is an RcBlock result of a Block::copy of the block received by startFilter....

It would therefore be nice to have an easy way to make this work out of the box. In the meantime, I'll try the workaround mentioned in #434 (comment) or #615 (comment) or something like that.

[PaulDance]

Actually, it really seems to be rather linked to the Block object itself and not directly the encoding of its argument and result types. Indeed, NSError already has an appropriate RefEncode implementation; comparing the block received from the Obj-C runtime and the one created using RcBlock::new, I can observe that only in the former case are flags.has_signature, flags.has_extended_layout and descriptor.encoding set to non-default values. I therefore don't think there is a way to currently work around this.

Thinking a bit out loud how to solve this: @madsmtm, do you think it would be possible to expose a possibly-unsafe function or method of at least one of the ...Blocks in order to enable one to construct or modify a block so that it would have the given encoding set for it and the appropriate flags as well? I think it would be preferable if it could be done using the already-existing objc2::Encoding, but if it's not possible, then maybe passing the raw encoding string could also be a possibility, although I would expect it to be much more unsafe. What do you think about this as a workaround candidate?

Regarding what is mentioned in #70 (comment), I find the macro version to be an acceptable backup solution while waiting for the desired Rust features. At the very least and if I understand things correctly, it would make it an optional requirement of (Ref)?Encode, which should mean easier integration with existing codebases. The macro might need to enclose the implementation however, as I don't think a macro can access information outside of its invocation brackets, although that remains to be checked for proc-macros. Also, the main solution could be tried again to see if the more recent versions of Rust don't support the desired operations as I couldn't see anything wildly exotic in them.

Another possibility would be to generate the C string at runtime and store it in the (Rc|Stack)Block or is its memory layout too restricted? The pointer to the heap-allocated bytes could therefore be set in the BlockDescriptorSignature without creating any self-referencing issues, supposedly.

Yet another possibility would be to use some kind of proc-macro, for example a derive one that would automatically implement an additional trait for the type that only carries the wanted constant and that would be a subtrait of (Ref)?Encode.

[madsmtm]

I don't really get why some functions would check for the block's signature and not others

Yeah, me neither. A guess is that the applySettings:completionHandler: method made a breaking change sometime in its history, and perhaps didn't originally pass the error to the completion handler, but now does if the block's signature allows for it (or something like that). But that's wild speculation. I'll also note that it's newer than the other methods you mentioned.

The only difference I see between the two cases is the value I move into the closure given to RcBlock, although I'm not sure it has an effect on anything

No, that shouldn't have an effect, those contents are fully transparent to Objective-C.

Actually, it really seems to be rather linked to the Block object itself and not directly the encoding of its argument and result types.

Indeed.

expose a possibly-unsafe function or method of at least one of the ...Blocks in order to enable one to construct or modify a block so that it would have the given encoding set for it and the appropriate flags as well?

I'd be fine with that - sorry for not responding sooner, I guess I kinda wanted to respond with a PR doing exactly this, but just never got around to doing so.

I will review and accept a PR if you want to attempt doing it, though? I think what I'd prefer would be something like the following, on both StackBlock and RcBlock, let's ignore it on global blocks for now (since I don't know what I'd like the syntax to be):

pub unsafe fn with_encoding<Closure>(encoding: &'static CStr, closure: Closure) -> Self
where
    // ... Same as the existing `new` methods
{
    #[cfg(debug_assertions)]
    {
        // Somehow help ensure that the encoding is correct. Not important, but nice to have.
        A::ENCODINGS;
        R::ENCODING_RETURN;
    }
    // ... implementation
}

// Usage
let block = RcBlock::with_encoding(c"i12@?0i8", |arg: i32| {
    arg + 2
});

There's a bit more to this, especially given that the block encoding seems to include some numbers, perhaps similar to the encoding for methods? Should be documented in the method, so that users are empowered to write the correct encoding.

Also, the main solution could be tried again to see if the more recent versions of Rust don't support the desired operations as I couldn't see anything wildly exotic in them.

I'm pretty sure that we still wouldn't be able to do what I wanted in that PR (automatic const ENCODING_STR: &str; on Encode), but that doesn't mean that a solution doesn't exist for block2's use-case that I just haven't come up with yet!

If I remember correctly, the basic limitation in const that we're hitting is that there is no way to allocate, so we have to create an array to put the encoding string into, and convert that to a byte slice. To do so, we have to know the size of said array - and to do that we need to use the encoding from the generic. Something like this:

const fn compute_static_str_len(encoding: &Encoding) -> usize { todo!() }

fn new<R: EncodeReturn>() {
    // Create a temporary array to store the static encoding in
    let arr: [0u8; compute_static_str_len(&R::ENCODING_RETURN)] = [0u8; compute_static_str_len(&R::ENCODING_RETURN)];
    // Use array
}

Which doesn't work currently. But maybe there are ways around this that I don't know about?

Another possibility would be to generate the C string at runtime and store it in the (Rc|Stack)Block or is its memory layout too restricted? The pointer to the heap-allocated bytes could therefore be set in the BlockDescriptorSignature without creating any self-referencing issues, supposedly.

Yeah, there might be ways to do this by storing the string and the descriptor in the block itself (though it'd probably have to store NonNull<CStr> and manually free it when the block drops, I don't think storing Box would be correct in the Rust abstract machine, but that's an implementation detail).

A bigger problem that I see with this would be regards to the correctness: It's been a while since I combed the spec, but I don't think the lifetime of the descriptor is explicitly noted anywhere in there? So it's a possibility that there is Objective-C code out there that relies on the descriptor or the encoding being 'static instead of the lifetime being tied to the block itself.

Yet another possibility would be to use some kind of proc-macro, for example a derive one that would automatically implement an additional trait for the type that only carries the wanted constant and that would be a subtrait of (Ref)?Encode.

True, though overkill IMO.

[PaulDance]

Hey, thanks for the answer!

I will review and accept a PR if you want to attempt doing it, though?

I don't exactly need it anymore as it seems that the default settings fit my requirements just fine, but it did limit me in my ability to experiment and having this part of the API available to me would help me investigate and understand some more things in order to get more confidence about what I'm doing. I might therefore open such a PR this week, as it shouldn't be too complicated.

Which doesn't work currently.

Yes, I can indeed reproduce separately: the constant expression depends on a generic parameter error blocks any use of R where we want. I experimented a bit and didn't find much ways to circumvent this since using an array forces const at least somewhere. However, I was able to find one possibility using &'static [u8] instead and by requiring a dummy method on the concerned trait to be implemented identically by each type, as Self also counts as a generic parameter when used in the trait's definition:

trait EncodeReturn {
    const ENCODING_RETURN: Encoding;
    fn init_arr() -> &'static [u8];
}

impl Encoding {
    // The aforementioned `compute_static_str_len`.
    const fn str_len(&self) -> usize {
        todo!()
    }
}

struct X;
impl EncodeReturn for X {
    const ENCODING_RETURN: Encoding = todo!();
    // Always the same for everyone:
    fn init_arr() -> &'static [u8] {
        &[0; Self::ENCODING_RETURN.str_len()]
    }
}

fn new<R: EncodeReturn>() {
    let arr = R::init_arr();
    // Use array
}

in which new may never be const since init_arr can't either due to it simply being a trait function. I don't really known if it will exactly fit our case, but at least it worked when I used &static strs instead of &'static Encodings as both str::as_bytes and slice::len are const. If it does fit, macros should help to ensure the init_arr implementation does not have to be actually repeated every time and that it is correct every time. Using a supplementary trait may help avoid breaking compatibility.

[madsmtm]

I would really like to avoid having to change the Encode/RefEncode traits, as they're public API with a fairly clean interface, and because doing these things will be possible in a future Rust version, at which point we'll have to change the traits back :/.

I have posted an alternative in #636 (comment), let's discuss it there.

[madsmtm]

I looked into how it's implemented for FileProvider (have not yet looked at NetworkExtension, the setup for testing it is kinda bothersome):

It seems like it's implemented by passing the block's encoding to -[NSMethodSignature signatureWithObjCTypes:] to construct an NSInvocation, which is then passed to the internal fp_zeroOutReplyBlockArgumentsWithError: before executing to trigger the block. Decompiling FileProvider (and cleaning it up) yields something like:

/* @class NSInvocation(FPExtensions) */
-(void)fp_zeroOutReplyBlockArgumentsWithError:(NSError*)error {
    NSMethodSignature* signature = [self methodSignature];
    NSUInteger numArgs = [signature numberOfArguments];
    NSUInteger frameLen = [signature frameLength];
    void* zerovalue = stack - (frameLen + 0xf & 0xfffffffffffffff0); // No idea what this is
    bzero(zerovalue, frameLen);
    if (numArgs >= 2) {
        for (int i = 1; i < numArgs; i++) {
            [self setArgument:zerovalue atIndex:i];
        }
    }
    NSUInteger i = [signature fp_indexOfLastArgumentWithType:"@\"NSError\""];
    if (i != 0x7fffffffffffffff) {
        [self setArgument:error atIndex:i];
    }
}

Not entirely sure what's going on, but I think my suspicion is correct, that they do this for backwards compatibility. From this, I also see a few major points:

1. Specifying the wrong encoding is definitely a soundness issue (NSInvocation uses the signature to correctly invoke the block).
2. For this to work fully, we need the "extended type info" see this as well (not necessary for the initial implementation in Manual block encodings #636 though).