[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH] x86emul: drop and avoid use of BUG()
Generally it is not a good idea to use BUG() in emulator code. Even for internal flaws we're better off returning errors to callers, rather than crashing the system. Replace the sole remaining use and remove the test / fuzzing harness surrogate. Put in place a declaration pleasing the compiler when finding uses in Xen headers, while at the same time breaking the build (at linking time) in case an active reference would newly appear. Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> --- The #undef and override may be going a little too far, but I guess we can revisit this if and when it actually gets in the way. While BUG_ON() references BUG() and is hence covered, we may want to consider to #undef/declare that separately. That way the linker error would make clear which one it is that was referenced. I could use EXPECT() instead of kind of open-coding it, but I don't really like EXPECT(false) or variations thereof. --- a/tools/tests/x86_emulator/x86-emulate.h +++ b/tools/tests/x86_emulator/x86-emulate.h @@ -42,7 +42,6 @@ #include <xen-tools/common-macros.h> -#define BUG() abort() #define ASSERT assert #define ASSERT_UNREACHABLE() assert(!__LINE__) --- a/xen/arch/x86/x86_emulate/decode.c +++ b/xen/arch/x86/x86_emulate/decode.c @@ -1122,7 +1122,9 @@ int x86emul_decode(struct x86_emulate_st switch ( def_ad_bytes ) { default: - BUG(); /* Shouldn't be possible. */ + ASSERT_UNREACHABLE(); /* Shouldn't be possible. */ + return X86EMUL_UNHANDLEABLE; + case 2: if ( ctxt->regs->eflags & X86_EFLAGS_VM ) break; --- a/xen/arch/x86/x86_emulate/private.h +++ b/xen/arch/x86/x86_emulate/private.h @@ -15,6 +15,9 @@ # include <asm/x86-vendors.h> # include <asm/x86_emulate.h> +# undef BUG /* Make sure it's not used anywhere here. */ +void BUG(void); + # ifndef CONFIG_HVM # define X86EMUL_NO_FPU # define X86EMUL_NO_MMX
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |