Re: [Qemu-devel] [PATCH v1 2/2] Revert "error: Don't use error_report() for assertion msgs."

[Peter Maydell]

On 15 January 2014 11:06, Peter Crosthwaite
<address@hidden> wrote:
> This reverts commit d32934c84c72f57e78d430c22974677b7bcabe5d.
>
> The original implementation before this patch makes abortive error
> messages much more friendly. The underlying bug that required this
> change is now fixed. Revert.

Unfortunately 'make check' still doesn't work on MacOS:

cc -m64 -DOS_OBJECT_USE_OBJC=0 -arch x86_64 -D_GNU_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes
-fno-strict-aliasing  -Wno-initializer-overrides -Wendif-labels
-Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition
-Wtype-limits -fstack-protector-all -I/sw/include
-I/sw/include/libpng15   -I/sw/include   -I/opt/X11/include/pixman-1
-I/Users/pm215/src/qemu/dtc/libfdt -I/sw/include/glib-2.0
-I/sw/lib/glib-2.0/include -I/Users/pm215/src/qemu/tests -O2
-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g  -m64 -framework
CoreFoundation -framework IOKit -arch x86_64 -g  -o tests/check-qjson
tests/check-qjson.o libqemuutil.a libqemustub.a  -L/sw/lib
-lgthread-2.0 -lglib-2.0 -lintl  -lz -L/sw/lib -lssh2 -L/sw/lib -lcurl
Undefined symbols for architecture x86_64:
  "_cur_mon", referenced from:
      _error_vprintf in libqemuutil.a(qemu-error.o)
      _error_printf in libqemuutil.a(qemu-error.o)
      _error_printf_unless_qmp in libqemuutil.a(qemu-error.o)
      _error_print_loc in libqemuutil.a(qemu-error.o)
      _error_report in libqemuutil.a(qemu-error.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [tests/check-qjson] Error 1


Curiously, if you edit the cc command line so it says not just
"libqemustub.a" but "libqemustub.a stubs/mon-set-error.o" then
it links successfully. nm says that that .o file is in the .a:

libqemustub.a(mon-set-error.o):
0000000000000a68 s EH_frame0
                 U ___stack_chk_fail
                 U ___stack_chk_guard
0000000000000008 C _cur_mon
0000000000000000 T _monitor_set_error
0000000000000a80 S _monitor_set_error.eh

This appears to be a deficiency in the MacOSX linker and/or
executable format:

http://stackoverflow.com/questions/19398742/os-x-linker-unable-to-find-symbols-from-a-c-file-which-only-contains-variables

To fix this we need to:
 * make the cur_mon in the stub-file not common (eg by initializing it)
 * make the cur_mon in monitor.c not common (because otherwise
   the linker will prefer to pull in the version from the stub file, which
   then causes a clash between the stub monitor_set_error() function
   and the real one)

Which is kind of ugly.

The other approach would be to ensure that the monitor-related
stubs are combined into fewer stub .c files, such that any binary
that needs cur_mon will pull in the stub .o file because it needed
some function from it anyway.

Related questions:

why does util/qemu-error.c guard its use of cur_mon only by
"if (cur_mon)" whereas qobject/qerror.c uses "if (monitor_cur_is_qmp())"?

why, given that qerror.c has made a specific decision not to
send its output to the monitor, is it ok for error_report and
error_vprintf to override it and send the output to the monitor
anyway? if error_vprintf is correct should we remove the
higher level check?

thanks
-- PMM