Re: [Crash-utility] [PATCH] Fix for 'struct -o' option to display the offsets of struct fields

Date: Fri, 27 May 2022 05:56:38 +0000 From: HAGIO KAZUHITO(?????) <k-hagio-ab(a)nec.com&gt; To: &quot;crash-utility(a)redhat.com&quot; <crash-utility(a)redhat.com&gt; Subject: Re: [Crash-utility] [PATCH] Fix for 'struct -o' option to display the offsets of struct fields Message-ID: <3b82e6a4-63db-d163-cc55-ec268c5a4b33(a)nec.com&gt; Content-Type: text/plain; charset="utf-8" On 2022/05/27 11:36, HAGIO KAZUHITO(?? ??) wrote: > On 2022/05/27 11:22, HAGIO KAZUHITO(?? ??) wrote: >> Hi Lianbo, >> >> On 2022/05/26 16:07, Lianbo Jiang wrote: >>> Currently, the 'struct -o' command does not display the offsets >>> of struct fields like this: >>> >>> crash> struct page -ox ffffce98870a0d40 >>> struct page { >>> [ffffce98870a0d40] unsigned long flags; >>> union { >>> struct {...}; >>> struct {...}; >>> struct {...}; >>> struct {...}; >>> struct {...}; >>> struct {...}; >>> struct {...}; >>> [ffffce98870a0d48] struct callback_head callback_head; >>> }; >>> ... >>> } >>> SIZE: 0x40 >> >> Good catch! >> >>> >>> The gdb-10.2 added a new option '/o' for the 'ptype' command, which >>> prints the offsets and sizes of struct fields, let's use it now to >>> fix the above issue. >> >> but isn't there another gdb option or setting to show their details? >> >> (gdb) ptype struct page >> type = struct page { >> unsigned long flags; >> union { >> struct {...}; >> struct {...}; >> struct {...}; >> ... >> >> If there is no setting other than '/o', it's a bit strange UI design, >> I think.. (although I cannot find so far..) >>

>> >> Also the 'ptype /o' overdoes unfolding struct members, >> and extra spaces and lines are printed with the patch: >> >> * crash-7.3.2 >> >> crash-7.3.2> struct address_space >> struct address_space { >> struct inode *host; >> struct xarray i_pages; >> atomic_t i_mmap_writable; >> struct rb_root_cached i_mmap; >> struct rw_semaphore i_mmap_rwsem; >> unsigned long nrpages; >> ... >> crash-7.3.2> struct -o address_space >> struct address_space { >> [0] struct inode *host; >> [8] struct xarray i_pages; >> [32] atomic_t i_mmap_writable; >> [40] struct rb_root_cached i_mmap; >> [56] struct rw_semaphore i_mmap_rwsem; >> [96] unsigned long nrpages; >> >> * With the patch >> >> crash-dev> struct address_space >> struct address_space { >> struct inode *host; >> struct xarray { >> spinlock_t xa_lock; >> gfp_t xa_flags; >> void *xa_head; >> size_t xarray_size_rh; >> struct xarray_rh { >> <no data fields> >> >> >> } _rh; >> >> >> } i_pages; >> atomic_t i_mmap_writable; >> >> struct rb_root_cached { >> struct rb_root { >> struct rb_node *rb_node; >> ... >> crash-dev> struct -o address_space >> struct address_space { >> [0] struct inode *host; >> struct xarray { >> spinlock_t xa_lock; >> gfp_t xa_flags; >> void *xa_head; >> size_t xarray_size_rh; >> struct xarray_rh { >> <no data fields> >> >> >> } _rh; >> >> >> [8] } i_pages; >> [32] atomic_t i_mmap_writable; >> >> struct rb_root_cached { >> struct rb_root { >> struct rb_node *rb_node; >> ... >> >> >> task_struct also looks wrong: >> >> crash-dev> struct -o task_struct >> struct task_struct { >> struct thread_info { >> [36] unsigned long flags; >> u32 status; >> >> >> >> [0] } thread_info; >> [16] volatile long state; >> [24] void *stack; >> ... >> >> >> If no another gdb setting, we will need to rewrite the parser. >> So first, I'd like to know whether there is no another setting. > > Otherwise, maybe we can patch the gdb code... > > That '{...}' is probably printed in c_type_print_base_struct_union(). > And found a comment for c_type_print_base_1() in gdb/c-typeprint.c: > > SHOW negative means just print the type name or struct tag if there > is one. If there is no name, print something sensible but concise > like "struct {...}". > > Just an idea and clue.

Just a rough try and I've not tested enough, but this patch might be somewhat good. --- gdb-10.2.orig/gdb/c-typeprint.c 2022-05-27 14:49:53.079853333 +0900 +++ gdb-10.2/gdb/c-typeprint.c 2022-05-27 14:47:18.729165094 +0900 @@ -1043,6 +1043,8 @@ struct type_print_options local_flags = *flags; local_flags.local_typedefs = NULL; + show = 1; + std::unique_ptr<typedef_hash_table> hash_holder; if (!flags->raw) {

Thanks, Kazu ------------------------------