Skip to content

Commit

Permalink
perf buildid-cache: Fix the file mode with copyfile() while adding fi…
Browse files Browse the repository at this point in the history
…le to build-id cache

The test "build id cache operations" fails on powerpc as below:

	Adding 5a0fd882b53084224ba47b624c55a469 ./tests/shell/../pe-file.exe: Ok
	build id: 5a0fd882b53084224ba47b624c55a469
	link: /tmp/perf.debug.ZTu/.build-id/5a/0fd882b53084224ba47b624c55a469
	file: /tmp/perf.debug.ZTu/.build-id/5a/../../root/linux/tools/perf/tests/pe-file.exe/5a0fd882b53084224ba47b624c55a469/elf
	failed: file /tmp/perf.debug.ZTu/.build-id/5a/../../root/linux/tools/perf/tests/pe-file.exe/5a0fd882b53084224ba47b624c55a469/elf does not exist
	test child finished with -1
	---- end ----
	build id cache operations: FAILED!

The failing test is when trying to add pe-file.exe to build id cache.

'perf buildid-cache' can be used to add/remove/manage files from the
build-id cache. "-a" option is used to add a file to the build-id cache.

Simple command to do so for a PE exe file:

  # ls -ltr tests/pe-file.exe
  -rw-r--r--. 1 root root 75595 Jan 10 23:35 tests/pe-file.exe

  The file is in home directory.

  # mkdir  /tmp/perf.debug.TeY1
  # perf --buildid-dir /tmp/perf.debug.TeY1 buildid-cache -v -a tests/pe-file.exe

The above will create ".build-id" folder in build id directory, which is
/tmp/perf.debug.TeY1. Also adds file to this folder under build id.
Example:

  # ls -ltr /tmp/perf.debug.TeY1/.build-id/5a/0fd882b53084224ba47b624c55a469/
  total 76
  -rw-r--r--. 1 root root     0 Jan 11 00:38 probes
  -rwxr-xr-x. 1 root root 75595 Jan 11 00:38 elf

We can see in the results that file mode for original file and file in
build id directory is different. ie, build id file has executable
permission whereas original file doesn’t have.

The code path and function (build_id_cache__add  to add a file to the
cache is in "util/build-id.c". In build_id_cache__add() function, it
first attempts to link the original file to destination cache folder.

If linking the file fails (which can happen if the destination and
source is on a different mount points), it will copy the file to
destination.  Here copyfile() routine explicitly uses mode as "755" and
hence file in the destination will have executable permission.

Code snippet:

 if (link(realname, filename) && errno != EEXIST && copyfile(name, filename))

strace logs:

	172285 link("/home/<user_name>/linux/tools/perf/tests/pe-file.exe", "/tmp/perf.debug.TeY1/home/<user_name>/linux/tools/perf/tests/pe-file.exe/5a0fd882b53084224ba47b624c55a469/elf") = -1 EXDEV (Invalid cross-device link)
	172285 newfstatat(AT_FDCWD, "tests/pe-file.exe", {st_mode=S_IFREG|0644, st_size=75595, ...}, 0) = 0
	172285 openat(AT_FDCWD, "/tmp/perf.debug.TeY1/home/<user_name>/linux/tools/perf/tests/pe-file.exe/5a0fd882b53084224ba47b624c55a469/.elf.KbAnsl", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
	172285 fchmod(3, 0755)                  = 0
	172285 openat(AT_FDCWD, "tests/pe-file.exe", O_RDONLY) = 4
	172285 mmap(NULL, 75595, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fffa5cd0000
	172285 pwrite64(3, "MZ\220\0\3\0\0\0\4\0\0\0\377\377\0\0\270\0\0\0\0\0\0\0@\0\0\0\0\0\0\0"..., 75595, 0) = 75595

Whereas if the link succeeds, it succeeds in the first attempt itself
and the file in the build-id dir will have same permission as original
file.

Example, above uses /tmp. Instead if we use "--buildid-dir /home/build",
linking will work here since mount points are same. Hence the
destination file will not have executable permission.

Since the testcase "tests/shell/buildid.sh" always looks for executable
file, test fails in powerpc environment when test is run from /root.

The patch adds a change in build_id_cache__add() to use copyfile_mode()
which also passes the file’s original mode as argument. This way the
destination file mode also will be same as original file.

Signed-off-by: Athira Jajeev <atrajeev@linux.vnet.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Disha Goel <disgoel@linux.ibm.com>
Cc: Ian Rogers <irogers@google.com>
Cc: James Clark <james.clark@arm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nageswara R Sastry <rnsastry@linux.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org
Link: https://lore.kernel.org/r/20230116050131.17221-1-atrajeev@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
  • Loading branch information
Athira Rajeev authored and Arnaldo Carvalho de Melo committed Jan 18, 2023
1 parent 85c4491 commit 4b21b3e
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions tools/perf/util/build-id.c
Original file line number Diff line number Diff line change
Expand Up @@ -715,9 +715,13 @@ build_id_cache__add(const char *sbuild_id, const char *name, const char *realnam
} else if (nsi && nsinfo__need_setns(nsi)) {
if (copyfile_ns(name, filename, nsi))
goto out_free;
} else if (link(realname, filename) && errno != EEXIST &&
copyfile(name, filename))
goto out_free;
} else if (link(realname, filename) && errno != EEXIST) {
struct stat f_stat;

if (!(stat(name, &f_stat) < 0) &&
copyfile_mode(name, filename, f_stat.st_mode))
goto out_free;
}
}

/* Some binaries are stripped, but have .debug files with their symbol
Expand Down

0 comments on commit 4b21b3e

Please sign in to comment.