On 10/08/2013 02:39 PM, Namhyung Kim wrote:
[...]
+
+               tmp = strdup(ptr);
+               if (!tmp)
+                       return -ENOMEM;
These -ENOMEM returning should free all memory region allocated
previously.

Yes, missed that.


+               pev->point.note->name = tmp;
+               pev->group = pev->point.note->provider;
+               if (!pev->event)
+                       pev->event = pev->point.note->name;
+               pev->sdt = true;
+               return 0;
+       }
        ptr = strpbrk(arg, ";:+@%");
        if (ptr) {
                nc = *ptr;
@@ -1270,6 +1299,13 @@ static char *synthesize_perf_probe_point(struct 
perf_probe_point *pp)
                ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
                                 offs, pp->retprobe ? "%return" : "", line,
                                 file);
+       else if (pp->note)
+               if (pp->note->bit32)
+                       ret = e_snprintf(buf, MAX_CMDLEN, "0x%x",
+                                        pp->note->addr.a32[0]);
+               else
+                       ret = e_snprintf(buf, MAX_CMDLEN, "0x%lx",
+                                        pp->note->addr.a64[0]);
This kind of code tends to cause 32/64-bit build problem.  Did you test
it on a 32-bit system too?  Anyway, I think things like below should
work:

                unsigned long long addr;

                if (pp->note->bit32)
                        addr = pp->note->addr.a32[0];
                else
                        addr = pp->note->addr.a64[0];

                ret = e_snprintf(buf, MAX_CMDLEN, "0x%llx", addr);


Looks better, thanks, will make the required changes.

        else
                ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
        if (ret <= 0)
@@ -1923,6 +1959,19 @@ static void cleanup_sdt_note_list(struct list_head 
*sdt_notes)
        }
  }
+static int try_to_find_sdt_notes(struct perf_probe_event *pev,
+                                const char *target)
+{
+       struct list_head sdt_notes;
+       int ret = -1;
+
+       INIT_LIST_HEAD(&sdt_notes);
You can use just LIST_HEAD(sdt_notes) here.


Ok.

+       ret = search_sdt_note(pev->point.note, &sdt_notes, target);
+       if (!list_empty(&sdt_notes))
+               cleanup_sdt_note_list(&sdt_notes);
+       return ret;
+}
+
  static int convert_to_probe_trace_events(struct perf_probe_event *pev,
                                          struct probe_trace_event **tevs,
                                          int max_tevs, const char *target)
@@ -1930,11 +1979,20 @@ static int convert_to_probe_trace_events(struct 
perf_probe_event *pev,
        struct symbol *sym;
        int ret = 0, i;
        struct probe_trace_event *tev;
+       char *buf;
- /* Convert perf_probe_event with debuginfo */
-       ret = try_to_find_probe_trace_events(pev, tevs, max_tevs, target);
-       if (ret != 0)
-               return ret;     /* Found in debuginfo or got an error */
+       if (pev->sdt) {
+               ret = try_to_find_sdt_notes(pev, target);
+               if (ret)
+                       return ret;
+       } else {
+               /* Convert perf_probe_event with debuginfo */
+               ret = try_to_find_probe_trace_events(pev, tevs, max_tevs,
+                                                    target);
+               /* Found in debuginfo or got an error */
+               if (ret != 0)
+                       return ret;
+       }
/* Allocate trace event buffer */
        tev = *tevs = zalloc(sizeof(struct probe_trace_event));
@@ -1942,10 +2000,25 @@ static int convert_to_probe_trace_events(struct 
perf_probe_event *pev,
                return -ENOMEM;
/* Copy parameters */
-       tev->point.symbol = strdup(pev->point.function);
-       if (tev->point.symbol == NULL) {
-               ret = -ENOMEM;
-               goto error;
+       if (pev->sdt) {
+               buf = (char *)zalloc(sizeof(char) * MAX_CMDLEN);
+               if (!buf) {
+                       ret = -ENOMEM;
+                       goto error;
+               }
+               if (pev->point.note->bit32)
+                       sprintf(buf, "0x%x",
+                               (unsigned)pev->point.note->addr.a32[0]);
+               else
+                       sprintf(buf, "0x%lx",
+                               (unsigned long)pev->point.note->addr.a64[0]);
Please see my comment above.

Ok. Will change that too.


+               tev->point.symbol = buf;
+       } else {
+               tev->point.symbol = strdup(pev->point.function);
+               if (tev->point.symbol == NULL) {
+                       ret = -ENOMEM;
+                       goto error;
+               }
        }
if (target) {
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index b8a9347..4bd50cc 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -47,6 +47,7 @@ struct perf_probe_point {
        bool            retprobe;       /* Return probe flag */
        char            *lazy_line;     /* Lazy matching pattern */
        unsigned long   offset;         /* Offset from function entry */
+       struct sdt_note *note;
I guess what you need is a 'struct list_head'.

Yes, struct list_head will be a better choice in struct perf_probe_point.


  };
/* Perf probe probing argument field chain */
@@ -72,6 +73,7 @@ struct perf_probe_event {
        struct perf_probe_point point;  /* Probe point */
        int                     nargs;  /* Number of arguments */
        bool                    uprobes;
+       bool                    sdt;
        struct perf_probe_arg   *args;  /* Arguments */
  };
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 6b17260..d0e7a66 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1022,6 +1022,86 @@ out_ret:
        return ret;
  }
+static void adjust_note_addr(struct sdt_note *tmp, struct sdt_note *key,
+                            Elf *elf)
+{
+       GElf_Ehdr ehdr;
+       GElf_Addr base_off = 0;
+       GElf_Shdr shdr;
+
+       if (!gelf_getehdr(elf, &ehdr)) {
+               pr_debug("%s : cannot get elf header.\n", __func__);
+               return;
+       }
+
+       /*
+        * Find out the .stapsdt.base section.
+        * This scn will help us to handle prelinking (if present).
+        * Compare the retrieved file offset of the base section with the
+        * base address in the description of the SDT note. If its different,
+        * then accordingly, adjust the note location.
+        */
+       if (elf_section_by_name(elf, &ehdr, &shdr, SDT_BASE, NULL))
+               base_off = shdr.sh_offset;
+       if (base_off) {
+               if (tmp->bit32)
+                       key->addr.a32[0] = tmp->addr.a32[0] + base_off -
+                               tmp->addr.a32[1];
+               else
+                       key->addr.a64[0] = tmp->addr.a64[0] + base_off -
+                               tmp->addr.a64[1];
+       }
+       key->bit32 = tmp->bit32;
+}
+
+int search_sdt_note(struct sdt_note *key, struct list_head *sdt_notes,
+                   const char *target)
+{
+       Elf *elf;
+       int fd, ret = -1;
+       struct list_head *pos = NULL, *head = NULL;
+       struct sdt_note *tmp = NULL;
+
+       fd = open(target, O_RDONLY);
+       if (fd < 0) {
+               pr_err("Failed to open %s\n", target);
+               goto out_ret;
+       }
+
+       symbol__elf_init();
+       elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
+       if (!elf) {
+               pr_err("%s : can't read %s ELF file.\n", __func__, target);
+               goto out_close;
+       }
+
+       head = construct_sdt_notes_list(elf);
+       if (!head)
+               goto out_end;
+
+       list_add(sdt_notes, head);
This list code looks strange.


Won't need that after making the required changes.

+
+       /* Iterate through the notes and retrieve the required note */
+       list_for_each(pos, sdt_notes) {
+               tmp = list_entry(pos, struct sdt_note, note_list);
+               if (!strcmp(key->name, tmp->name) &&
+                   !strcmp(key->provider, tmp->provider)) {
+                       adjust_note_addr(tmp, key, elf);
+                       ret = 0;
+                       break;
+               }
+       }
+       if (ret)
+               printf("%%%s:%s not found!\n", key->provider, key->name);
+
+out_end:
+       elf_end(elf);
+out_close:
+       close(fd);
+out_ret:
+       return ret;
+}
+
  void symbol__elf_init(void)
  {
        elf_version(EV_CURRENT);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 037185c..0b0545b 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -260,9 +260,12 @@ void __map_groups__fixup_end(struct map_groups *mg, enum 
map_type type);
/* Specific to SDT notes */
  int get_sdt_note_list(struct list_head *head, const char *target);
+int search_sdt_note(struct sdt_note *key, struct list_head *head,
+                   const char *target);
#define SDT_NOTE_TYPE 3
  #define NOTE_SCN ".note.stapsdt"
  #define SDT_NOTE_NAME "stapsdt"
+#define SDT_BASE ".stapsdt.base"
What about SDT_BASE_SCN ?

Seems better.

--
Thanks
Hemant

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to