Re: [RFC v5 05/10] tcg: Add tcg opcodes and helpers for native library calls

[Richard Henderson]

On 8/25/23 03:45, Yeqi Fu wrote:
> This commit implements tcg opcodes and helpers for native library
> calls. A table is used to store the parameter types and return value
> types for each native library function. In terms of types, only three
> types are of real concern: the two base sizes int and intptr_t, and
> if the value is a pointer, tcg_gen_g2h and tcg_gen_h2g are used for
> address conversion.
>
> Signed-off-by: Yeqi Fu <fufuyqqqqqq@gmail.com>
> ---
>   accel/tcg/tcg-runtime.h      |  22 ++++
>   include/native/native-defs.h |  42 ++++++++
>   include/tcg/tcg-op-common.h  |  11 ++
>   include/tcg/tcg.h            |   9 ++
>   tcg/tcg-op.c                 | 193 ++++++++++++++++++++++++++++++++++-
>   5 files changed, 276 insertions(+), 1 deletion(-)
>   create mode 100644 include/native/native-defs.h
>
> diff --git a/accel/tcg/tcg-runtime.h b/accel/tcg/tcg-runtime.h
> index 39e68007f9..bda78b4489 100644
> --- a/accel/tcg/tcg-runtime.h
> +++ b/accel/tcg/tcg-runtime.h
> @@ -37,6 +37,28 @@ DEF_HELPER_FLAGS_1(exit_atomic, TCG_CALL_NO_WG, noreturn, 
> env)
>    */
>   #define helper_memset memset
>   DEF_HELPER_FLAGS_3(memset, TCG_CALL_NO_RWG, ptr, ptr, int, ptr)
> +
> +#define helper_memcpy memcpy
> +DEF_HELPER_FLAGS_3(memcpy, TCG_CALL_NO_RWG, ptr, ptr, ptr, ptr)
> +
> +#define helper_strncpy strncpy
> +DEF_HELPER_FLAGS_3(strncpy, TCG_CALL_NO_RWG, ptr, ptr, ptr, ptr)
> +
> +#define helper_memcmp memcmp
> +DEF_HELPER_FLAGS_3(memcmp, TCG_CALL_NO_RWG, int, ptr, ptr, ptr)
> +
> +#define helper_strncmp strncmp
> +DEF_HELPER_FLAGS_3(strncmp, TCG_CALL_NO_RWG, int, ptr, ptr, ptr)
> +
> +#define helper_strcpy strcpy
> +DEF_HELPER_FLAGS_2(strcpy, TCG_CALL_NO_RWG, ptr, ptr, ptr)
> +
> +#define helper_strcat strcat
> +DEF_HELPER_FLAGS_2(strcat, TCG_CALL_NO_RWG, ptr, ptr, ptr)
> +
> +#define helper_strcmp strcmp
> +DEF_HELPER_FLAGS_2(strcmp, TCG_CALL_NO_RWG, int, ptr, ptr)

You cannot just call these directly.  This will fail immediately whenever the guest does 
something silly like

        memcpy(NULL, "foo", 4);

This must raise SIGSEGV to the guest.

If we leave the bulk transform to tcg, the absolute minimum is

void * HELPER(memcpy)(void *dst, void *src, target_ulong len)
{
    set_helper_retaddr(GETPC());
    void *r = memcpy(dst, src, len);
    clear_helper_retaddr();
    return r;
}

There is no way to do this thread-local storage update from TCG.

But if we need to have a helper at all, we might as well do more and *not* leave the 
transform to tcg.  Something akin to

target_ulong HELPER(memcpy)(target_ulong dst, target_ulong src, target_ulong 
len)
{
    uintptr_t ra = GETPC();
    CPUState *cpu = thread_cpu;
    void *h_dst, *h_src;

    if (!h2g_valid(src)) {
       cpu_loop_exit_sigsegv(cpu, src, MMU_DATA_LOAD, 1, ra);
    }
    if (!h2g_valid(dst)) {
       cpu_loop_exit_sigsegv(cpu, dst, MMU_DATA_STORE, 1, ra);
    }

    set_helper_retaddr(ra);
    memcpy(g2h(cpu, dst), g2h(cpu, src), len);
    clear_helper_retaddr(ra);

    /* memcpy always returns its first argument */
    return dst;
}


> --- /dev/null
> +++ b/include/native/native-defs.h
> @@ -0,0 +1,42 @@
> +/*
> + * Argument encoding. We only really care about 3 types. The two base
> + * sizes (int and intptr_t) and if the value is a pointer (in which
> + * case we need to adjust it g2h before passing to the native
> + * function).
> + */
> +#define TYPE_NO_ARG 0x0
> +#define TYPE_INT_ARG 0x1
> +#define TYPE_IPTR_ARG 0x2
> +#define TYPE_PTR_ARG 0x3
> +
> +#define ENCODE_TYPE(ret_value, arg1, arg2, arg3) \
> +    ((ret_value) | (arg1 << 4) | (arg2 << 8) | (arg3 << 12))

Supposing we do the transform in tcg, this duplicates include/exec/helper-head.h, and 
dh_typemask().

> +static const FuncHelper func_helper_table[] = {
> +    { .func = "memset",
> +      .helper = (helper_func)gen_helper_memset,
> +      .type = TYPE_AAIP },
> +    { .func = "memcpy",
> +      .helper = (helper_func)gen_helper_memcpy,
> +      .type = TYPE_AAAP },
> +    { .func = "strncpy",
> +      .helper = (helper_func)gen_helper_strncpy,
> +      .type = TYPE_AAAP },
> +    { .func = "memcmp",
> +      .helper = (helper_func)gen_helper_memcmp,
> +      .type = TYPE_IAAP },
> +    { .func = "strncmp",
> +      .helper = (helper_func)gen_helper_strncmp,
> +      .type = TYPE_IAAP },
> +    { .func = "strcpy",
> +      .helper = (helper_func)gen_helper_strcpy,
> +      .type = TYPE_AAA },
> +    { .func = "strcat",
> +      .helper = (helper_func)gen_helper_strcat,
> +      .type = TYPE_AAA },
> +    { .func = "strcmp",
> +      .helper = (helper_func)gen_helper_strcmp,
> +      .type = TYPE_IAA },
> +};
> +/* p: iptr ; i: i32 ; a: ptr(address) */
> +void gen_native_call_i32(const char *func_name, TCGv_i32 ret, TCGv_i32 arg1,
> +                         TCGv_i32 arg2, TCGv_i32 arg3)
> +{
> +    TCGv_ptr arg1_ptr = tcg_temp_new_ptr();
> +    TCGv_ptr arg2_ptr = tcg_temp_new_ptr();
> +    TCGv_ptr arg3_ptr = tcg_temp_new_ptr();
> +    TCGv_ptr ret_ptr = tcg_temp_new_ptr();
> +    unsigned int i;
> +    for (i = 0; i < sizeof(func_helper_table) / sizeof(FuncHelper); i++) {
> +        if (strcmp(func_name, func_helper_table[i].func) == 0) {
> +            break;
> +        }
> +    }
> +    g_assert(i < sizeof(func_helper_table) / sizeof(FuncHelper));

So you assert() if libnative.so gives you something that doesn't match?

Surely returning false here, and allowing the translator to fail the transformation is 
better.  This would generate SIGILL at runtime, which still kills the guest program, but 
it feels cleaner.  You could also do

  qemu_log_mask(LOG_GUEST_ERROR, "Unimplemented libnative call to \"%s\"\n", 
func_name);


r~