| Message ID | 20191030144926.11873-2-alxndr@bu.edu |
|---|---|
| State | New |
| Headers | show |
| Series | Add virtual device fuzzing support | expand |
On Wed, Oct 30, 2019 at 02:49:48PM +0000, Oleinik, Alexander wrote: >From: Alexander Oleinik <alxndr@bu.edu> > >A program might rely on functions implemented in vl.c, but implement its >own main(). By placing main into a separate source file, there are no >complaints about duplicate main()s when linking against vl.o. For >example, the virtual-device fuzzer uses a main() provided by libfuzzer, >and needs to perform some initialization before running the softmmu >initialization. Now, main simply calls three vl.c functions which >handle the guest initialization, main loop and cleanup. > >Signed-off-by: Alexander Oleinik <alxndr@bu.edu> >--- > Makefile | 1 + > Makefile.objs | 2 ++ > include/sysemu/sysemu.h | 4 ++++ > main.c | 52 +++++++++++++++++++++++++++++++++++++++++ > vl.c | 36 +++++++--------------------- > 5 files changed, 68 insertions(+), 27 deletions(-) > create mode 100644 main.c > >diff --git a/Makefile b/Makefile >index 0e994a275d..d2b2ecd3c4 100644 >--- a/Makefile >+++ b/Makefile >@@ -474,6 +474,7 @@ $(SOFTMMU_ALL_RULES): $(crypto-obj-y) > $(SOFTMMU_ALL_RULES): $(io-obj-y) > $(SOFTMMU_ALL_RULES): config-all-devices.mak > $(SOFTMMU_ALL_RULES): $(edk2-decompressed) >+$(SOFTMMU_ALL_RULES): $(softmmu-main-y) > > .PHONY: $(TARGET_DIRS_RULES) > # The $(TARGET_DIRS_RULES) are of the form SUBDIR/GOAL, so that >diff --git a/Makefile.objs b/Makefile.objs >index 11ba1a36bd..9ff9b0c6f9 100644 >--- a/Makefile.objs >+++ b/Makefile.objs >@@ -86,6 +86,8 @@ common-obj-$(CONFIG_FDT) += device_tree.o > # qapi > > common-obj-y += qapi/ >+ >+softmmu-main-y = main.o > endif > > ####################################################################### >diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h >index 44f18eb739..03f9838b81 100644 >--- a/include/sysemu/sysemu.h >+++ b/include/sysemu/sysemu.h >@@ -114,6 +114,10 @@ QemuOpts *qemu_get_machine_opts(void); > > bool defaults_enabled(void); > >+void main_loop(void); >+void qemu_init(int argc, char **argv, char **envp); >+void qemu_cleanup(void); >+ > extern QemuOptsList qemu_legacy_drive_opts; > extern QemuOptsList qemu_common_drive_opts; > extern QemuOptsList qemu_drive_opts; >diff --git a/main.c b/main.c >new file mode 100644 >index 0000000000..ecd6389424 >--- /dev/null >+++ b/main.c >@@ -0,0 +1,52 @@ >+/* >+ * QEMU System Emulator >+ * >+ * Copyright (c) 2003-2008 Fabrice Bellard >+ * >+ * Permission is hereby granted, free of charge, to any person obtaining a copy >+ * of this software and associated documentation files (the "Software"), to deal >+ * in the Software without restriction, including without limitation the rights >+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell >+ * copies of the Software, and to permit persons to whom the Software is >+ * furnished to do so, subject to the following conditions: >+ * >+ * The above copyright notice and this permission notice shall be included in >+ * all copies or substantial portions of the Software. >+ * >+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR >+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER >+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, >+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN >+ * THE SOFTWARE. >+ */ >+ >+#include "qemu/osdep.h" >+#include "sysemu/sysemu.h" >+ >+#ifdef CONFIG_SDL >+#if defined(__APPLE__) || defined(main) >+#include <SDL.h> >+int main(int argc, char **argv) >+{ >+ return qemu_main(argc, argv, NULL); >+} >+#undef main >+#define main qemu_main This /looks/ wrong, you're defining a function main(), and then immediately #undef and #define main again. Maybe this could be written differently, or add a comment here as to why you need to do this. >+#endif >+#endif /* CONFIG_SDL */ >+ >+#ifdef CONFIG_COCOA >+#undef main >+#define main qemu_main >+#endif /* CONFIG_COCOA */ I don't really know the combinations that might exist, but it looks like if CONFIG_SDL is not defined, then we're redefining main() to be qemi_main() - so what main() function will actually be used here? Thanks, Darren. >+ >+int main(int argc, char **argv, char **envp) >+{ >+ qemu_init(argc, argv, envp); >+ main_loop(); >+ qemu_cleanup(); >+ >+ return 0; >+} >diff --git a/vl.c b/vl.c >index c389d24b2c..472f09e12a 100644 >--- a/vl.c >+++ b/vl.c >@@ -36,25 +36,6 @@ > #include "sysemu/seccomp.h" > #include "sysemu/tcg.h" > >-#ifdef CONFIG_SDL >-#if defined(__APPLE__) || defined(main) >-#include <SDL.h> >-int qemu_main(int argc, char **argv, char **envp); >-int main(int argc, char **argv) >-{ >- return qemu_main(argc, argv, NULL); >-} >-#undef main >-#define main qemu_main >-#endif >-#endif /* CONFIG_SDL */ >- >-#ifdef CONFIG_COCOA >-#undef main >-#define main qemu_main >-#endif /* CONFIG_COCOA */ >- >- > #include "qemu/error-report.h" > #include "qemu/sockets.h" > #include "sysemu/accel.h" >@@ -1797,7 +1778,7 @@ static bool main_loop_should_exit(void) > return false; > } > >-static void main_loop(void) >+void main_loop(void) > { > #ifdef CONFIG_PROFILER > int64_t ti; >@@ -2824,7 +2805,7 @@ static void user_register_global_props(void) > global_init_func, NULL, NULL); > } > >-int main(int argc, char **argv, char **envp) >+void qemu_init(int argc, char **argv, char **envp) > { > int i; > int snapshot, linux_boot; >@@ -3404,7 +3385,7 @@ int main(int argc, char **argv, char **envp) > case QEMU_OPTION_watchdog: > if (watchdog) { > error_report("only one watchdog option may be given"); >- return 1; >+ exit(1); > } > watchdog = optarg; > break; >@@ -4440,7 +4421,7 @@ int main(int argc, char **argv, char **envp) > if (vmstate_dump_file) { > /* dump and exit */ > dump_vmstate_json_to_file(vmstate_dump_file); >- return 0; >+ exit(0); > } > > if (incoming) { >@@ -4457,8 +4438,11 @@ int main(int argc, char **argv, char **envp) > accel_setup_post(current_machine); > os_setup_post(); > >- main_loop(); >+ return; >+} > >+void qemu_cleanup(void) >+{ > gdbserver_cleanup(); > > /* >@@ -4495,6 +4479,4 @@ int main(int argc, char **argv, char **envp) > qemu_chr_cleanup(); > user_creatable_cleanup(); > /* TODO: unref root container, check all devices are ok */ >- >- return 0; > } >-- >2.23.0 > >
On Wed, Oct 30, 2019 at 02:49:48PM +0000, Oleinik, Alexander wrote: > diff --git a/main.c b/main.c > new file mode 100644 > index 0000000000..ecd6389424 > --- /dev/null > +++ b/main.c > @@ -0,0 +1,52 @@ > +/* > + * QEMU System Emulator > + * > + * Copyright (c) 2003-2008 Fabrice Bellard > + * > + * Permission is hereby granted, free of charge, to any person obtaining a copy > + * of this software and associated documentation files (the "Software"), to deal > + * in the Software without restriction, including without limitation the rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. > + */ > + > +#include "qemu/osdep.h" Perhaps this should be added: #include "qemu-common.h" It has: /* main function, renamed */ #if defined(CONFIG_COCOA) int qemu_main(int argc, char **argv, char **envp); #endif This way the compiler can check prototypes.
On 11/5/19 11:41 AM, Darren Kenny wrote: > On Wed, Oct 30, 2019 at 02:49:48PM +0000, Oleinik, Alexander wrote: >> From: Alexander Oleinik <alxndr@bu.edu> >> >> A program might rely on functions implemented in vl.c, but implement its >> own main(). By placing main into a separate source file, there are no >> complaints about duplicate main()s when linking against vl.o. For >> example, the virtual-device fuzzer uses a main() provided by libfuzzer, >> and needs to perform some initialization before running the softmmu >> initialization. Now, main simply calls three vl.c functions which >> handle the guest initialization, main loop and cleanup. >> >> Signed-off-by: Alexander Oleinik <alxndr@bu.edu> >> --- >> Makefile | 1 + >> Makefile.objs | 2 ++ >> include/sysemu/sysemu.h | 4 ++++ >> main.c | 52 +++++++++++++++++++++++++++++++++++++++++ >> vl.c | 36 +++++++--------------------- >> 5 files changed, 68 insertions(+), 27 deletions(-) >> create mode 100644 main.c >> >> diff --git a/Makefile b/Makefile >> index 0e994a275d..d2b2ecd3c4 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -474,6 +474,7 @@ $(SOFTMMU_ALL_RULES): $(crypto-obj-y) >> $(SOFTMMU_ALL_RULES): $(io-obj-y) >> $(SOFTMMU_ALL_RULES): config-all-devices.mak >> $(SOFTMMU_ALL_RULES): $(edk2-decompressed) >> +$(SOFTMMU_ALL_RULES): $(softmmu-main-y) >> >> .PHONY: $(TARGET_DIRS_RULES) >> # The $(TARGET_DIRS_RULES) are of the form SUBDIR/GOAL, so that >> diff --git a/Makefile.objs b/Makefile.objs >> index 11ba1a36bd..9ff9b0c6f9 100644 >> --- a/Makefile.objs >> +++ b/Makefile.objs >> @@ -86,6 +86,8 @@ common-obj-$(CONFIG_FDT) += device_tree.o >> # qapi >> >> common-obj-y += qapi/ >> + >> +softmmu-main-y = main.o >> endif >> >> ####################################################################### >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h >> index 44f18eb739..03f9838b81 100644 >> --- a/include/sysemu/sysemu.h >> +++ b/include/sysemu/sysemu.h >> @@ -114,6 +114,10 @@ QemuOpts *qemu_get_machine_opts(void); >> >> bool defaults_enabled(void); >> >> +void main_loop(void); >> +void qemu_init(int argc, char **argv, char **envp); >> +void qemu_cleanup(void); >> + >> extern QemuOptsList qemu_legacy_drive_opts; >> extern QemuOptsList qemu_common_drive_opts; >> extern QemuOptsList qemu_drive_opts; >> diff --git a/main.c b/main.c >> new file mode 100644 >> index 0000000000..ecd6389424 >> --- /dev/null >> +++ b/main.c >> @@ -0,0 +1,52 @@ >> +/* >> + * QEMU System Emulator >> + * >> + * Copyright (c) 2003-2008 Fabrice Bellard >> + * >> + * Permission is hereby granted, free of charge, to any person >> obtaining a copy >> + * of this software and associated documentation files (the >> "Software"), to deal >> + * in the Software without restriction, including without limitation >> the rights >> + * to use, copy, modify, merge, publish, distribute, sublicense, >> and/or sell >> + * copies of the Software, and to permit persons to whom the Software is >> + * furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be >> included in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT >> SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES >> OR OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >> ARISING FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER >> DEALINGS IN >> + * THE SOFTWARE. >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "sysemu/sysemu.h" >> + >> +#ifdef CONFIG_SDL >> +#if defined(__APPLE__) || defined(main) >> +#include <SDL.h> >> +int main(int argc, char **argv) >> +{ >> + return qemu_main(argc, argv, NULL); >> +} >> +#undef main >> +#define main qemu_main > > This /looks/ wrong, you're defining a function main(), and then > immediately #undef and #define main again. > > Maybe this could be written differently, or add a comment here as to > why you need to do this. > >> +#endif >> +#endif /* CONFIG_SDL */ >> + >> +#ifdef CONFIG_COCOA >> +#undef main >> +#define main qemu_main >> +#endif /* CONFIG_COCOA */ > > I don't really know the combinations that might exist, but it looks > like if CONFIG_SDL is not defined, then we're redefining main() to be > qemi_main() - so what main() function will actually be used here? I tried to copy this straight from vl.c. It seems that this was originally added for similar reasons that I added this patch - similarly to libfuzzer, SDL has its own main function, and I'm guessing its similar for cocoa. With some preprocessor flags, the result looks like: int SDL_main(int argc, char **argv) { return qemu_main(argc, argv, ((void *)0) ); } int qemu_main(int argc, char **argv, char **envp) { qemu_init(argc, argv, envp); main_loop(); qemu_cleanup(); return 0; } So it looks like this is there since SDL expects main to have two args. Maybe this is something that can be solved by adding separate main-sdl.c and main-cocoa.c files and tweaks to the build system, which should be possible now that qemu_init is separate from main(). -Alex > Thanks, > > Darren. > >> + >> +int main(int argc, char **argv, char **envp) >> +{ >> + qemu_init(argc, argv, envp); >> + main_loop(); >> + qemu_cleanup(); >> + >> + return 0; >> +} >> diff --git a/vl.c b/vl.c >> index c389d24b2c..472f09e12a 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -36,25 +36,6 @@ >> #include "sysemu/seccomp.h" >> #include "sysemu/tcg.h" >> >> -#ifdef CONFIG_SDL >> -#if defined(__APPLE__) || defined(main) >> -#include <SDL.h> >> -int qemu_main(int argc, char **argv, char **envp); >> -int main(int argc, char **argv) >> -{ >> - return qemu_main(argc, argv, NULL); >> -} >> -#undef main >> -#define main qemu_main >> -#endif >> -#endif /* CONFIG_SDL */ >> - >> -#ifdef CONFIG_COCOA >> -#undef main >> -#define main qemu_main >> -#endif /* CONFIG_COCOA */ >> - >> - >> #include "qemu/error-report.h" >> #include "qemu/sockets.h" >> #include "sysemu/accel.h" >> @@ -1797,7 +1778,7 @@ static bool main_loop_should_exit(void) >> return false; >> } >> >> -static void main_loop(void) >> +void main_loop(void) >> { >> #ifdef CONFIG_PROFILER >> int64_t ti; >> @@ -2824,7 +2805,7 @@ static void user_register_global_props(void) >> global_init_func, NULL, NULL); >> } >> >> -int main(int argc, char **argv, char **envp) >> +void qemu_init(int argc, char **argv, char **envp) >> { >> int i; >> int snapshot, linux_boot; >> @@ -3404,7 +3385,7 @@ int main(int argc, char **argv, char **envp) >> case QEMU_OPTION_watchdog: >> if (watchdog) { >> error_report("only one watchdog option may be >> given"); >> - return 1; >> + exit(1); >> } >> watchdog = optarg; >> break; >> @@ -4440,7 +4421,7 @@ int main(int argc, char **argv, char **envp) >> if (vmstate_dump_file) { >> /* dump and exit */ >> dump_vmstate_json_to_file(vmstate_dump_file); >> - return 0; >> + exit(0); >> } >> >> if (incoming) { >> @@ -4457,8 +4438,11 @@ int main(int argc, char **argv, char **envp) >> accel_setup_post(current_machine); >> os_setup_post(); >> >> - main_loop(); >> + return; >> +} >> >> +void qemu_cleanup(void) >> +{ >> gdbserver_cleanup(); >> >> /* >> @@ -4495,6 +4479,4 @@ int main(int argc, char **argv, char **envp) >> qemu_chr_cleanup(); >> user_creatable_cleanup(); >> /* TODO: unref root container, check all devices are ok */ >> - >> - return 0; >> } >> -- >> 2.23.0 >> >>
diff --git a/Makefile b/Makefile index 0e994a275d..d2b2ecd3c4 100644 --- a/Makefile +++ b/Makefile @@ -474,6 +474,7 @@ $(SOFTMMU_ALL_RULES): $(crypto-obj-y) $(SOFTMMU_ALL_RULES): $(io-obj-y) $(SOFTMMU_ALL_RULES): config-all-devices.mak $(SOFTMMU_ALL_RULES): $(edk2-decompressed) +$(SOFTMMU_ALL_RULES): $(softmmu-main-y) .PHONY: $(TARGET_DIRS_RULES) # The $(TARGET_DIRS_RULES) are of the form SUBDIR/GOAL, so that diff --git a/Makefile.objs b/Makefile.objs index 11ba1a36bd..9ff9b0c6f9 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -86,6 +86,8 @@ common-obj-$(CONFIG_FDT) += device_tree.o # qapi common-obj-y += qapi/ + +softmmu-main-y = main.o endif ####################################################################### diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 44f18eb739..03f9838b81 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -114,6 +114,10 @@ QemuOpts *qemu_get_machine_opts(void); bool defaults_enabled(void); +void main_loop(void); +void qemu_init(int argc, char **argv, char **envp); +void qemu_cleanup(void); + extern QemuOptsList qemu_legacy_drive_opts; extern QemuOptsList qemu_common_drive_opts; extern QemuOptsList qemu_drive_opts; diff --git a/main.c b/main.c new file mode 100644 index 0000000000..ecd6389424 --- /dev/null +++ b/main.c @@ -0,0 +1,52 @@ +/* + * QEMU System Emulator + * + * Copyright (c) 2003-2008 Fabrice Bellard + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "sysemu/sysemu.h" + +#ifdef CONFIG_SDL +#if defined(__APPLE__) || defined(main) +#include <SDL.h> +int main(int argc, char **argv) +{ + return qemu_main(argc, argv, NULL); +} +#undef main +#define main qemu_main +#endif +#endif /* CONFIG_SDL */ + +#ifdef CONFIG_COCOA +#undef main +#define main qemu_main +#endif /* CONFIG_COCOA */ + +int main(int argc, char **argv, char **envp) +{ + qemu_init(argc, argv, envp); + main_loop(); + qemu_cleanup(); + + return 0; +} diff --git a/vl.c b/vl.c index c389d24b2c..472f09e12a 100644 --- a/vl.c +++ b/vl.c @@ -36,25 +36,6 @@ #include "sysemu/seccomp.h" #include "sysemu/tcg.h" -#ifdef CONFIG_SDL -#if defined(__APPLE__) || defined(main) -#include <SDL.h> -int qemu_main(int argc, char **argv, char **envp); -int main(int argc, char **argv) -{ - return qemu_main(argc, argv, NULL); -} -#undef main -#define main qemu_main -#endif -#endif /* CONFIG_SDL */ - -#ifdef CONFIG_COCOA -#undef main -#define main qemu_main -#endif /* CONFIG_COCOA */ - - #include "qemu/error-report.h" #include "qemu/sockets.h" #include "sysemu/accel.h" @@ -1797,7 +1778,7 @@ static bool main_loop_should_exit(void) return false; } -static void main_loop(void) +void main_loop(void) { #ifdef CONFIG_PROFILER int64_t ti; @@ -2824,7 +2805,7 @@ static void user_register_global_props(void) global_init_func, NULL, NULL); } -int main(int argc, char **argv, char **envp) +void qemu_init(int argc, char **argv, char **envp) { int i; int snapshot, linux_boot; @@ -3404,7 +3385,7 @@ int main(int argc, char **argv, char **envp) case QEMU_OPTION_watchdog: if (watchdog) { error_report("only one watchdog option may be given"); - return 1; + exit(1); } watchdog = optarg; break; @@ -4440,7 +4421,7 @@ int main(int argc, char **argv, char **envp) if (vmstate_dump_file) { /* dump and exit */ dump_vmstate_json_to_file(vmstate_dump_file); - return 0; + exit(0); } if (incoming) { @@ -4457,8 +4438,11 @@ int main(int argc, char **argv, char **envp) accel_setup_post(current_machine); os_setup_post(); - main_loop(); + return; +} +void qemu_cleanup(void) +{ gdbserver_cleanup(); /* @@ -4495,6 +4479,4 @@ int main(int argc, char **argv, char **envp) qemu_chr_cleanup(); user_creatable_cleanup(); /* TODO: unref root container, check all devices are ok */ - - return 0; }