perf script: fix duplicate symbols in db-export

Changes since v1:
- fixed scripts/checkpatch.pl errors

This patch set contains 3 fixes for duplicate symbol creation in the db-export implementation and one new symbol API required for the fixes.

commit 9c7b37cd63d0 ("perf symbols: Fix handling of zero-length symbols.") already removed the majority of duplicates, but these fixes take care of the remaining corner cases.

each patch (except for the 1st, which is a dependency for patch 2) reduces the number of duplicate symbols exported. When all patches are applied, my test workload has no more duplicate symbols being exported.

Tests ran:

$perf record --call-graph=dwarf stress -c 2 -t 20
$perf script -s scripts/python/export-to-postgresql.py test all callchains $psql test

To show the effect of the changes we run the following query before/after the changes on a database created using the export-to-postgresql.py script with callchains enabled. If this query returns any value greater than 1, then it means that there are duplicates present.

In the test workload, at least one symbol occurs 299 times before applying the fixes:

test=# select count(*) as cnt from symbols group by

	sym_start,sym_end,dso_id order by cnt desc limit 1;

cnt
-----
299
(1 row)

After applying the fixes no symbol occurs more than once:

test=# select count(*) as cnt from symbols group by

	sym_start,sym_end,dso_id order by cnt desc limit 1;
   cnt 
  -----
     1

(1 row)

Chris Phlipot (4):
perf symbols: add dso__insert_symbol function
perf script: fix symbol insertion behavior in db-export perf script: fix callchain addresses in db-export
perf script: fix export of callchains with recursion in db-export

 tools/perf/util/db-export.c | 12 ++++++------
 tools/perf/util/symbol.c    | 12 ++++++++++++
 tools/perf/util/symbol.h    |  3 +++
 3 files changed, 21 insertions(+), 6 deletions(-)

2.7.4

The current method for inserting symbols is to use the symbols__insert function. However symbols__insert does not update the dso symbol cache. This causes problems in the following scenario:

1. symbol not found at ip using dso__find_symbol

2. symbol inserted at ip using the existing symbols__insert function

3. symbol still not found at ip using dso__find_symbol because cache isn't updated. This is undesired behavior.

The undesired behavior in (3) is addressed by creating a new function, dso__insert_symbol to both insert the symbol and update the symbol cache if necessary.

If dso__insert_symbol is used in (2) instead of symbols__insert, then the undesired behavior in (3) is avoided.

---
 tools/perf/util/symbol.c | 12 ++++++++++++
 tools/perf/util/symbol.h |  3 +++
 2 files changed, 15 insertions(+)

index 2946295..21af8e8 100644
@@ -413,6 +413,18 @@ void dso__reset_find_symbol_cache(struct dso *dso)
 	}
 }
 
+void dso__insert_symbol(struct dso *dso, enum map_type type, struct symbol *sym)
+{
+	symbols__insert(&dso->symbols[type], sym);
+
+	/* update the symbol cache if necessary */
+	if (dso->last_find_result[type].addr >= sym->start &&
+	    (dso->last_find_result[type].addr < sym->end ||
+	    sym->start == sym->end)) {
+		dso->last_find_result[type].symbol = sym;
+	}
+}
+
 struct symbol *dso__find_symbol(struct dso *dso,
 				enum map_type type, u64 addr)
 {

index 07211c2..2b5e4ed 100644
@@ -246,6 +246,9 @@ int __dso__load_kallsyms(struct dso *dso, const char *filename, struct map *map,
 int dso__load_kallsyms(struct dso *dso, const char *filename, struct map *map,
 		       symbol_filter_t filter);
 
+void dso__insert_symbol(struct dso *dso, enum map_type type,
+			struct symbol *sym);
+
 struct symbol *dso__find_symbol(struct dso *dso, enum map_type type,
 				u64 addr);
 struct symbol *dso__find_symbol_by_name(struct dso *dso, enum map_type type,

2.7.4

Use the dso__insert_symbol function instead of symbols__insert in order to properly update the dso symbol cache.

If the cache is not updated, then duplicate symbols can be unintentionally created, inserted, and exported.

This change prevents duplicate symbols from being exported due to dso__find_symbol using a stale symbol cache.

---
 tools/perf/util/db-export.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index f8e3057..2ef1f69 100644
@@ -260,8 +260,7 @@ static int db_ids_from_al(struct db_export *dbe, struct addr_location *al,
 		if (!al->sym) {
 			al->sym = symbol__new(al->addr, 0, 0, "unknown");
 			if (al->sym)
-				symbols__insert(&dso->symbols[al->map->type],
-						al->sym);
+				dso__insert_symbol(dso, al->map->type, al->sym);
 		}
 
 		if (al->sym) {

2.7.4

Remove the call to map_ip, because it has already been called when assembling the callchain. Calling it a second time can result in incorrect addresses being used. This can have effects such as duplicate symbols being created and exported.

---
 tools/perf/util/db-export.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

index 2ef1f69..8ca4186 100644
@@ -324,10 +324,7 @@ static struct call_path *call_path_from_sample(struct db_export *dbe,
 		al.sym = node->sym;
 		al.map = node->map;
 		al.machine = machine;
-		if (al.map)
-			al.addr = al.map->map_ip(al.map, node->ip);
-		else
-			al.addr = node->ip;
+		al.addr = node->ip;
 
 		db_ids_from_al(dbe, &al, &dso_db_id, &sym_db_id, &offset);
 

2.7.4

When an IP with an unresolved symbol occurs in the callchain more than once (ie. recursion), then duplicate symbols can be created because the callchain nodes are never updated after they are first created.

To fix this issue we call dso__find_symbol whenever we encounter a NULL symbol, in case we already added a symbol at that IP since we started traversing the callchain.

This change prevents duplicate symbols from being exported when duplicate IPs are present in the callchain.

---
 tools/perf/util/db-export.c | 4 ++++
 1 file changed, 4 insertions(+)

index 8ca4186..8d96c80 100644
@@ -326,6 +326,10 @@ static struct call_path *call_path_from_sample(struct db_export *dbe,
 		al.machine = machine;
 		al.addr = node->ip;
 
+		if (al.map && !al.sym)
+			al.sym = dso__find_symbol(al.map->dso, MAP__FUNCTION,
+						  al.addr);
+
 		db_ids_from_al(dbe, &al, &dso_db_id, &sym_db_id, &offset);
 
 		/* add node to the call path tree if it doesn't exist */

2.7.4