lkml.org 
[lkml]   [2014]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/5] perf/sdt: ELF support for SDT

On 10/03/2014 05:09 PM, Masami Hiramatsu wrote:
> (2014/10/01 11:47), Hemant Kumar wrote:
>> This patch serves as the basic support to identify and list SDT events in binaries.
>> When programs containing SDT markers are compiled, gcc with the help of assembler
>> directives identifies them and places them in the section ".note.stapsdt". To find
>> these markers from the binaries, one needs to traverse through this section and
>> parse the relevant details like the name, type and location of the marker. Also,
>> the original location could be skewed due to the effect of prelinking. If that is
>> the case, the locations need to be adjusted.
>>
>> The functions in this patch open a given ELF, find out the SDT section, parse the
>> relevant details, adjust the location (if necessary) and populate them in a list.
>>
>> Made the necessary changes as suggested by Namhyung Kim.
> Looks almost good!
> I have some comments, please see below.

Ok.

>> Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
>> ---
>> tools/perf/util/symbol-elf.c | 207 ++++++++++++++++++++++++++++++++++++++++++
>> tools/perf/util/symbol.h | 19 ++++
>> 2 files changed, 226 insertions(+)
>>
>> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
>> index 2a92e10..9702167 100644
>> --- a/tools/perf/util/symbol-elf.c
>> +++ b/tools/perf/util/symbol-elf.c
>> @@ -1672,6 +1672,213 @@ void kcore_extract__delete(struct kcore_extract *kce)
>> unlink(kce->extract_filename);
>> }
>>
>> +/*
>> + * populate_sdt_note() : Responsible for parsing the section .note.stapsdt and
>> + * after adjusting the note's location, returns that to the calling functions.
>> + */
>> +static int populate_sdt_note(Elf **elf, const char *data, size_t len, int type,
>> + struct sdt_note **note)
>> +{
>> + const char *provider, *name;
>> + struct sdt_note *tmp = NULL;
>> + GElf_Ehdr ehdr;
>> + GElf_Addr base_off = 0;
>> + GElf_Shdr shdr;
>> + int ret = -1;
> -1 is not an error code. maybe, -EINVAL is better.

Sure, will change this to EINVAL.

>> + int i;
>> +
>> + union {
>> + Elf64_Addr a64[3];
>> + Elf32_Addr a32[3];
>> + } buf;
>> +
>> + Elf_Data dst = {
>> + .d_buf = &buf, .d_type = ELF_T_ADDR, .d_version = EV_CURRENT,
>> + .d_size = gelf_fsize((*elf), ELF_T_ADDR, 3, EV_CURRENT),
>> + .d_off = 0, .d_align = 0
>> + };
>> + Elf_Data src = {
>> + .d_buf = (void *) data, .d_type = ELF_T_ADDR,
>> + .d_version = EV_CURRENT, .d_size = dst.d_size, .d_off = 0,
>> + .d_align = 0
>> + };
>> +
>> + /* Check the type of each of the notes */
>> + if (type != SDT_NOTE_TYPE)
>> + goto out_err;
>> +
>> + tmp = (struct sdt_note *)calloc(1, sizeof(struct sdt_note));
>> + if (!tmp) {
>> + ret = -ENOMEM;
>> + goto out_err;
>> + }
>> +
>> + INIT_LIST_HEAD(&tmp->note_list);
>> +
>> + if (len < dst.d_size + 3)
>> + goto out_free_note;
>> +
>> + /* Translation from file representation to memory representation */
>> + if (gelf_xlatetom(*elf, &dst, &src,
>> + elf_getident(*elf, NULL)[EI_DATA]) == NULL)
>> + printf("gelf_xlatetom : %s\n", elf_errmsg(-1));
> pr_err? and shouldn't it goes to out_free_note?

Yes it should go to out_free_note. Will make the change.

>> [...]
>>
>>

Thank You for reviewing this patch.

--
Thanks,
Hemant Kumar



\
 
 \ /
  Last update: 2014-10-05 11:01    [W:0.076 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site