Re: [Crash-utility] [PATCH] crash-gcore-command extension module: ARM64 support

Hi Dave, which glibc version are you using? for us, it seems /usr/include/sys/procfs.h is not included, we still met build error with your patch

so Lei and me made some patches to use the old way for ARM64, and also add compat mode support for ARM64

these patches are verified on both ARM64 compat mode and ARM64, please help review, thanks

Best Regards, Wei -----Original Message----- From: crash-utility-bounces(a)redhat.com [mailto:crash-utility-bounces@redhat.com] On Behalf Of Dave Anderson Sent: Tuesday, July 29, 2014 8:31 PM To: HATAYAMA Daisuke Cc: crash-utility(a)redhat.com Subject: Re: [Crash-utility] [PATCH] crash-gcore-command extension module: ARM64 support ----- Original Message ----- > From: Dave Anderson <anderson(a)redhat.com&gt; > Subject: [PATCH] crash-gcore-command extension module: ARM64 support > Date: Thu, 17 Jul 2014 12:04:45 -0400 > > > > > > > Hello Daisuke, > > > > Hello Dave, > > Sorry for very late responce.... > > > Attached is a patch to introduce support for the ARM64 architecture > > for the gcore extension module. > > > > The patch is fairly straight-forward other than the fact that on > > ARM64 machines, the chain of headers included from the crash > > utility's "defs.h" looks like this: > > > > /usr/include/crash/defs.h > > /usr/include/signal.h > > /usr/include/sys/ucontext.h > > /usr/include/sys/procfs.h > > > > The <sys/procfs.h> file defines several of the ELF-related > > structures that are hard-coded in gcore_defs.h, causing compile > > failures due to duplicate structure declarations. > > > > Note that the <sys/ucontext.h> file on the other three architectures > > does not #include <sys/procfs.h> so there are no conflicts. It > > would be possible to move all architectures to include > > <sys/procfs.h>, but for example, that would also bring in the > > <sys/user.h> definition of the user_regs_struct, which in turn > > causes a myriad of register name mismatches in gcore_x86.c. > > So for the sake of simplicity, wherever there is an ARM64-only > > duplicate structure or definition in gcore_defs.h, I've encapsulated them > > by: > > > > #if defined(X86) || defined(X86_64) || defined(ARM) > > > > Thanks for this explanation. I'll also try to investigate this. > > > Also, there are two generic fixes, one where the gcore module fails > > on Linux 3.11 and later kernels due to a structure member name > > change, and another that changes the getopt() return variable to an > > "int" instead of a "char". > > > > Here are the details: > > > > gcore.mk: > > > > - Introduce ARM64 as a supported architecture > > - Add libgcore/gcore_arm64 to GCORE_CFILES > > > > gcore.c: > > > > - In cmd_gcore() change "c" type to "int" to correctly match > > the return type of getopt(); without it, the while loop > > spins indefinitely on ARM64. > > > - In gcore_offset_table_init(), account for the Linux 3.11 > > structure member name-change from ns_proxy.pid_ns to > > ns_proxy.pid_ns_for_children; without it, the gcore command > > fails during initialization. > > I've already dealt with this locally, but thanks. > > > > > libgcore/gcore_defs.h: > > > > - Add ARM64 ELF- and REGSET_VIEW-related #defines required for > > each architecture. > > - Account for variable page sizes in ARM64. > > - Restrict the hard-coded ELF_NGREG, elf_siginfo, elf_prstatus, > > __kernel_old_uid_t and __kernel_old_gid_t, and elf_prsinfo > > definitions to X86, X86_64 and ARM architectures. > > - Add ARM64 thread_struct_fpsmid_state and thread_struct_tp_value > > offsets to gcore_offset_table. > > > > libgcore/gcore_coredump.c: > > > > - In fill_prstatus_note(), account for the ARM64 usage of > > "user_pt_regs" structure instead of the "user_regs_struct" > > used by the other architectures. > > > > libgcore/gcore_arm64.c: > > > > - Implement ARM64-specific user_regset and user_regset_view > > structures and all required support functions. > > > > Please accept these changes into an new package version. > > > > I'll commit these into my local repository with some changes for ease > of my maintainance purpose. Maybe, I'll ask you to review the changed > version. Excellent -- thanks! I thought I'd lost you! Dave -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility