Fix of aslresponse_next and aslresponse_free deprecation crashes on iOS 7

[rodmaz]

The current implementation of deprecation of aslresponse_free and aslresponse_next is incorrect.
Code compiled with Xcode 6 (iOS 8) will crash when executed on iOS 7.
This check must be also done during runtime.

[rodmaz]

This is the correct approach in handling the different versions of the framework.
We have to take care of it both during compile-time and runtime:

#if defined(__IPHONE_8_0) || defined(__MAC_10_10)
                if (floor(NSFoundationVersionNumber) <= NSFoundationVersionNumber_iOS_7_1)
                {
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
                    msg = aslresponse_next(response);
#pragma GCC diagnostic pop
                } else {
                    msg = asl_next(response);
                }
#else
                msg = aslresponse_next(response);
#endif
                while (msg)
                {
                    [DDASLLogCapture aslMessageRecieved:msg];

                    // Keep track of which messages we've seen.
                    lastSeenID = atoll(asl_get(msg, ASL_KEY_MSG_ID));
                }

[rodmaz]

The same with asl_release:

#if defined(__IPHONE_8_0) || defined(__MAC_10_10)
                if (floor(NSFoundationVersionNumber) <= NSFoundationVersionNumber_iOS_7_1)
                {
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
                    aslresponse_free(response);
#pragma GCC diagnostic pop
                } else {
                    asl_release(response);
                }
#else
                aslresponse_free(response);
#endif

[rivera-ernesto]

Would you lie to make a pull request?

Also if you could shed some light on discussion #324 it would be great.

[rodmaz]

This is tricky. Apple header file is wrong. It says asl_next was introduced on iOS 7. The correct is iOS 8.

asl_object_t asl_next(asl_object_t obj) __OSX_AVAILABLE_STARTING(__MAC_10_10, __IPHONE_7_0);
void asl_release(asl_object_t obj) __OSX_AVAILABLE_STARTING(__MAC_10_10, __IPHONE_7_0);

If we compile this on Xcode 6 beta x, we don't get both symbols compiled as weak symbols. If you build a library, which incorporates CocoaLumberjack (our case), Xcode 5 will complain.

I tried to add the weak symbol by directly adding it to the DDASLLogCapture.m:

extern asl_object_t asl_next(asl_object_t obj) __attribute__((weak_import));
extern void asl_release(asl_object_t obj) __attribute__((weak_import));

Both symbols are then correctly compiled as weak:

nm -m libMyLibrary.a | grep _asl
                 (undefined) external _asl_free
                 (undefined) external _asl_get
                 (undefined) external _asl_log
                 (undefined) external _asl_new
                 (undefined) weak external _asl_next
                 (undefined) external _asl_open
                 (undefined) weak external _asl_release
                 (undefined) external _asl_search
                 (undefined) external _asl_set
                 (undefined) external _asl_set_query
                 (undefined) external _aslresponse_free
                 (undefined) external _aslresponse_next

As weaks symbols, Xcode 5 should not complain about it and simply generate NULL pointers, in case both functions are not available. That's not what happens, and LLVM tries to find the weak symbols during linkage. This looks like a bug in the linker in how it handles weak symbols.

[rivera-ernesto]

I think we should wait a little bit more until Xcode 6 gets closer to release.

In the mean time filing a bug could be a good idea.

[problame]

Has Apple responded to the radar yet? (He asked sarcastically)
Anyways, what has happened regarding this issue?

[rodmaz]

If you compile on Xcode 6 GM or later, this is fixed.

[rivera-ernesto]

Nice.

[ghost]

i just started getting this issue, but i'm on the normal Xcode 6 release i believe

[rivera-ernesto]

Reopened this.

Will get fixed by either #362 or #365.

[DivineDominion]

I ran into the same issue building for Mac.

(I ended up adding helper methods to hide away the switches because at first I tried something more complicated. I kind of like it this way, though.)

What about this? Does the following work in your cases?

+ (void)captureAslLogs {
    @autoreleasepool
    {
        /*
           We use ASL_KEY_MSG_ID to see each message once, but there's no
           obvious way to get the "next" ID. To bootstrap the process, we'll
           search by timestamp until we've seen a message.
         */

        struct timeval timeval = {
            .tv_sec = 0
        };
        gettimeofday(&timeval, NULL);
        unsigned long long startTime = timeval.tv_sec;
        __block unsigned long long lastSeenID = 0;

        /*
           syslogd posts kNotifyASLDBUpdate (com.apple.system.logger.message)
           through the notify API when it saves messages to the ASL database.
           There is some coalescing - currently it is sent at most twice per
           second - but there is no documented guarantee about this. In any
           case, there may be multiple messages per notification.

           Notify notifications don't carry any payload, so we need to search
           for the messages.
         */
        int notifyToken = 0;  // Can be used to unregister with notify_cancel().
        notify_register_dispatch(kNotifyASLDBUpdate, &notifyToken, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^(int token)
        {
            // At least one message has been posted; build a search query.
            @autoreleasepool
            {
                aslmsg query = asl_new(ASL_TYPE_QUERY);
                char stringValue[64];

                if (lastSeenID > 0) {
                    snprintf(stringValue, sizeof stringValue, "%llu", lastSeenID);
                    asl_set_query(query, ASL_KEY_MSG_ID, stringValue, ASL_QUERY_OP_GREATER | ASL_QUERY_OP_NUMERIC);
                } else {
                    snprintf(stringValue, sizeof stringValue, "%llu", startTime);
                    asl_set_query(query, ASL_KEY_TIME, stringValue, ASL_QUERY_OP_GREATER_EQUAL | ASL_QUERY_OP_NUMERIC);
                }

                [DDASLLogCapture configureAslQuery:query];

                // Iterate over new messages.
                aslmsg msg;
                aslresponse response = asl_search(NULL, query);
                while ((msg = [self nextMessageFromResponse:response]))
                {
                    [DDASLLogCapture aslMessageRecieved:msg];

                    // Keep track of which messages we've seen.
                    lastSeenID = atoll(asl_get(msg, ASL_KEY_MSG_ID));
                }
                [self releaseResponse:response];

                if (_cancel) {
                    notify_cancel(notifyToken);
                    return;
                }

                free(query);
            }
        });
    }
}

+ (aslmsg)nextMessageFromResponse:(aslresponse)response
{
#if (__MAC_OS_X_VERSION_MIN_REQUIRED < __MAC_10_10 || __IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_8_0)
    return aslresponse_next(response);
#else
    return asl_next(response);
#endif
}

+ (void)releaseResponse:(aslresponse)response
{
#if (__MAC_OS_X_VERSION_MIN_REQUIRED < __MAC_10_10 || __IPHONE_OS_VERSION_MIN_REQUIRED < __IPHONE_8_0)
    aslresponse_free(response);
#else
    asl_release(response);
#endif
}

[rivera-ernesto]

#365 just got merged so this should be fixed.

Could everyone try it so we can close this issue and release?

pod 'CocoaLumberjack', :head

[DivineDominion]

Bad luck:

Undefined symbols for architecture x86_64:
  "_asl_next", referenced from:
      +[DDASLLogCapture initialize] in libPods-CocoaLumberjack.a(DDASLLogCapture.o)
     (maybe you meant: _dd_asl_next)
  "_asl_release", referenced from:
      +[DDASLLogCapture initialize] in libPods-CocoaLumberjack.a(DDASLLogCapture.o)
     (maybe you meant: _dd_asl_release)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Xcode 6.1 with SDK for Mac OS 10.10, building for 10.9

[ghost]

thanks for the feedback @DivineDominion, I'll be trying to resolve this by beginning of next week.

[ghost]

i fixed my 7.1 device startup crash with #376, and it MAY have fixed the issue @DivineDominion ran into.

[rivera-ernesto]

@DivineDominion Another try with changes from #376?

[DivineDominion]

It works fine!

I'd suggest moving the macro definitions outside +initialize to not raise the false impression they will only be executed when this method runs.

The part in question:

#ifdef __IPHONE_8_0
    #define DDASL_IOS_PIVOT_VERSION __IPHONE_8_0
#endif
#ifdef __MAC_10_10
    #define DDASL_OSX_PIVOT_VERSION __MAC_10_10
#endif

[ghost]

¡that's great to hear @DivineDominion!
i also agree with your point, but i won't be doing it in the near term.
¿will you do it yourself?

[DivineDominion]

Of course! Cf. #378

[bpoplauschi]

#378 is now merged, so I will close this issue. Please let me know if there is still an issue here.

[ghost]

sounds good, i'll switch back to the official fork today and let you know if i run into any issues