Skip to content

Commit bde4ccf

Browse files
captain5050namhyung
captain5050
authored and
namhyung
committed
perf annotate: Use an array for the disassembler preference
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>
1 parent 013eb04 commit bde4ccf

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
@@ -2100,6 +2100,57 @@ int symbol__annotate2(struct map_symbol *ms, struct evsel *evsel,
21002100
return 0;
21012101
}
21022102

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

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

21932259
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
@@ -2216,56 +2216,6 @@ static int symbol__disassemble_objdump(const char *filename, struct symbol *sym,
22162216
return err;
22172217
}
22182218

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

@@ -2334,28 +2283,26 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
23342283
}
23352284
}
23362285

2337-
err = annotation_options__init_disassemblers(options);
2338-
if (err)
2339-
goto out_remove_tmp;
2340-
23412286
err = -1;
2287+
for (u8 i = 0; i < ARRAY_SIZE(options->disassemblers) && err != 0; i++) {
2288+
enum perf_disassembler dis = options->disassemblers[i];
23422289

2343-
for (int i = 0; i < options->nr_disassemblers && err != 0; ++i) {
2344-
disassembler = options->disassemblers[i];
2345-
2346-
if (!strcmp(disassembler, "llvm"))
2290+
switch (dis) {
2291+
case PERF_DISASM_LLVM:
23472292
err = symbol__disassemble_llvm(symfs_filename, sym, args);
2348-
else if (!strcmp(disassembler, "capstone"))
2293+
break;
2294+
case PERF_DISASM_CAPSTONE:
23492295
err = symbol__disassemble_capstone(symfs_filename, sym, args);
2350-
else if (!strcmp(disassembler, "objdump"))
2296+
break;
2297+
case PERF_DISASM_OBJDUMP:
23512298
err = symbol__disassemble_objdump(symfs_filename, sym, args);
2352-
else
2353-
pr_debug("Unknown disassembler %s, skipping...\n", disassembler);
2354-
}
2355-
2356-
if (err == 0) {
2357-
pr_debug("Disassembled with %s\nannotate.disassemblers=%s\n",
2358-
disassembler, options->disassemblers_str);
2299+
break;
2300+
case PERF_DISASM_UNKNOWN: /* End of disassemblers. */
2301+
default:
2302+
goto out_remove_tmp;
2303+
}
2304+
if (err == 0)
2305+
pr_debug("Disassembled with %s\n", perf_disassembler__strs[dis]);
23592306
}
23602307
out_remove_tmp:
23612308
if (decomp)

0 commit comments

Comments
 (0)