Re: [PATCH v3 20/20] mcdstub: updated MAINTAINERS file and fully activated the mcdstub in the meson build system

[Alex Bennée]

Nicolas Eder <nicolas.eder@lauterbach.com> writes:

> ---
>  MAINTAINERS            | 11 +++++++++++
>  mcdstub/meson.build    | 15 +++++++++++++++
>  meson.build            |  1 +
>  target/arm/meson.build |  1 +
>  4 files changed, 28 insertions(+)
>  create mode 100644 mcdstub/meson.build
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cd8d6b140f..58decd218c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2908,6 +2908,17 @@ F: tests/tcg/multiarch/gdbstub/
>  F: scripts/feature_to_c.py
>  F: scripts/probe-gdb-support.py
>  
> +MCD stub
> +M: Nicolas Eder <nicolas.eder@lauterbach.com>
> +R: Alex Bennée <alex.bennee@linaro.org>
> +S: Maintained
> +F: mcdstub/*
> +F: include/mcdstub/*
> +F: include/qemu/debug.h
> +F: debug/debug-common.c
> +F: debug/debug-mcd.c
> +F: target/arm/mcdstub.c
> +
>  Memory API
>  M: Paolo Bonzini <pbonzini@redhat.com>
>  M: Peter Xu <peterx@redhat.com>
> diff --git a/mcdstub/meson.build b/mcdstub/meson.build
> new file mode 100644
> index 0000000000..1f9315f9f5
> --- /dev/null
> +++ b/mcdstub/meson.build
> @@ -0,0 +1,15 @@
> +# only system emulation is supported over mcd
> +mcd_system_ss = ss.source_set()
> +mcd_system_ss.add(files('mcdstub.c', '../debug/debug-mcd.c'))
> +mcd_system_ss = mcd_system_ss.apply(config_host, strict: false)
> +
> +libmcd_system = static_library('mcd_system',
> +                                mcd_system_ss.sources() + genh,
> +                                name_suffix: 'fa',
> +                                build_by_default: have_system)
> +
> +mcd_system = declare_dependency(link_whole: libmcd_system)
> +system_ss.add(mcd_system)
> +
> +# used for gdb (user/system) and mcd:
> +common_ss.add(files('../debug/debug-common.c'))
> diff --git a/meson.build b/meson.build
> index dcef8b1e79..6dfa49160c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3348,6 +3348,7 @@ subdir('crypto')
>  subdir('ui')
>  subdir('hw')
>  subdir('gdbstub')
> +subdir('mcdstub')

OK this is fine *if* you haven't already got code attempting to link to
bits of your stub. You can either deal with this by declaring stub
functions where the real symbol doesn't exist or having empty inlines
when the feature isn't enabled.

That said even here the build fails for me:

  FAILED: mcdstub/libmcd_system.fa.p/mcdstub.c.o 
  cc -m64 -mcx16 -Imcdstub/libmcd_system.fa.p -Imcdstub -I../../mcdstub -I. 
-Iqapi -Itrace -Iui -Iui/shader -I/usr/include/glib-2.0 
-I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall 
-Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -Wundef 
-Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls 
-Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security 
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs 
-Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 
-Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value 
-Wno-psabi -Wshadow=local -isystem /home/alex/lsrc/qemu.git/linux-headers 
-isystem linux-headers -iquote . -iquote /home/alex/lsrc/qemu.git -iquote 
/home/alex/lsrc/qemu.git/include -iquote 
/home/alex/lsrc/qemu.git/host/include/x86_64 -iquote 
/home/alex/lsrc/qemu.git/host/include/generic -iquote 
/home/alex/lsrc/qemu.git/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
-D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -MD -MQ 
mcdstub/libmcd_system.fa.p/mcdstub.c.o -MF 
mcdstub/libmcd_system.fa.p/mcdstub.c.o.d -o 
mcdstub/libmcd_system.fa.p/mcdstub.c.o -c ../../mcdstub/mcdstub.c
  ../../mcdstub/mcdstub.c: In function ‘mcdserver_start’:
  ../../mcdstub/mcdstub.c:286:25: error: ‘,wait=off,nodelay=on,server=on’ 
directive output may be truncated writing 30 bytes into a region of size 
between 1 and 128 [-Werror=format-truncation=]
    286 |                      "%s,wait=off,nodelay=on,server=on", device);
        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  In file included from /usr/include/stdio.h:906,
                   from /home/alex/lsrc/qemu.git/include/qemu/osdep.h:114,
                   from ../../mcdstub/mcdstub.c:5:
  In function ‘snprintf’,
      inlined from ‘mcdserver_start’ at ../../mcdstub/mcdstub.c:285:13:
  /usr/include/x86_64-linux-gnu/bits/stdio2.h:54:10: note: 
‘__builtin___snprintf_chk’ output between 31 and 158 bytes into a destination 
of size 128
     54 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     55 |                                    __glibc_objsize (__s), __fmt,
        |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     56 |                                    __va_arg_pack ());
        |                                    ~~~~~~~~~~~~~~~~~
  ../../mcdstub/mcdstub.c: In function ‘cmd_parse_params’:
  ../../mcdstub/mcdstub.c:716:13: error: ‘__builtin___strncpy_chk’ output 
truncated before terminating nul copying as many bytes from a string as its 
length [-Werror=stringop-truncation]
    716 |             strncpy(data_buffer, remaining_data, 
strlen(remaining_data));
        |             ^
  ../../mcdstub/mcdstub.c:716:13: note: length computed here
    716 |             strncpy(data_buffer, remaining_data, 
strlen(remaining_data));
        |             
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  In file included from /usr/include/glib-2.0/glib.h:33,
                   from /home/alex/lsrc/qemu.git/include/glib-compat.h:32,
                   from /home/alex/lsrc/qemu.git/include/qemu/osdep.h:161:
  /usr/include/glib-2.0/glib/garray.h: In function ‘parse_reg_xml’:
  /usr/include/glib-2.0/glib/garray.h:69:54: error: ‘reg_data’ may be used 
uninitialized [-Werror=maybe-uninitialized]
     69 | #define g_array_index(a,t,i)      (((t*) (void *) (a)->data) [(i)])
        |                                                      ^~
  ../../mcdstub/mcdstub.c:1081:13: note: ‘reg_data’ was declared here
   1081 |     GArray *reg_data;
        |             ^~~~~~~~
  ../../mcdstub/mcdstub.c:1136:28: error: ‘internal_id’ may be used 
uninitialized [-Werror=maybe-uninitialized]
   1136 |                 internal_id++;
        |                 ~~~~~~~~~~~^~
  ../../mcdstub/mcdstub.c:1074:14: note: ‘internal_id’ was declared here
   1074 |     uint32_t internal_id;
        |              ^~~~~~~~~~~
  cc1: all warnings being treated as errors


See also:

  https://gitlab.com/stsquad/qemu/-/pipelines/1089441465/failures

Please ensure next post that you can at least pass a "make all" of a
normally configured build and that each intermediate stage also builds.

>  
>  if enable_modules
>    libmodulecommon = static_library('module-common', files('module-common.c') 
> + genh, pic: true, c_args: '-DBUILD_DSO')
> diff --git a/target/arm/meson.build b/target/arm/meson.build
> index 5d04a8e94f..84fedbc6e1 100644
> --- a/target/arm/meson.build
> +++ b/target/arm/meson.build
> @@ -3,6 +3,7 @@ arm_ss.add(files(
>    'cpu.c',
>    'debug_helper.c',
>    'gdbstub.c',
> +  'mcdstub.c',
>    'helper.c',
>    'vfp_helper.c',
>  ))

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro