Re: [PATCH 2/2] Revert "crash_taget: fetch_registers support"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



cc Alexey, reverting this affects VMware folks.

On 2022/09/27 15:27, Lianbo Jiang wrote:
> This reverts commit 2f967fb5ebd737ce5eadba462df35935122e8865. John
> Pittman reports that it causes a regression issue on the old Vmware
> vmcore as below:
> 
>    crash> set scope dm_table_create
>    set: attempting to find/load "dm_mod" module debuginfo
>         MODULE       NAME                      BASE           SIZE  OBJECT FILE
>    ffffffffa0012dc0  dm_mod             ffffffffa0000000   102823  /home/2.6.32-754.35.1.el6.x86_64/kernel/drivers/md/dm-mod.ko.debug
>    scope: ffffffffa0006cf0 (dm_table_create)
>    crash> whatis struct dm_table
>    struct dm_table {
>        uint64_t features;
>        struct mapped_device *md;
>        unsigned int type;
>        unsigned int depth;
>        unsigned int counts[16];
>        sector_t *index[16];
>        unsigned int num_targets;
>        unsigned int num_allocated;
>        sector_t *highs;
>        struct dm_target *targets;
>        struct target_type *immutable_target_type;
>        unsigned int integrity_supported : 1;
>        unsigned int singleton : 1;
>        fmode_t mode;
>        struct list_head devices;
>        void (*event_fn)(void *);
>        void *event_context;
>        struct dm_md_mempools *mempools;
>        struct list_head target_callbacks;
>    }
>    SIZE: 312
>    crash> whatis _name_buckets        --->| After using the whatis _name_buckets command,
>    struct list_head _name_buckets[64];    |
>    crash> whatis struct dm_table      <---| it won't show the contents of the dm_table struct.
>    struct dm_table {
>        int undefined__;
>    }
>    SIZE: 312
>    crash>
> 
> With the patch, this issue disappeared.
At first glance, I did not find any part of the patch that can cause
the issue.  Do you have any detailed information?

Thanks,
Kazu

> 
> Signed-off-by: Lianbo Jiang <lijiang@xxxxxxxxxx>
> ---
>   crash_target.c  | 33 +-------------------------------
>   defs.h          | 51 -------------------------------------------------
>   gdb_interface.c | 19 +++++-------------
>   vmware_vmss.c   | 51 +------------------------------------------------
>   x86_64.c        | 16 ----------------
>   5 files changed, 7 insertions(+), 163 deletions(-)
> 
> diff --git a/crash_target.c b/crash_target.c
> index 455480679741..a123329019f5 100644
> --- a/crash_target.c
> +++ b/crash_target.c
> @@ -27,8 +27,6 @@ void crash_target_init (void);
>   
>   extern "C" int gdb_readmem_callback(unsigned long, void *, int, int);
>   extern "C" int crash_get_nr_cpus(void);
> -extern "C" int crash_get_cpu_reg (int cpu, int regno, const char *regname,
> -                                  int regsize, void *val);
>   
>   
>   /* The crash target.  */
> @@ -46,7 +44,6 @@ public:
>     const target_info &info () const override
>     { return crash_target_info; }
>   
> -  void fetch_registers (struct regcache *, int) override;
>     enum target_xfer_status xfer_partial (enum target_object object,
>                                           const char *annex,
>                                           gdb_byte *readbuf,
> @@ -57,35 +54,13 @@ public:
>     bool has_all_memory () override { return true; }
>     bool has_memory () override { return true; }
>     bool has_stack () override { return true; }
> -  bool has_registers () override { return true; }
> +  bool has_registers () override { return false; }
>     bool thread_alive (ptid_t ptid) override { return true; }
>     std::string pid_to_str (ptid_t ptid) override
>     { return string_printf ("CPU %ld", ptid.tid ()); }
>   
>   };
>   
> -/* We just get all the registers, so we don't use regno.  */
> -void
> -crash_target::fetch_registers (struct regcache *regcache, int regno)
> -{
> -  gdb_byte regval[16];
> -  int cpu = inferior_ptid.tid();
> -  struct gdbarch *arch = regcache->arch ();
> -
> -  for (int r = 0; r < gdbarch_num_regs (arch); r++)
> -    {
> -      const char *regname = gdbarch_register_name(arch, r);
> -      int regsize = register_size (arch, r);
> -      if (regsize > sizeof (regval))
> -        error (_("fatal error: buffer size is not enough to fit register value"));
> -
> -      if (crash_get_cpu_reg (cpu, r, regname, regsize, (void *)&regval))
> -        regcache->raw_supply (r, regval);
> -      else
> -        regcache->raw_supply (r, NULL);
> -    }
> -}
> -
>   
>   enum target_xfer_status
>   crash_target::xfer_partial (enum target_object object, const char *annex,
> @@ -126,10 +101,4 @@ crash_target_init (void)
>         if (!i)
>           switch_to_thread (thread);
>       }
> -
> -  /* Fetch all registers from core file.  */
> -  target_fetch_registers (get_current_regcache (), -1);
> -
> -  /* Now, set up the frame cache. */
> -  reinit_frame_cache ();
>   }
> diff --git a/defs.h b/defs.h
> index e0ccf2ddc9c2..e8acf49894b8 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -1015,7 +1015,6 @@ struct machdep_table {
>           ulong (*processor_speed)(void);
>           int (*uvtop)(struct task_context *, ulong, physaddr_t *, int);
>           int (*kvtop)(struct task_context *, ulong, physaddr_t *, int);
> -	int (*get_cpu_reg)(int, int, const char *, int, void *);
>           ulong (*get_task_pgd)(ulong);
>   	void (*dump_irq)(int);
>   	void (*get_stack_frame)(struct bt_info *, ulong *, ulong *);
> @@ -6977,7 +6976,6 @@ int vmware_vmss_get_nr_cpus(void);
>   int vmware_vmss_get_cr3_cr4_idtr(int, ulong *, ulong *, ulong *);
>   int vmware_vmss_phys_base(ulong *phys_base);
>   int vmware_vmss_set_phys_base(ulong);
> -int vmware_vmss_get_cpu_reg(int, int, const char *, int, void *);
>   
>   /*
>    * vmware_guestdump.c
> @@ -7403,53 +7401,4 @@ extern int have_full_symbols(void);
>   #define XEN_HYPERVISOR_ARCH
>   #endif
>   
> -/*
> - * Register numbers must be in sync with gdb/features/i386/64bit-core.c
> - * to make crash_target->fetch_registers() ---> machdep->get_cpu_reg()
> - * working properly.
> - */
> -enum x86_64_regnum {
> -        RAX_REGNUM,
> -        RBX_REGNUM,
> -        RCX_REGNUM,
> -        RDX_REGNUM,
> -        RSI_REGNUM,
> -        RDI_REGNUM,
> -        RBP_REGNUM,
> -        RSP_REGNUM,
> -        R8_REGNUM,
> -        R9_REGNUM,
> -        R10_REGNUM,
> -        R11_REGNUM,
> -        R12_REGNUM,
> -        R13_REGNUM,
> -        R14_REGNUM,
> -        R15_REGNUM,
> -        RIP_REGNUM,
> -        EFLAGS_REGNUM,
> -        CS_REGNUM,
> -        SS_REGNUM,
> -        DS_REGNUM,
> -        ES_REGNUM,
> -        FS_REGNUM,
> -        GS_REGNUM,
> -        ST0_REGNUM,
> -        ST1_REGNUM,
> -        ST2_REGNUM,
> -        ST3_REGNUM,
> -        ST4_REGNUM,
> -        ST5_REGNUM,
> -        ST6_REGNUM,
> -        ST7_REGNUM,
> -        FCTRL_REGNUM,
> -        FSTAT_REGNUM,
> -        FTAG_REGNUM,
> -        FISEG_REGNUM,
> -        FIOFF_REGNUM,
> -        FOSEG_REGNUM,
> -        FOOFF_REGNUM,
> -        FOP_REGNUM,
> -        LAST_REGNUM
> -};
> -
>   #endif /* !GDB_COMMON */
> diff --git a/gdb_interface.c b/gdb_interface.c
> index b14319c66147..574447fd82b7 100644
> --- a/gdb_interface.c
> +++ b/gdb_interface.c
> @@ -710,10 +710,11 @@ static char *prohibited_list[] = {
>   	"run", "r", "break", "b", "tbreak", "hbreak", "thbreak", "rbreak",
>   	"watch", "rwatch", "awatch", "attach", "continue", "c", "fg", "detach",
>   	"finish", "handle", "interrupt", "jump", "kill", "next", "nexti",
> -	"signal", "step", "s", "stepi", "target", "until", "delete",
> -	"clear", "disable", "enable", "condition", "ignore", "frame", "catch",
> -	"tcatch", "return", "file", "exec-file", "core-file", "symbol-file",
> -	"load", "si", "ni", "shell", "sy",
> +	"signal", "step", "s", "stepi", "target", "thread", "until", "delete",
> +	"clear", "disable", "enable", "condition", "ignore", "frame",
> +	"select-frame", "f", "up", "down", "catch", "tcatch", "return",
> +	"file", "exec-file", "core-file", "symbol-file", "load", "si", "ni",
> +	"shell", "sy",
>   	NULL  /* must be last */
>   };
>   
> @@ -1068,8 +1069,6 @@ unsigned long crash_get_kaslr_offset(void)
>   
>   /* Callbacks for crash_target */
>   int crash_get_nr_cpus(void);
> -int crash_get_cpu_reg (int cpu, int regno, const char *regname,
> -                       int regsize, void *val);
>   
>   int crash_get_nr_cpus(void)
>   {
> @@ -1086,11 +1085,3 @@ int crash_get_nr_cpus(void)
>           return 1;
>   }
>   
> -int crash_get_cpu_reg (int cpu, int regno, const char *regname,
> -                       int regsize, void *value)
> -{
> -        if (!machdep->get_cpu_reg)
> -                return FALSE;
> -        return machdep->get_cpu_reg(cpu, regno, regname, regsize, value);
> -}
> -
> diff --git a/vmware_vmss.c b/vmware_vmss.c
> index f6c5f32ea4c0..151cfdbca5e5 100644
> --- a/vmware_vmss.c
> +++ b/vmware_vmss.c
> @@ -1,7 +1,7 @@
>   /*
>    * vmware_vmss.c
>    *
> - * Copyright (c) 2015, 2020 VMware, Inc.
> + * Copyright (c) 2015 VMware, Inc.
>    * Copyright (c) 2018 Red Hat Inc.
>    *
>    * This program is free software; you can redistribute it and/or modify
> @@ -16,7 +16,6 @@
>    *
>    * Authors: Dyno Hongjun Fu <hfu@xxxxxxxxxx>
>    *          Sergio Lopez <slp@xxxxxxxxxx>
> - *          Alexey Makhalov <amakhalov@xxxxxxxxxx>
>    */
>   
>   #include "defs.h"
> @@ -895,54 +894,6 @@ vmware_vmss_get_cr3_cr4_idtr(int cpu, ulong *cr3, ulong *cr4, ulong *idtr)
>   	return TRUE;
>   }
>   
> -int
> -vmware_vmss_get_cpu_reg(int cpu, int regno, const char *name, int size,
> -                        void *value)
> -{
> -        if (cpu >= vmss.num_vcpus)
> -                return FALSE;
> -
> -        /* All supported registers are 8 bytes long. */
> -        if (size != 8)
> -                return FALSE;
> -
> -#define CASE(R,r) \
> -                case R##_REGNUM: \
> -                        if (!(vmss.vcpu_regs[cpu] & REGS_PRESENT_##R)) \
> -                                return FALSE; \
> -                        memcpy(value, &vmss.regs64[cpu]->r, size); \
> -                        break
> -
> -
> -        switch (regno) {
> -                CASE (RAX, rax);
> -                CASE (RBX, rbx);
> -                CASE (RCX, rcx);
> -                CASE (RDX, rdx);
> -                CASE (RSI, rsi);
> -                CASE (RDI, rdi);
> -                CASE (RBP, rbp);
> -                CASE (RSP, rsp);
> -                CASE (R8, r8);
> -                CASE (R9, r9);
> -                CASE (R10, r10);
> -                CASE (R11, r11);
> -                CASE (R12, r12);
> -                CASE (R13, r13);
> -                CASE (R14, r14);
> -                CASE (R15, r15);
> -                CASE (RIP, rip);
> -                case EFLAGS_REGNUM:
> -                        if (!(vmss.vcpu_regs[cpu] & REGS_PRESENT_RFLAGS))
> -                                return FALSE;
> -                        memcpy(value, &vmss.regs64[cpu]->rflags, size);
> -                        break;
> -                default:
> -                        return FALSE;
> -        }
> -        return TRUE;
> -}
> -
>   int
>   vmware_vmss_phys_base(ulong *phys_base)
>   {
> diff --git a/x86_64.c b/x86_64.c
> index 74bd1bbde41c..da94b1c0e917 100644
> --- a/x86_64.c
> +++ b/x86_64.c
> @@ -126,7 +126,6 @@ static int x86_64_get_framesize(struct bt_info *, ulong, ulong);
>   static void x86_64_framesize_debug(struct bt_info *);
>   static void x86_64_get_active_set(void);
>   static int x86_64_get_kvaddr_ranges(struct vaddr_range *);
> -static int x86_64_get_cpu_reg(int, int, const char *, int, void *);
>   static int x86_64_verify_paddr(uint64_t);
>   static void GART_init(void);
>   static void x86_64_exception_stacks_init(void);
> @@ -195,7 +194,6 @@ x86_64_init(int when)
>   		machdep->machspec->irq_eframe_link = UNINITIALIZED;
>   		machdep->machspec->irq_stack_gap = UNINITIALIZED;
>   		machdep->get_kvaddr_ranges = x86_64_get_kvaddr_ranges;
> -		machdep->get_cpu_reg = x86_64_get_cpu_reg;
>                   if (machdep->cmdline_args[0])
>                           parse_cmdline_args();
>   		if ((string = pc->read_vmcoreinfo("relocate"))) {
> @@ -889,7 +887,6 @@ x86_64_dump_machdep_table(ulong arg)
>           fprintf(fp, "        is_page_ptr: x86_64_is_page_ptr()\n");
>           fprintf(fp, "       verify_paddr: x86_64_verify_paddr()\n");
>           fprintf(fp, "  get_kvaddr_ranges: x86_64_get_kvaddr_ranges()\n");
> -	fprintf(fp, "        get_cpu_reg: x86_64_get_cpu_reg()\n");
>           fprintf(fp, "    init_kernel_pgd: x86_64_init_kernel_pgd()\n");
>           fprintf(fp, "clear_machdep_cache: x86_64_clear_machdep_cache()\n");
>   	fprintf(fp, " xendump_p2m_create: %s\n", PVOPS_XEN() ?
> @@ -8957,19 +8954,6 @@ x86_64_get_kvaddr_ranges(struct vaddr_range *vrp)
>   	return cnt;
>   }
>   
> -static int
> -x86_64_get_cpu_reg(int cpu, int regno, const char *name,
> -                   int size, void *value)
> -{
> -        if (regno >= LAST_REGNUM)
> -                return FALSE;
> -
> -        if (VMSS_DUMPFILE())
> -                return vmware_vmss_get_cpu_reg(cpu, regno, name, size, value);
> -
> -        return FALSE;
> -}
> -
>   /*
>    *  Determine the physical memory range reserved for GART.
>    */
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux