On Mon, 2021-03-01 at 12:17 +0100, Paolo Bonzini wrote: > If kvm_arch_remove_sw_breakpoint finds that a software breakpoint does not > have an INT3 instruction, it fails. This can happen if one sets a > software breakpoint in a kernel module and then reloads it. gdb then > thinks the breakpoint cannot be deleted and there is no way to add it > back. > > Suggested-by: Maxim Levitsky <mlevi...@redhat.com> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > target/i386/kvm/kvm.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index 0b5755e42b..c8d61daf68 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -4352,8 +4352,13 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct > kvm_sw_breakpoint *bp) > { > uint8_t int3; > > - if (cpu_memory_rw_debug(cs, bp->pc, &int3, 1, 0) || int3 != 0xcc || > - cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1)) { > + if (cpu_memory_rw_debug(cs, bp->pc, &int3, 1, 0)) { > + return -EINVAL; > + } > + if (int3 != 0xcc) { > + return 0; > + } > + if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 1, 1)) { > return -EINVAL; > } > return 0;
There still remains a philosopical question if kvm_arch_remove_sw_breakpoint should always return 0, since for the usual case of kernel debugging where a breakpoint is in unloaded module, the above will probably still fail as paging for this module is removed as well by the kernel. It is still better though so: Reviewed-by: Maxim Levitsky <mlevi...@redhat.com> Note that I managed to make lx-symbols to work in a very stable way with attached WIP patch I hacked on this Sunday. I will send a cleaned up version of it to upstream when I have time. Since I make gdb unload the symbols, it works even without this patch. Added Stefano Garzarella to CC as I know that he tried to make this work as well. https://lkml.org/lkml/2020/10/5/514 Best regards, Maxim Levitsky
commit 86f0d5a08a40528350589ed54126f06d4ac293a8 Author: Maxim Levitsky <mlevi...@redhat.com> Date: Sun Feb 28 23:52:08 2021 +0200 gdb: rework gdb debug script Signed-off-by: Maxim Levitsky <mlevi...@redhat.com> diff --git a/scripts/gdb/linux/symbols.py b/scripts/gdb/linux/symbols.py index 1be9763cf8bb..13f21afc2059 100644 --- a/scripts/gdb/linux/symbols.py +++ b/scripts/gdb/linux/symbols.py @@ -17,6 +17,24 @@ import re from linux import modules, utils +def save_state(): + breakpoints = [] + if hasattr(gdb, 'breakpoints') and not gdb.breakpoints() is None: + for bp in gdb.breakpoints(): + breakpoints.append({'breakpoint': bp, 'enabled': bp.enabled}) + bp.enabled = False + + show_pagination = gdb.execute("show pagination", to_string=True) + pagination = show_pagination.endswith("on.\n") + gdb.execute("set pagination off") + + return {"breakpoints":breakpoints, "show_pagination": show_pagination} + +def load_state(state): + for breakpoint in state["breakpoints"]: + breakpoint['breakpoint'].enabled = breakpoint['enabled'] + gdb.execute("set pagination %s" % ("on" if state["show_pagination"] else "off")) + if hasattr(gdb, 'Breakpoint'): class LoadModuleBreakpoint(gdb.Breakpoint): @@ -30,26 +48,38 @@ if hasattr(gdb, 'Breakpoint'): module_name = module['name'].string() cmd = self.gdb_command + # module already loaded, false alarm + if module_name in cmd.loaded_modules: + return False + # enforce update if object file is not found cmd.module_files_updated = False # Disable pagination while reporting symbol (re-)loading. # The console input is blocked in this context so that we would # get stuck waiting for the user to acknowledge paged output. - show_pagination = gdb.execute("show pagination", to_string=True) - pagination = show_pagination.endswith("on.\n") - gdb.execute("set pagination off") + state = save_state() + cmd.load_module_symbols(module) + load_state(state) + return False - if module_name in cmd.loaded_modules: - gdb.write("refreshing all symbols to reload module " - "'{0}'\n".format(module_name)) - cmd.load_all_symbols() - else: - cmd.load_module_symbols(module) + class UnLoadModuleBreakpoint(gdb.Breakpoint): + def __init__(self, spec, gdb_command): + super(UnLoadModuleBreakpoint, self).__init__(spec, internal=True) + self.silent = True + self.gdb_command = gdb_command - # restore pagination state - gdb.execute("set pagination %s" % ("on" if pagination else "off")) + def stop(self): + module = gdb.parse_and_eval("mod") + module_name = module['name'].string() + cmd = self.gdb_command + if not module_name in cmd.loaded_modules: + return False + + state = save_state() + cmd.unload_module_symbols(module) + load_state(state) return False @@ -64,8 +94,9 @@ lx-symbols command.""" module_paths = [] module_files = [] module_files_updated = False - loaded_modules = [] + loaded_modules = {} breakpoint = None + ubreakpoint = None def __init__(self): super(LxSymbols, self).__init__("lx-symbols", gdb.COMMAND_FILES, @@ -129,21 +160,32 @@ lx-symbols command.""" filename=module_file, addr=module_addr, sections=self._section_arguments(module)) + gdb.execute(cmdline, to_string=True) - if module_name not in self.loaded_modules: - self.loaded_modules.append(module_name) + self.loaded_modules[module_name] = {"module_file": module_file, + "module_addr": module_addr} else: gdb.write("no module object found for '{0}'\n".format(module_name)) + def unload_module_symbols(self, module): + module_name = module['name'].string() + + module_file = self.loaded_modules[module_name]["module_file"] + module_addr = self.loaded_modules[module_name]["module_addr"] + + gdb.write("unloading @{addr}: {filename}\n".format( + addr=module_addr, filename=module_file)) + cmdline = "remove-symbol-file {filename}".format( + filename=module_file) + + gdb.execute(cmdline, to_string=True) + del self.loaded_modules[module_name] + + def load_all_symbols(self): gdb.write("loading vmlinux\n") - # Dropping symbols will disable all breakpoints. So save their states - # and restore them afterward. - saved_states = [] - if hasattr(gdb, 'breakpoints') and not gdb.breakpoints() is None: - for bp in gdb.breakpoints(): - saved_states.append({'breakpoint': bp, 'enabled': bp.enabled}) + state = save_state() # drop all current symbols and reload vmlinux orig_vmlinux = 'vmlinux' @@ -153,15 +195,14 @@ lx-symbols command.""" gdb.execute("symbol-file", to_string=True) gdb.execute("symbol-file {0}".format(orig_vmlinux)) - self.loaded_modules = [] + self.loaded_modules = {} module_list = modules.module_list() if not module_list: gdb.write("no modules found\n") else: [self.load_module_symbols(module) for module in module_list] - for saved_state in saved_states: - saved_state['breakpoint'].enabled = saved_state['enabled'] + load_state(state) def invoke(self, arg, from_tty): self.module_paths = [os.path.expanduser(p) for p in arg.split()] @@ -177,8 +218,13 @@ lx-symbols command.""" if self.breakpoint is not None: self.breakpoint.delete() self.breakpoint = None - self.breakpoint = LoadModuleBreakpoint( - "kernel/module.c:do_init_module", self) + self.breakpoint = LoadModuleBreakpoint("kernel/module.c:do_init_module", self) + + if self.ubreakpoint is not None: + self.ubreakpoint.delete() + self.ubreakpoint = None + self.ubreakpoint = UnLoadModuleBreakpoint("kernel/module.c:free_module", self) + else: gdb.write("Note: symbol update on module loading not supported " "with this gdb version\n")