Darwin syscalls calling convention doesn't clobber CF as it should

[mostobriv]

Version and Platform (required):

• Binary Ninja Version: 5.4.9459-dev Personal (b2c37a6b)
• Edition: Non-Commercial

Bug Description:
Actually it's a semi-feature request semi-bug.

Currently when lifting syscalls on Darwin platforms, BN doesn't use CF as one of the return values of a syscall (on darwin platforms system calls return success status in CarryFlag, CF = 0 - success, fail otherwise), which leads to incorrect IL code being generated when the original code takes CF into account.

Here is example of such case:

IL uses cond:0 from earlier comparison instead of using syscalls result.

Steps To Reproduce:
Please provide all steps required to reproduce the behavior:

1. Open sharedcache
2. Navigate to some syscall wrapper (for example getpid)

Expected Behavior:
BN should clobber the CF and count it.

Binary:
I won't upload whole ios sharedcache, but you can check yourself with any of syscall wrappers there

Additional Information:
As @ glenns suggested me in slack, it can be workarounded via tweaking lifting of syscall instruction, but it is still a workaround and not as clean as it desired to be.

[mostobriv]

So, as i've mentioned (thanks to Glenn), probable workaround is to register ArchitectureHook replicating such patch:

diff --git a/arch/arm64/il.cpp b/arch/arm64/il.cpp
--- a/arch/arm64/il.cpp
+++ b/arch/arm64/il.cpp
@@ -3659,5 +3659,5 @@
 		il.AddInstruction(
 		    il.SetRegister(4, FAKEREG_SYSCALL_INFO, il.Const(4, el_mark | IMM_O(operand1))));
-		il.AddInstruction(il.SystemCall());
+		il.AddInstruction(il.AddExpr(LLIL_SYSCALL, 0, GetFlagWriteTypeForEffect(FlagEffect::FLAGEFFECT_SETS)));
 		break;
 	}

which is leading to uninitialized variable appears right after the syscall instruction:

instead of current state:

[mostobriv]

Here is small repro binary. It is generated by LLM, however it looks correct and highlight the issue

Source code

#include <stdio.h>
#include <fcntl.h>
#include <sys/syscall.h>

/*
 * macOS arm64 syscall ABI:
 *   x16 = syscall number (Unix class: positive number from sys/syscall.h)
 *   x0..x5 = args
 *   svc #0x80
 * On return:
 *   carry flag (C bit of NZCV) set => error, x0 holds errno
 *   carry clear                   => success, x0 holds return value
 *
 * We capture the carry flag with `cset` into a separate register and
 * return errno (negated) from x0 when an error occurred, otherwise the
 * syscall's return value. Callers distinguish by sign: negative => -errno.
 */

static inline long do_syscall0(long num)
{
    register long x16 __asm__("x16") = num;
    register long x0  __asm__("x0");
    long err;
    __asm__ volatile (
        "svc        #0x80      \n\t"
        "cset       %1, cs     \n\t"   /* err = 1 if carry set, else 0 */
        : "=r"(x0), "=r"(err)
        : "r"(x16)
        : "cc", "memory"
    );
    return err ? -x0 : x0;
}

static inline long do_syscall3(long num, long a0, long a1, long a2)
{
    register long x16 __asm__("x16") = num;
    register long x0  __asm__("x0")  = a0;
    register long x1  __asm__("x1")  = a1;
    register long x2  __asm__("x2")  = a2;
    long err;
    __asm__ volatile (
        "svc        #0x80      \n\t"
        "cset       %1, cs     \n\t"
        : "+r"(x0), "=r"(err)
        : "r"(x16), "r"(x1), "r"(x2)
        : "cc", "memory"
    );
    return err ? -x0 : x0;
}

static inline long do_syscall1(long num, long a0)
{
    return do_syscall3(num, a0, 0, 0);
}

static long sys_getpid(void)            { return do_syscall0(SYS_getpid); }
static long sys_getppid(void)           { return do_syscall0(SYS_getppid); }
static long sys_open(const char *p, int flags, int mode)
{
    return do_syscall3(SYS_open, (long)p, flags, mode);
}
static long sys_close(int fd)           { return do_syscall1(SYS_close, fd); }

static void report(const char *name, long r)
{
    if (r < 0)
        printf("%-8s -> error, errno=%ld\n", name, -r);
    else
        printf("%-8s -> %ld\n", name, r);
}

int main(int argc, const char** const argv)
{
    if (argc < 2) {
        puts("Missing 'path' argument");
        return 1;
    }

    report("getpid",  sys_getpid());
    report("getppid", sys_getppid());

    long fd = sys_open(argv[1], O_RDONLY, 0);
    report("open",    fd);
    if (fd >= 0)
        report("close", sys_close((int)fd));

    /* Trigger an error path to exercise CF handling. */
    long bad = sys_open("/no/such/path/definitely", O_RDONLY, 0);
    report("open",    bad);

    long badclose = sys_close(9999);
    report("close",   badclose);

    return 0;
}

darwin_syscalls.zip

[fuzyll]

I don't know that the proposed solution in this ticket is the right way to go about fixing this more generally. I'm not sure what that would do to other platforms, since it's a hook into the architecture itself. I am not sure if the new calling convention stuff that Rusty just landed will get us all of the way to what we need here, either. So, I think this would require a larger change.

The proposed change above as an architecture hook should totally work just fine as a workaround for anyone running into this that predominantly looks at Darwin stuff, though.