Messages in this thread | | | Subject | Re: [RFC][PATCH v4 27/32] objtool: mcount: Generic location and relocation table types | From | Kamalesh Babulal <> | Date | Wed, 10 Jun 2020 10:05:21 +0530 |
| |
On 6/9/20 11:42 PM, Matt Helsley wrote:
[...]
>> Hi Matt, >> >> I was trying out the patch series on ppc64le and found that __mcount_loc >> and .rela__mcount_loc section pairs do not get generated. >> >> # readelf -S fs/proc/cmdline.o|grep mcount >> # >> >> Debugged the cause to get_mcountsym()'s return type. It returns reloc >> type from GELF_R_INFO() and expects Elf64_Xword a.k.a unsigned long >> to be the return type but get_mcountsym() returns unsigned int on 64-bit. >> >> On power the _mcount is of relocation type R_PPC64_REL24 (info 0x170000000a), >> using unsigned int truncates the value to 0xa and fails the above check. >> Using below fix, that converts mcount_sym_info to use unsigned long, generates >> the __mcount_loc section pairs. >> >> --- a/tools/objtool/mcount.c >> +++ b/tools/objtool/mcount.c >> @@ -163,7 +163,7 @@ static int is_mcounted_section_name(char const *const txtname) >> strcmp(".cpuidle.text", txtname) == 0; >> } >> >> -static unsigned int get_mcount_sym_info(struct reloc *reloc) >> +static unsigned long get_mcount_sym_info(struct reloc *reloc) >> { >> struct symbol *sym = reloc->sym; >> char const *symname = sym->name; >> @@ -274,7 +274,7 @@ static int nop_mcount(struct section * const rels, >> { >> struct reloc *reloc; >> struct section *txts = find_section_by_index(lf, rels->sh.sh_info); >> - unsigned int mcount_sym_info = 0; >> + unsigned long mcount_sym_info = 0; >> int once = 0; >> >> list_for_each_entry(reloc, &rels->reloc_list, list) { >> @@ -363,7 +363,7 @@ static void sift_rel_mcount(GElf_Addr **mlocpp, >> { >> GElf_Rel *mrelp = *mrelpp; >> GElf_Rela *mrelap = *mrelpp; >> - unsigned int mcount_sym_info = 0; >> + unsigned long mcount_sym_info = 0; >> struct reloc *reloc; >> >> list_for_each_entry(reloc, &rels->reloc_list, list) { >> >> # readelf -S fs/proc/cmdline.o|grep mcount >> [31] __mcount_loc PROGBITS 0000000000000000 00022f10 >> [32] .rela__mcount_loc RELA 0000000000000000 00022f20 > > Fixed for next posting. > > I've essentially added this as another patch before it moves into > recordmcount.c, gets renamed to get_mcount_sym_info(), etc. I did it > this way because it only becomes necessary to change the type before > moving the function (and eventually its callers) out of the wrapper. > > I feel I should credit you as author or at least co-author of the added > patch since it's basically a "backported" version of the changes you > suggested. I reviewed the process in submitting-patches.rst and propose > the commit message: > > objtool: mcount: Extend mcountsym size > > Before we can move this function out of the wrapper and into > wordsize-independent code we need to explicitly size the > type returned from get_mcountsym() to preserve the symbol info. > > Reported-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> > Signed-off-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> > Signed-off-by: Matt Helsley <mhelsley@vmware.com> > > Is that OK with you or do you have another preference?
Thanks, it works for me.
-- Kamalesh
| |