| Message ID | 20210122201113.63788-6-j@getutm.app (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iOS and Apple Silicon host support | expand |
On Fri, 22 Jan 2021 at 20:13, Joelle van Dyne <j@getutm.app> wrote: > > Build without error on hosts without a working system(). An assertion > will trigger if system() is called. > > Signed-off-by: Joelle van Dyne <j@getutm.app> configure | 19 +++++++++++++++++++ Can we do the "does system() exist?" check in meson.build ? Untested, but looking at the existing check for "does gettid() exist?" it should be two lines: has_system = cc.has_function('system') and then later: config_host_data.set('HAVE_SYSTEM_FUNCTION', has_system) > +/** > + * Platforms which do not support system() gets an assertion failure. > + */ > +#ifndef HAVE_SYSTEM_FUNCTION > +#define system platform_does_not_support_system > +static inline int platform_does_not_support_system(const char *command) > +{ > + assert(0); > +} > +#endif /* !HAVE_SYSTEM_FUNCTION */ I think we should make this return an error code rather than assert: errno = ENOSYS; return -1; In particular, the arm, m68k and nios2 semihosting ABIs presented to the guest include 'SYSTEM' semihosting calls which we implement as "call system() with the string the guest hands us". On a platform without a system() function we want to return an error to the guest there, not assert. The other possible approach would be to find all the places which want to call system() and add suitable ifdeffery to handle platforms without system: * a win32-specific part of the guest-agent (no action needed) * net/slirp.c (already handled by the smbd patch in this series) * code in tests/ (5 instances) * the 3 semihosting uses But I think providing an always-fails system() is fine. thanks -- PMM
On Fri, 22 Jan 2021 at 23:12, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Fri, 22 Jan 2021 at 20:13, Joelle van Dyne <j@getutm.app> wrote: > > > > Build without error on hosts without a working system(). An assertion > > will trigger if system() is called. > > > > Signed-off-by: Joelle van Dyne <j@getutm.app> > > configure | 19 +++++++++++++++++++ > > Can we do the "does system() exist?" check in meson.build ? > Untested, but looking at the existing check for "does gettid() exist?" > it should be two lines: > > has_system = cc.has_function('system') > > and then later: > > config_host_data.set('HAVE_SYSTEM_FUNCTION', has_system) ...looking at how we do the HAVE_FOO_H settings, I think we can just collapse this into one line: config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system')) thanks -- PMM
Unfortunately, this doesn't work for iOS, which defines system() but throws a compile time error if you try to call it. -j On Fri, Jan 22, 2021 at 3:17 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Fri, 22 Jan 2021 at 23:12, Peter Maydell <peter.maydell@linaro.org> wrote: > > > > On Fri, 22 Jan 2021 at 20:13, Joelle van Dyne <j@getutm.app> wrote: > > > > > > Build without error on hosts without a working system(). An assertion > > > will trigger if system() is called. > > > > > > Signed-off-by: Joelle van Dyne <j@getutm.app> > > > > configure | 19 +++++++++++++++++++ > > > > Can we do the "does system() exist?" check in meson.build ? > > Untested, but looking at the existing check for "does gettid() exist?" > > it should be two lines: > > > > has_system = cc.has_function('system') > > > > and then later: > > > > config_host_data.set('HAVE_SYSTEM_FUNCTION', has_system) > > ...looking at how we do the HAVE_FOO_H settings, I think we > can just collapse this into one line: > > config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system')) > > thanks > -- PMM
On Sat, 23 Jan 2021 at 03:18, Joelle van Dyne <j@getutm.app> wrote: On Fri, Jan 22, 2021 at 3:17 PM Peter Maydell <peter.maydell@linaro.org> wrote: >> Can we do the "does system() exist?" check in meson.build ? >> config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system')) > Unfortunately, this doesn't work for iOS, which defines system() but > throws a compile time error if you try to call it. That's odd -- as far as I can tell the meson implementation of has_function() does what I expected it to do, ie "try to compile and link a little program that uses the function, and see if it successfully links": https://github.com/mesonbuild/meson/blob/39ede12aa5b27376341df85bc9ec254913f044bd/mesonbuild/compilers/mixins/clike.py#L791 There's some initial cleverness there too, so I guess some part of that must be what's tripping us up. In any case, I think we should be doing new checks in meson.build, not configure. Paolo, what's the right way to do a meson "really compile this program and check it built" test? thanks -- PMM
On 23/01/21 14:45, Peter Maydell wrote: > On Sat, 23 Jan 2021 at 03:18, Joelle van Dyne <j@getutm.app> wrote: > On Fri, Jan 22, 2021 at 3:17 PM Peter Maydell <peter.maydell@linaro.org> wrote: >>> Can we do the "does system() exist?" check in meson.build ? > >>> config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system')) > >> Unfortunately, this doesn't work for iOS, which defines system() but >> throws a compile time error if you try to call it. > > That's odd -- as far as I can tell the meson implementation > of has_function() does what I expected it to do, ie > "try to compile and link a little program that uses the > function, and see if it successfully links": > https://github.com/mesonbuild/meson/blob/39ede12aa5b27376341df85bc9ec254913f044bd/mesonbuild/compilers/mixins/clike.py#L791 > There's some initial cleverness there too, so I guess some > part of that must be what's tripping us up. > > In any case, I think we should be doing new checks in > meson.build, not configure. Paolo, what's the right > way to do a meson "really compile this program and > check it built" test? One possibility is that you have to specify the #include in the "prefix" argument of cc.has_function for the test to behave as the QEMU code? If cc.has_function doesn't work, there's cc.compiles() and cc.links(). Paolo
Here's how meson does cc.has_function https://github.com/mesonbuild/meson/blob/master/mesonbuild/compilers/mixins/clike.py#L761 Since the compiler error comes from the header file with __attribute__((availability(ios,unavailable))) The meson check will always pass. cc.compiles should work though. Is there a reason why it's not used instead of all the compile_prog checks in ./configure ? -j On Mon, Jan 25, 2021 at 12:04 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 23/01/21 14:45, Peter Maydell wrote: > > On Sat, 23 Jan 2021 at 03:18, Joelle van Dyne <j@getutm.app> wrote: > > On Fri, Jan 22, 2021 at 3:17 PM Peter Maydell <peter.maydell@linaro.org> wrote: > >>> Can we do the "does system() exist?" check in meson.build ? > > > >>> config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system')) > > > >> Unfortunately, this doesn't work for iOS, which defines system() but > >> throws a compile time error if you try to call it. > > > > That's odd -- as far as I can tell the meson implementation > > of has_function() does what I expected it to do, ie > > "try to compile and link a little program that uses the > > function, and see if it successfully links": > > https://github.com/mesonbuild/meson/blob/39ede12aa5b27376341df85bc9ec254913f044bd/mesonbuild/compilers/mixins/clike.py#L791 > > There's some initial cleverness there too, so I guess some > > part of that must be what's tripping us up. > > > > In any case, I think we should be doing new checks in > > meson.build, not configure. Paolo, what's the right > > way to do a meson "really compile this program and > > check it built" test? > > One possibility is that you have to specify the #include in the "prefix" > argument of cc.has_function for the test to behave as the QEMU code? > > If cc.has_function doesn't work, there's cc.compiles() and cc.links(). > > Paolo >
On 26/01/21 01:12, Joelle van Dyne wrote: > Here's how meson does cc.has_function > > https://github.com/mesonbuild/meson/blob/master/mesonbuild/compilers/mixins/clike.py#L761 > > Since the compiler error comes from the header file with > > __attribute__((availability(ios,unavailable))) > > The meson check will always pass. > > cc.compiles should work though. Is there a reason why it's not used > instead of all the compile_prog checks in ./configure ? Just because the Meson build system is only a few months old. Alternatively: # If we have any includes in the prefix supplied by the user, assume # that the user wants us to use the symbol prototype defined in those # includes. If not, then try to do the Autoconf-style check with # a dummy prototype definition of our own. so adding a "prefix: '#include <stdlib.h>'" to cc.has_function should work too. Paolo
On Tue, 26 Jan 2021 at 00:12, Joelle van Dyne <j@getutm.app> wrote: > cc.compiles should work though. Is there a reason why it's not used > instead of all the compile_prog checks in ./configure ? As Paolo says, that's because our transition to Meson is pretty recent. We put in the minimum necessary to move all our old makefiles over to meson.build, but left the bulk of the old configure script in place initially. Since then we've been gradually converting tests in configure to meson.build tests, but configure's a big script and it's taking us a while. In the meantime we're preferring new tests to be done in meson. thanks -- PMM
diff --git a/configure b/configure index 92da27846e..82ce28c660 100755 --- a/configure +++ b/configure @@ -5318,6 +5318,21 @@ else sys_disk_h=no fi +########################################## +# check for system() + +have_system_function=no +cat > $TMPC << EOF +#include <stdlib.h> +int main(void) { + return system(""); +} +EOF +if compile_prog "" "" ; then + have_system_function=yes +fi + + ########################################## # End of CC checks # After here, no more $cc or $ld runs @@ -6222,6 +6237,10 @@ if test "$secret_keyring" = "yes" ; then echo "CONFIG_SECRET_KEYRING=y" >> $config_host_mak fi +if test "$have_system_function" = "yes" ; then + echo "HAVE_SYSTEM_FUNCTION=y" >> $config_host_mak +fi + echo "ROMS=$roms" >> $config_host_mak echo "MAKE=$make" >> $config_host_mak echo "PYTHON=$python" >> $config_host_mak diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index a434382c58..73346c4349 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -682,4 +682,15 @@ char *qemu_get_host_name(Error **errp); */ size_t qemu_get_host_physmem(void); +/** + * Platforms which do not support system() gets an assertion failure. + */ +#ifndef HAVE_SYSTEM_FUNCTION +#define system platform_does_not_support_system +static inline int platform_does_not_support_system(const char *command) +{ + assert(0); +} +#endif /* !HAVE_SYSTEM_FUNCTION */ + #endif
Build without error on hosts without a working system(). An assertion will trigger if system() is called. Signed-off-by: Joelle van Dyne <j@getutm.app> --- configure | 19 +++++++++++++++++++ include/qemu/osdep.h | 11 +++++++++++ 2 files changed, 30 insertions(+)