Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC][PATCH v4 27/32] objtool: mcount: Generic location and
 relocation table types
To:     Matt Helsley <mhelsley@vmware.com>, linux-kernel@vger.kernel.org,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Sami Tolvanen <samitolvanen@google.com>,
        Julien Thierry <jthierry@redhat.com>
References: <cover.1591125127.git.mhelsley@vmware.com>
 <311ccd169cec877d0c1993772818f0e5cdf11dc4.1591125127.git.mhelsley@vmware.com>
 <79552506-b994-63ce-d3d9-8053dcbc02db@linux.vnet.ibm.com>
 <20200609181231.GD1284251@rlwimi.vmware.com>
From:   Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>
Message-ID: <a9bd2caa-5e82-e7e1-b68e-b2d0bbab6d8b@linux.vnet.ibm.com>
Date:   Wed, 10 Jun 2020 10:05:21 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.0
MIME-Version: 1.0
In-Reply-To: <20200609181231.GD1284251@rlwimi.vmware.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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