Re: [PATCH v2 3/3] linux-user: Add strace support for printing arguments for ioctls used for terminals and serial lines

[Laurent Vivier]

Le 23/07/2020 à 23:02, Filip Bozuta a écrit :
> Functions "print_ioctl()" and "print_syscall_ret_ioctl()" are used
> to print arguments of "ioctl()" with "-strace". These functions
> use "thunk_print()", which is defined in "thunk.c", to print the
> contents of ioctl's third arguments that are not basic types.
> 
> However, this function doesn't handle ioctls of group ioctl_tty which
> are used for terminals and serial lines. These ioctls use a type
> "struct termios" which thunk type is defined in a non standard
> way using "STRUCT_SPECIAL()". This means that this type is not decoded
> regularly using "thunk_convert()" and uses special converting functions
> "target_to_host_termios()" and "host_to_target_termios()", which are defined
> in "syscall.c" to decode it's values.
> 
> For simillar reasons, this type is also not printed regularly using
> "thunk_print()". That is the reason why a separate printing function
> "print_termios()" is defined in file "strace.c". This function decodes
> and prints flag values of the "termios" structure.
> 
> Implementation notes:
> 
>     Function "print_termios()" was implemented in "strace.c" using
>     an existing function "print_flags()" to print flag values of
>     "struct termios" fields. Also, recently implemented function
>     "print_enums()" was also used to print enumareted values which
>     are contained in the fields of 'struct termios'.
> 
>     These flag values were defined using an existing macro "FLAG_TARGET()"
>     that generates aproppriate target flag values and string representations
>     of these flags. Also, the recently defined macro "ENUM_TARGET()" was
>     used to generate aproppriate enumarated values and their respective
>     string representations.
> 
>     Function "print_termios()" was declared in "qemu.h" so that it can
>     be accessed in "syscall.c". Type "StructEntry" defined in
>     "exec/user/thunk.h" contains information that is used to decode
>     structure values. Field "void print(void *arg)" was added in this
>     structure as a special print function. Also, function "thunk_print()"
>     was changed a little so that it uses this special print function
>     in case it is defined. This printing function was instantiated with
>     the defined "print_termios()" in "syscall.c" in "struct_termios_def".
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@syrmia.com>
> ---
>  include/exec/user/thunk.h |   1 +
>  linux-user/qemu.h         |   1 +
>  linux-user/strace.c       | 195 ++++++++++++++++++++++++++++++++++++++
>  linux-user/syscall.c      |   1 +
>  thunk.c                   |  23 +++--
>  5 files changed, 212 insertions(+), 9 deletions(-)
> 
> diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
> index 7992475c9f..a5bbb2c733 100644
> --- a/include/exec/user/thunk.h
> +++ b/include/exec/user/thunk.h
> @@ -55,6 +55,7 @@ typedef struct {
>      int *field_offsets[2];
>      /* special handling */
>      void (*convert[2])(void *dst, const void *src);
> +    void (*print)(void *arg);
>      int size[2];
>      int align[2];
>      const char *name;
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index f431805e57..a69a0bd347 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -706,6 +706,7 @@ static inline uint64_t target_offset64(uint64_t word0, 
> uint64_t word1)
>  }
>  #endif /* TARGET_ABI_BITS != 32 */
>  
> +void print_termios(void *arg);
>  
>  /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
>  #ifdef TARGET_ARM
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 3f16bb2c53..b9ba39ce6e 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1284,6 +1284,140 @@ UNUSED static struct flags falloc_flags[] = {
>  #endif
>  };
>  
> +UNUSED static struct flags termios_iflags[] = {
> +    FLAG_TARGET(IGNBRK),
> +    FLAG_TARGET(BRKINT),
> +    FLAG_TARGET(IGNPAR),
> +    FLAG_TARGET(PARMRK),
> +    FLAG_TARGET(INPCK),
> +    FLAG_TARGET(ISTRIP),
> +    FLAG_TARGET(INLCR),
> +    FLAG_TARGET(IGNCR),
> +    FLAG_TARGET(ICRNL),
> +    FLAG_TARGET(IUCLC),
> +    FLAG_TARGET(IXON),
> +    FLAG_TARGET(IXANY),
> +    FLAG_TARGET(IXOFF),
> +    FLAG_TARGET(IMAXBEL),
> +    FLAG_TARGET(IUTF8),
> +    FLAG_END,
> +};
> +
> +UNUSED static struct flags termios_oflags[] = {
> +    FLAG_TARGET(OPOST),
> +    FLAG_TARGET(OLCUC),
> +    FLAG_TARGET(ONLCR),
> +    FLAG_TARGET(OCRNL),
> +    FLAG_TARGET(ONOCR),
> +    FLAG_TARGET(ONLRET),
> +    FLAG_TARGET(OFILL),
> +    FLAG_TARGET(OFDEL),
> +    FLAG_END,
> +};
> +
> +UNUSED static struct enums termios_oflags_NLDLY[] = {
> +    ENUM_TARGET(NL0),
> +    ENUM_TARGET(NL1),
> +    ENUM_END,
> +};
> +
> +UNUSED static struct enums termios_oflags_CRDLY[] = {
> +    ENUM_TARGET(CR0),
> +    ENUM_TARGET(CR1),
> +    ENUM_TARGET(CR2),
> +    ENUM_TARGET(CR3),
> +    ENUM_END,
> +};
> +
> +UNUSED static struct enums termios_oflags_TABDLY[] = {
> +    ENUM_TARGET(TAB0),
> +    ENUM_TARGET(TAB1),
> +    ENUM_TARGET(TAB2),
> +    ENUM_TARGET(TAB3),
> +    ENUM_END,
> +};
> +
> +UNUSED static struct enums termios_oflags_VTDLY[] = {
> +    ENUM_TARGET(VT0),
> +    ENUM_TARGET(VT1),
> +    ENUM_END,
> +};
> +
> +UNUSED static struct enums termios_oflags_FFDLY[] = {
> +    ENUM_TARGET(FF0),
> +    ENUM_TARGET(FF1),
> +    ENUM_END,
> +};
> +
> +UNUSED static struct enums termios_oflags_BSDLY[] = {
> +    ENUM_TARGET(BS0),
> +    ENUM_TARGET(BS1),
> +    ENUM_END,
> +};
> +
> +UNUSED static struct enums termios_cflags_CBAUD[] = {
> +    ENUM_TARGET(B0),
> +    ENUM_TARGET(B50),
> +    ENUM_TARGET(B75),
> +    ENUM_TARGET(B110),
> +    ENUM_TARGET(B134),
> +    ENUM_TARGET(B150),
> +    ENUM_TARGET(B200),
> +    ENUM_TARGET(B300),
> +    ENUM_TARGET(B600),
> +    ENUM_TARGET(B1200),
> +    ENUM_TARGET(B1800),
> +    ENUM_TARGET(B2400),
> +    ENUM_TARGET(B4800),
> +    ENUM_TARGET(B9600),
> +    ENUM_TARGET(B19200),
> +    ENUM_TARGET(B38400),
> +    ENUM_TARGET(B57600),
> +    ENUM_TARGET(B115200),
> +    ENUM_TARGET(B230400),
> +    ENUM_TARGET(B460800),
> +    ENUM_END,
> +};
> +
> +UNUSED static struct enums termios_cflags_CSIZE[] = {
> +    ENUM_TARGET(CS5),
> +    ENUM_TARGET(CS6),
> +    ENUM_TARGET(CS7),
> +    ENUM_TARGET(CS8),
> +    ENUM_END,
> +};
> +
> +UNUSED static struct flags termios_cflags[] = {
> +    FLAG_TARGET(CSTOPB),
> +    FLAG_TARGET(CREAD),
> +    FLAG_TARGET(PARENB),
> +    FLAG_TARGET(PARODD),
> +    FLAG_TARGET(HUPCL),
> +    FLAG_TARGET(CLOCAL),
> +    FLAG_TARGET(CRTSCTS),
> +    FLAG_END,
> +};
> +
> +UNUSED static struct flags termios_lflags[] = {
> +    FLAG_TARGET(ISIG),
> +    FLAG_TARGET(ICANON),
> +    FLAG_TARGET(XCASE),
> +    FLAG_TARGET(ECHO),
> +    FLAG_TARGET(ECHOE),
> +    FLAG_TARGET(ECHOK),
> +    FLAG_TARGET(ECHONL),
> +    FLAG_TARGET(NOFLSH),
> +    FLAG_TARGET(TOSTOP),
> +    FLAG_TARGET(ECHOCTL),
> +    FLAG_TARGET(ECHOPRT),
> +    FLAG_TARGET(ECHOKE),
> +    FLAG_TARGET(FLUSHO),
> +    FLAG_TARGET(PENDIN),
> +    FLAG_TARGET(IEXTEN),
> +    FLAG_TARGET(EXTPROC),
> +    FLAG_END,
> +};
> +
>  UNUSED static struct flags mlockall_flags[] = {
>      FLAG_TARGET(MCL_CURRENT),
>      FLAG_TARGET(MCL_FUTURE),
> @@ -1579,6 +1713,67 @@ print_itimerval(abi_ulong it_addr, int last)
>      }
>  }
>  
> +void
> +print_termios(void *arg)
> +{
> +    const struct target_termios *target = arg;
> +
> +    target_tcflag_t iflags = tswap32(target->c_iflag);
> +    target_tcflag_t oflags = tswap32(target->c_oflag);
> +    target_tcflag_t cflags = tswap32(target->c_cflag);
> +    target_tcflag_t lflags = tswap32(target->c_lflag);
> +
> +    qemu_log("{");
> +
> +    qemu_log("c_iflag = ");
> +    print_flags(termios_iflags, iflags, 0);
> +
> +    qemu_log("c_oflag = ");
> +    target_tcflag_t oflags_clean =  oflags & ~(TARGET_NLDLY | TARGET_CRDLY |
> +                                               TARGET_TABDLY | TARGET_BSDLY |
> +                                               TARGET_VTDLY | TARGET_FFDLY);
> +    print_flags(termios_oflags, oflags_clean, 0);
> +    if (oflags & TARGET_NLDLY) {
> +        print_enums(termios_oflags_NLDLY, oflags & TARGET_NLDLY, 0);
> +    }
> +    if (oflags & TARGET_CRDLY) {
> +        print_enums(termios_oflags_CRDLY, oflags & TARGET_CRDLY, 0);
> +    }
> +    if (oflags & TARGET_TABDLY) {
> +        print_enums(termios_oflags_TABDLY, oflags & TARGET_TABDLY, 0);
> +    }
> +    if (oflags & TARGET_BSDLY) {
> +        print_enums(termios_oflags_BSDLY, oflags & TARGET_BSDLY, 0);
> +    }
> +    if (oflags & TARGET_VTDLY) {
> +        print_enums(termios_oflags_VTDLY, oflags & TARGET_VTDLY, 0);
> +    }
> +    if (oflags & TARGET_FFDLY) {
> +        print_enums(termios_oflags_FFDLY, oflags & TARGET_FFDLY, 0);
> +    }
> +
> +    qemu_log("c_cflag = ");
> +    if (cflags & TARGET_CBAUD) {
> +        print_enums(termios_cflags_CBAUD, cflags & TARGET_CBAUD, 0);
> +    }
> +    if (cflags & TARGET_CSIZE) {
> +        print_enums(termios_cflags_CSIZE, cflags & TARGET_CSIZE, 0);
> +    }
> +    target_tcflag_t cflags_clean = cflags & ~(TARGET_CBAUD | TARGET_CSIZE);
> +    print_flags(termios_cflags, cflags_clean, 0);
> +
> +    qemu_log("c_lflag = ");
> +    print_flags(termios_lflags, lflags, 0);
> +
> +    qemu_log("c_cc = ");
> +    qemu_log("\"%s\",", target->c_cc);
> +
> +    qemu_log("c_line = ");
> +    print_raw_param("\'%c\'", target->c_line, 1);
> +
> +    qemu_log("}");
> +}
> +
>  #undef UNUSED
>  
>  #ifdef TARGET_NR_accept
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e815f14bac..6dc230cd0b 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5669,6 +5669,7 @@ static const StructEntry struct_termios_def = {
>      .convert = { host_to_target_termios, target_to_host_termios },
>      .size = { sizeof(struct target_termios), sizeof(struct host_termios) },
>      .align = { __alignof__(struct target_termios), __alignof__(struct 
> host_termios) },
> +    .print = print_termios,
>  };
>  
>  static bitmask_transtbl mmap_flags_tbl[] = {
> diff --git a/thunk.c b/thunk.c
> index c5d9719747..0718325d86 100644
> --- a/thunk.c
> +++ b/thunk.c
> @@ -404,19 +404,24 @@ const argtype *thunk_print(void *arg, const argtype 
> *type_ptr)
>              const int *arg_offsets;
>  
>              se = struct_entries + *type_ptr++;
> -            a = arg;
>  
> -            field_types = se->field_types;
> -            arg_offsets = se->field_offsets[0];
> +            if (se->print != NULL) {
> +                se->print(arg);
> +            } else {
> +                a = arg;
>  
> -            qemu_log("{");
> -            for (i = 0; i < se->nb_fields; i++) {
> -                if (i > 0) {
> -                    qemu_log(",");
> +                field_types = se->field_types;
> +                arg_offsets = se->field_offsets[0];
> +
> +                qemu_log("{");
> +                for (i = 0; i < se->nb_fields; i++) {
> +                    if (i > 0) {
> +                        qemu_log(",");
> +                    }
> +                    field_types = thunk_print(a + arg_offsets[i], 
> field_types);
>                  }
> -                field_types = thunk_print(a + arg_offsets[i], field_types);
> +                qemu_log("}");
>              }
> -            qemu_log("}");
>          }
>          break;
>      default:
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>