From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754036AbbIRO32 (ORCPT ); Fri, 18 Sep 2015 10:29:28 -0400 Received: from eu-smtp-delivery-143.mimecast.com ([146.101.78.143]:45152 "EHLO eu-smtp-delivery-143.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753141AbbIRO31 convert rfc822-to-8bit (ORCPT ); Fri, 18 Sep 2015 10:29:27 -0400 Message-ID: <55FC1FC4.8070807@arm.com> Date: Fri, 18 Sep 2015 15:29:24 +0100 From: Kapileshwar Singh User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Steven Rostedt CC: Namhyung Kim , "linux-kernel@vger.kernel.org" , Arnaldo Carvalho de Melo , Javi Merino , David Ahern , Jiri Olsa Subject: Re: [PATCH] tools lib traceevent: Mask higher bits of str addresses for 32-bit traces References: <1442488476-15366-1-git-send-email-kapileshwar.singh@arm.com> <20150917091154.575d031f@gandalf.local.home> <55FAD512.5030206@arm.com> <55FBEDB3.3030705@arm.com> <20150918094552.0c2d3a5b@gandalf.local.home> In-Reply-To: <20150918094552.0c2d3a5b@gandalf.local.home> X-OriginalArrivalTime: 18 Sep 2015 14:29:24.0408 (UTC) FILETIME=[6A897B80:01D0F21E] X-MC-Unique: LWEhuanoR3WtOsX43CLzoQ-1 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Steve, On 18/09/15 14:45, Steven Rostedt wrote: > On Fri, 18 Sep 2015 11:55:47 +0100 > Kapileshwar Singh wrote: > >>>>> Perhaps we need to make addr into a unsigned long long, and then add: >>>>> >>>>> addr = (pevent->long_size == 8) ? >>>>> *(unsigned long long *)(data + field->offset) : >>>>> (unsigned long long )*(unsigned int *)(data + field->offset); >>> >>> What about this? (untested) >>> >>> addr = *(uint64_t *)(data + field->offset) & >>> ((1ULL << pevent->long_size * 8) - 1); >> >> I tested this and it works fine. > > Except that I think it may be buggy. > >> >>> >>> Do we also need to consider byte endians? Maybe it'd be better adding >>> a helper to dereference pointers then.. > > Yes and no. > >> >> In this particular case, since the address is just a key for a lookup into the >> printk_map, which seems like a (addr -> const char *) mapping for string >> literals in the trace file, the endian-ness should not matter (I could be wrong though). > > Correct, which is why I said "no", BUT! this is why I think Namhyung's > version may be buggy (besides the overflow of the buffer). > > If this is a 64 bit big endian reading a 32 bit little endian file, I > think the result will be incorrect. > > The *(uint64_t *) will return a 64bit number, but the address (with > long_size == 4) only needs 32bits. Thus, we are getting 32 more bits > than needed. Let's say the address is 0x12345678 that is loaded in the > file. Being little endian, it would be loaded as "78 56 34 12". Let's > say the 32bits after that is 0xDEADBEEF, loaded as "EF BE AD DE". Now > the number returned to addr (being a 64 bit big endian) would be: > 0x785643412EFBEADDE But then we do the shift: > > (1ULL << pevent->long_size * 8) - 1; which would leave us with: > > 0xEFBEADDE > > Not what we wanted. Agreed. > > My version only reads the necessary bytes, and also wont suffer from > reading past the data size of the buffer (which is another bug). > Thanks for noticing and explaining this, makes perfect sense now! Will submit a v3 for this. Regards, KP > -- Steve > > >