Skip to content

Commit 1f6b3ab

Browse files
rfmvhrfmvh
committed
perf annotate: Use an array for the disassembler preference
JIRA: https://issues.redhat.com/browse/RHEL-77935 upstream ======== commit bde4ccf Author: Ian Rogers <irogers@google.com> Date: Thu Jan 23 20:38:56 2025 -0800 description =========== Prior to this change a string was used which could cause issues with an unrecognized disassembler in symbol__disassembler. Change to initializing an array of perf_disassembler enum values. If a value already exists then adding it a second time is ignored to avoid array out of bounds problems present in the previous code, it also allows a statically sized array and removes memory allocation needs. Errors in the disassembler string are reported when the config is parsed during perf annotate or perf top start up. If the array is uninitialized after processing the config file the default llvm, capstone then objdump values are added but without a need to parse a string. Fixes: a6e8a58 ("perf disasm: Allow configuring what disassemblers to use") Closes: https://lore.kernel.org/lkml/CAP-5=fUdfCyxmEiTpzS2uumUp3-SyQOseX2xZo81-dQtWXj6vA@mail.gmail.com/ Signed-off-by: Ian Rogers <irogers@google.com> Tested-by: Namhyung Kim <namhyung@kernel.org> Link: https://lore.kernel.org/r/20250124043856.1177264-1-irogers@google.com Signed-off-by: Namhyung Kim <namhyung@kernel.org> Signed-off-by: Michael Petlan <mpetlan@redhat.com>
1 parent a8aee09 commit 1f6b3ab

File tree

3 files changed

+96
-78
lines changed

3 files changed

+96
-78
lines changed

‎tools/perf/util/annotate.c‎

Lines changed: 71 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2102,6 +2102,57 @@ int symbol__annotate2(struct map_symbol *ms, struct evsel *evsel,
21022102
return 0;
21032103
}
21042104

2105+
const char * const perf_disassembler__strs[] = {
2106+
[PERF_DISASM_UNKNOWN] = "unknown",
2107+
[PERF_DISASM_LLVM] = "llvm",
2108+
[PERF_DISASM_CAPSTONE] = "capstone",
2109+
[PERF_DISASM_OBJDUMP] = "objdump",
2110+
};
2111+
2112+
2113+
static void annotation_options__add_disassembler(struct annotation_options *options,
2114+
enum perf_disassembler dis)
2115+
{
2116+
for (u8 i = 0; i < ARRAY_SIZE(options->disassemblers); i++) {
2117+
if (options->disassemblers[i] == dis) {
2118+
/* Disassembler is already present then don't add again. */
2119+
return;
2120+
}
2121+
if (options->disassemblers[i] == PERF_DISASM_UNKNOWN) {
2122+
/* Found a free slot. */
2123+
options->disassemblers[i] = dis;
2124+
return;
2125+
}
2126+
}
2127+
pr_err("Failed to add disassembler %d\n", dis);
2128+
}
2129+
2130+
static int annotation_options__add_disassemblers_str(struct annotation_options *options,
2131+
const char *str)
2132+
{
2133+
while (str && *str != '\0') {
2134+
const char *comma = strchr(str, ',');
2135+
int len = comma ? comma - str : (int)strlen(str);
2136+
bool match = false;
2137+
2138+
for (u8 i = 0; i < ARRAY_SIZE(perf_disassembler__strs); i++) {
2139+
const char *dis_str = perf_disassembler__strs[i];
2140+
2141+
if (len == (int)strlen(dis_str) && !strncmp(str, dis_str, len)) {
2142+
annotation_options__add_disassembler(options, i);
2143+
match = true;
2144+
break;
2145+
}
2146+
}
2147+
if (!match) {
2148+
pr_err("Invalid disassembler '%.*s'\n", len, str);
2149+
return -1;
2150+
}
2151+
str = comma ? comma + 1 : NULL;
2152+
}
2153+
return 0;
2154+
}
2155+
21052156
static int annotation__config(const char *var, const char *value, void *data)
21062157
{
21072158
struct annotation_options *opt = data;
@@ -2117,11 +2168,10 @@ static int annotation__config(const char *var, const char *value, void *data)
21172168
else if (opt->offset_level < ANNOTATION__MIN_OFFSET_LEVEL)
21182169
opt->offset_level = ANNOTATION__MIN_OFFSET_LEVEL;
21192170
} else if (!strcmp(var, "annotate.disassemblers")) {
2120-
opt->disassemblers_str = strdup(value);
2121-
if (!opt->disassemblers_str) {
2122-
pr_err("Not enough memory for annotate.disassemblers\n");
2123-
return -1;
2124-
}
2171+
int err = annotation_options__add_disassemblers_str(opt, value);
2172+
2173+
if (err)
2174+
return err;
21252175
} else if (!strcmp(var, "annotate.hide_src_code")) {
21262176
opt->hide_src_code = perf_config_bool("hide_src_code", value);
21272177
} else if (!strcmp(var, "annotate.jump_arrows")) {
@@ -2187,9 +2237,25 @@ void annotation_options__exit(void)
21872237
zfree(&annotate_opts.objdump_path);
21882238
}
21892239

2240+
static void annotation_options__default_init_disassemblers(struct annotation_options *options)
2241+
{
2242+
if (options->disassemblers[0] != PERF_DISASM_UNKNOWN) {
2243+
/* Already initialized. */
2244+
return;
2245+
}
2246+
#ifdef HAVE_LIBLLVM_SUPPORT
2247+
annotation_options__add_disassembler(options, PERF_DISASM_LLVM);
2248+
#endif
2249+
#ifdef HAVE_LIBCAPSTONE_SUPPORT
2250+
annotation_options__add_disassembler(options, PERF_DISASM_CAPSTONE);
2251+
#endif
2252+
annotation_options__add_disassembler(options, PERF_DISASM_OBJDUMP);
2253+
}
2254+
21902255
void annotation_config__init(void)
21912256
{
21922257
perf_config(annotation__config, &annotate_opts);
2258+
annotation_options__default_init_disassemblers(&annotate_opts);
21932259
}
21942260

21952261
static unsigned int parse_percent_type(char *str1, char *str2)

‎tools/perf/util/annotate.h‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,13 @@ struct annotated_data_type;
3434
#define ANNOTATION__BR_CNTR_WIDTH 30
3535
#define ANNOTATION_DUMMY_LEN 256
3636

37-
// llvm, capstone, objdump
38-
#define MAX_DISASSEMBLERS 3
37+
enum perf_disassembler {
38+
PERF_DISASM_UNKNOWN = 0,
39+
PERF_DISASM_LLVM,
40+
PERF_DISASM_CAPSTONE,
41+
PERF_DISASM_OBJDUMP,
42+
};
43+
#define MAX_DISASSEMBLERS (PERF_DISASM_OBJDUMP + 1)
3944

4045
struct annotation_options {
4146
bool hide_src_code,
@@ -52,14 +57,12 @@ struct annotation_options {
5257
annotate_src,
5358
full_addr;
5459
u8 offset_level;
55-
u8 nr_disassemblers;
60+
u8 disassemblers[MAX_DISASSEMBLERS];
5661
int min_pcnt;
5762
int max_lines;
5863
int context;
5964
char *objdump_path;
6065
char *disassembler_style;
61-
const char *disassemblers_str;
62-
const char *disassemblers[MAX_DISASSEMBLERS];
6366
const char *prefix;
6467
const char *prefix_strip;
6568
unsigned int percent_type;
@@ -134,6 +137,8 @@ struct disasm_line {
134137
struct annotation_line al;
135138
};
136139

140+
extern const char * const perf_disassembler__strs[];
141+
137142
void annotation_line__add(struct annotation_line *al, struct list_head *head);
138143

139144
static inline double annotation_data__percent(struct annotation_data *data,

‎tools/perf/util/disasm.c‎

Lines changed: 15 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -2213,56 +2213,6 @@ static int symbol__disassemble_objdump(const char *filename, struct symbol *sym,
22132213
return err;
22142214
}
22152215

2216-
static int annotation_options__init_disassemblers(struct annotation_options *options)
2217-
{
2218-
char *disassembler;
2219-
2220-
if (options->disassemblers_str == NULL) {
2221-
const char *default_disassemblers_str =
2222-
#ifdef HAVE_LIBLLVM_SUPPORT
2223-
"llvm,"
2224-
#endif
2225-
#ifdef HAVE_LIBCAPSTONE_SUPPORT
2226-
"capstone,"
2227-
#endif
2228-
"objdump";
2229-
2230-
options->disassemblers_str = strdup(default_disassemblers_str);
2231-
if (!options->disassemblers_str)
2232-
goto out_enomem;
2233-
}
2234-
2235-
disassembler = strdup(options->disassemblers_str);
2236-
if (disassembler == NULL)
2237-
goto out_enomem;
2238-
2239-
while (1) {
2240-
char *comma = strchr(disassembler, ',');
2241-
2242-
if (comma != NULL)
2243-
*comma = '\0';
2244-
2245-
options->disassemblers[options->nr_disassemblers++] = strim(disassembler);
2246-
2247-
if (comma == NULL)
2248-
break;
2249-
2250-
disassembler = comma + 1;
2251-
2252-
if (options->nr_disassemblers >= MAX_DISASSEMBLERS) {
2253-
pr_debug("annotate.disassemblers can have at most %d entries, ignoring \"%s\"\n",
2254-
MAX_DISASSEMBLERS, disassembler);
2255-
break;
2256-
}
2257-
}
2258-
2259-
return 0;
2260-
2261-
out_enomem:
2262-
pr_err("Not enough memory for annotate.disassemblers\n");
2263-
return -1;
2264-
}
2265-
22662216
int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
22672217
{
22682218
struct annotation_options *options = args->options;
@@ -2271,7 +2221,6 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
22712221
char symfs_filename[PATH_MAX];
22722222
bool delete_extract = false;
22732223
struct kcore_extract kce;
2274-
const char *disassembler;
22752224
bool decomp = false;
22762225
int err = dso__disassemble_filename(dso, symfs_filename, sizeof(symfs_filename));
22772226

@@ -2331,28 +2280,26 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
23312280
}
23322281
}
23332282

2334-
err = annotation_options__init_disassemblers(options);
2335-
if (err)
2336-
goto out_remove_tmp;
2337-
23382283
err = -1;
2284+
for (u8 i = 0; i < ARRAY_SIZE(options->disassemblers) && err != 0; i++) {
2285+
enum perf_disassembler dis = options->disassemblers[i];
23392286

2340-
for (int i = 0; i < options->nr_disassemblers && err != 0; ++i) {
2341-
disassembler = options->disassemblers[i];
2342-
2343-
if (!strcmp(disassembler, "llvm"))
2287+
switch (dis) {
2288+
case PERF_DISASM_LLVM:
23442289
err = symbol__disassemble_llvm(symfs_filename, sym, args);
2345-
else if (!strcmp(disassembler, "capstone"))
2290+
break;
2291+
case PERF_DISASM_CAPSTONE:
23462292
err = symbol__disassemble_capstone(symfs_filename, sym, args);
2347-
else if (!strcmp(disassembler, "objdump"))
2293+
break;
2294+
case PERF_DISASM_OBJDUMP:
23482295
err = symbol__disassemble_objdump(symfs_filename, sym, args);
2349-
else
2350-
pr_debug("Unknown disassembler %s, skipping...\n", disassembler);
2351-
}
2352-
2353-
if (err == 0) {
2354-
pr_debug("Disassembled with %s\nannotate.disassemblers=%s\n",
2355-
disassembler, options->disassemblers_str);
2296+
break;
2297+
case PERF_DISASM_UNKNOWN: /* End of disassemblers. */
2298+
default:
2299+
goto out_remove_tmp;
2300+
}
2301+
if (err == 0)
2302+
pr_debug("Disassembled with %s\n", perf_disassembler__strs[dis]);
23562303
}
23572304
out_remove_tmp:
23582305
if (decomp)

0 commit comments

Comments
 (0)