forked from mirrors/linux
		
	perf srcline: Make sentinel reading for binutils addr2line more robust
The addr2line process is sent an address then multiple function, filename:line "records" are read. To detect the end of output a ',' is sent and for llvm-addr2line a ',' is then read back showing the end of addrline's output. For binutils addr2line the ',' translates to address 0 and we expect the bogus filename marker "??:0" (see filename_split) to be sent from addr2line. For some kernels address 0 may have a mapping and so a seemingly valid inline output is given and breaking the sentinel discovery: ``` $ addr2line -e vmlinux -f -i , __per_cpu_start ./arch/x86/kernel/cpu/common.c:1850 ``` To avoid this problem enable the address dumping for addr2line (the -a option). If an address of 0x0000000000000000 is read then this is the sentinel value working around the problem above. The filename_split still needs to check for "??:0" as bogus non-zero addresses also need handling. Reported-by: Changbin Du <changbin.du@huawei.com> Signed-off-by: Ian Rogers <irogers@google.com> Tested-by: Changbin Du <changbin.du@huawei.com> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Tom Rix <trix@redhat.com> Cc: llvm@lists.linux.dev Link: https://lore.kernel.org/r/20230613034817.1356114-3-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This commit is contained in:
		
							parent
							
								
									c7a0023a14
								
							
						
					
					
						commit
						8dc26b6f71
					
				
					 1 changed files with 53 additions and 8 deletions
				
			
		|  | @ -408,7 +408,7 @@ static struct child_process *addr2line_subprocess_init(const char *addr2line_pat | ||||||
| 	const char *argv[] = { | 	const char *argv[] = { | ||||||
| 		addr2line_path ?: "addr2line", | 		addr2line_path ?: "addr2line", | ||||||
| 		"-e", binary_path, | 		"-e", binary_path, | ||||||
| 		"-i", "-f", NULL | 		"-a", "-i", "-f", NULL | ||||||
| 	}; | 	}; | ||||||
| 	struct child_process *a2l = zalloc(sizeof(*a2l)); | 	struct child_process *a2l = zalloc(sizeof(*a2l)); | ||||||
| 	int start_command_status = 0; | 	int start_command_status = 0; | ||||||
|  | @ -463,10 +463,10 @@ static enum a2l_style addr2line_configure(struct child_process *a2l, const char | ||||||
| 			style = LLVM; | 			style = LLVM; | ||||||
| 			cached = true; | 			cached = true; | ||||||
| 			lines = 1; | 			lines = 1; | ||||||
| 		} else if (ch == '?') { | 		} else if (ch == '0') { | ||||||
| 			style = GNU_BINUTILS; | 			style = GNU_BINUTILS; | ||||||
| 			cached = true; | 			cached = true; | ||||||
| 			lines = 2; | 			lines = 3; | ||||||
| 		} else { | 		} else { | ||||||
| 			if (!symbol_conf.disable_add2line_warn) { | 			if (!symbol_conf.disable_add2line_warn) { | ||||||
| 				char *output = NULL; | 				char *output = NULL; | ||||||
|  | @ -518,13 +518,56 @@ static int read_addr2line_record(struct io *io, | ||||||
| 	if (line_nr != NULL) | 	if (line_nr != NULL) | ||||||
| 		*line_nr = 0; | 		*line_nr = 0; | ||||||
| 
 | 
 | ||||||
|  | 	/*
 | ||||||
|  | 	 * Read the first line. Without an error this will be either an address | ||||||
|  | 	 * like 0x1234 or for llvm-addr2line the sentinal ',' character. | ||||||
|  | 	 */ | ||||||
| 	if (io__getline(io, &line, &line_len) < 0 || !line_len) | 	if (io__getline(io, &line, &line_len) < 0 || !line_len) | ||||||
| 		goto error; | 		goto error; | ||||||
| 
 | 
 | ||||||
| 	if (style == LLVM && line_len == 2 && line[0] == ',') { | 	if (style == LLVM) { | ||||||
|  | 		if (line_len == 2 && line[0] == ',') { | ||||||
| 			zfree(&line); | 			zfree(&line); | ||||||
| 			return 0; | 			return 0; | ||||||
| 		} | 		} | ||||||
|  | 	} else { | ||||||
|  | 		int zero_count = 0, non_zero_count = 0; | ||||||
|  | 
 | ||||||
|  | 		/* The address should always start 0x. */ | ||||||
|  | 		if (line_len < 2 || line[0] != '0' || line[1] != 'x') | ||||||
|  | 			goto error; | ||||||
|  | 
 | ||||||
|  | 		for (size_t i = 2; i < line_len; i++) { | ||||||
|  | 			if (line[i] == '0') | ||||||
|  | 				zero_count++; | ||||||
|  | 			else if (line[i] != '\n') | ||||||
|  | 				non_zero_count++; | ||||||
|  | 		} | ||||||
|  | 		if (!non_zero_count) { | ||||||
|  | 			int ch; | ||||||
|  | 
 | ||||||
|  | 			if (!zero_count) { | ||||||
|  | 				/* Line was erroneous just '0x'. */ | ||||||
|  | 				goto error; | ||||||
|  | 			} | ||||||
|  | 			/*
 | ||||||
|  | 			 * Line was 0x0..0, the sentinel for binutils. Remove | ||||||
|  | 			 * the function and filename lines. | ||||||
|  | 			 */ | ||||||
|  | 			zfree(&line); | ||||||
|  | 			do { | ||||||
|  | 				ch = io__get_char(io); | ||||||
|  | 			} while (ch > 0 && ch != '\n'); | ||||||
|  | 			do { | ||||||
|  | 				ch = io__get_char(io); | ||||||
|  | 			} while (ch > 0 && ch != '\n'); | ||||||
|  | 			return 0; | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	/* Read the second function name line. */ | ||||||
|  | 	if (io__getline(io, &line, &line_len) < 0 || !line_len) | ||||||
|  | 		goto error; | ||||||
| 
 | 
 | ||||||
| 	if (function != NULL) | 	if (function != NULL) | ||||||
| 		*function = strdup(strim(line)); | 		*function = strdup(strim(line)); | ||||||
|  | @ -532,6 +575,7 @@ static int read_addr2line_record(struct io *io, | ||||||
| 	zfree(&line); | 	zfree(&line); | ||||||
| 	line_len = 0; | 	line_len = 0; | ||||||
| 
 | 
 | ||||||
|  | 	/* Read the third filename and line number line. */ | ||||||
| 	if (io__getline(io, &line, &line_len) < 0 || !line_len) | 	if (io__getline(io, &line, &line_len) < 0 || !line_len) | ||||||
| 		goto error; | 		goto error; | ||||||
| 
 | 
 | ||||||
|  | @ -635,8 +679,9 @@ static int addr2line(const char *dso_name, u64 addr, | ||||||
| 		goto out; | 		goto out; | ||||||
| 	case 0: | 	case 0: | ||||||
| 		/*
 | 		/*
 | ||||||
| 		 * The first record was invalid, so return failure, but first read another | 		 * The first record was invalid, so return failure, but first | ||||||
| 		 * record, since we asked a junk question and have to clear the answer out. | 		 * read another record, since we sent a sentinel ',' for the | ||||||
|  | 		 * sake of detected the last inlined function. | ||||||
| 		 */ | 		 */ | ||||||
| 		switch (read_addr2line_record(&io, a2l_style, NULL, NULL, NULL)) { | 		switch (read_addr2line_record(&io, a2l_style, NULL, NULL, NULL)) { | ||||||
| 		case -1: | 		case -1: | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Ian Rogers
						Ian Rogers