| Message ID | 20240202220459.527138-15-namhyung@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | perf tools: Remaining bits of data type profiling (v5) | expand |
On Fri, Feb 2, 2024 at 2:05 PM Namhyung Kim <namhyung@kernel.org> wrote: > > When the stack protector is enabled, compiler would generate code to > check stack overflow with a special value called 'stack carary' at > runtime. On x86_64, GCC hard-codes the stack canary as %gs:40. > > While there's a definition of fixed_percpu_data in asm/processor.h, > it seems that the header is not included everywhere and many places > it cannot find the type info. As it's in the well-known location (at > %gs:40), let's add a pseudo stack canary type to handle it specially. I wonder if cases like this can be handled by debug info rather than special cases in the tool. Special cases are fine too, but are potentially less portable. Thanks, Ian > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/util/annotate-data.h | 1 + > tools/perf/util/annotate.c | 24 ++++++++++++++++++++++++ > 2 files changed, 25 insertions(+) > > diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h > index 0bfef29fa52c..e293980eb11b 100644 > --- a/tools/perf/util/annotate-data.h > +++ b/tools/perf/util/annotate-data.h > @@ -77,6 +77,7 @@ struct annotated_data_type { > > extern struct annotated_data_type unknown_type; > extern struct annotated_data_type stackop_type; > +extern struct annotated_data_type canary_type; > > /** > * struct data_loc_info - Data location information > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index 5f3136f57c62..f2683dadf3cf 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -116,6 +116,13 @@ struct annotated_data_type stackop_type = { > }, > }; > > +struct annotated_data_type canary_type = { > + .self = { > + .type_name = (char *)"(stack canary)", > + .children = LIST_HEAD_INIT(canary_type.self.children), > + }, > +}; > + > static int arch__grow_instructions(struct arch *arch) > { > struct ins *new_instructions; > @@ -3764,6 +3771,17 @@ static bool is_stack_operation(struct arch *arch, struct disasm_line *dl) > return false; > } > > +static bool is_stack_canary(struct arch *arch, struct annotated_op_loc *loc) > +{ > + /* On x86_64, %gs:40 is used for stack canary */ > + if (arch__is(arch, "x86")) { > + if (loc->segment == INSN_SEG_X86_GS && loc->offset == 40) > + return true; > + } > + > + return false; > +} > + > u64 annotate_calc_pcrel(struct map_symbol *ms, u64 ip, int offset, > struct disasm_line *dl) > { > @@ -3938,6 +3956,12 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he) > } > > mem_type = find_data_type(&dloc); > + > + if (mem_type == NULL && is_stack_canary(arch, op_loc)) { > + mem_type = &canary_type; > + dloc.type_offset = 0; > + } > + > if (mem_type) > istat->good++; > else > -- > 2.43.0.594.gd9cf4e227d-goog >
On Fri, Feb 2, 2024 at 7:21 PM Ian Rogers <irogers@google.com> wrote: > > On Fri, Feb 2, 2024 at 2:05 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > When the stack protector is enabled, compiler would generate code to > > check stack overflow with a special value called 'stack carary' at > > runtime. On x86_64, GCC hard-codes the stack canary as %gs:40. > > > > While there's a definition of fixed_percpu_data in asm/processor.h, > > it seems that the header is not included everywhere and many places > > it cannot find the type info. As it's in the well-known location (at > > %gs:40), let's add a pseudo stack canary type to handle it specially. > > I wonder if cases like this can be handled by debug info rather than > special cases in the tool. Special cases are fine too, but are > potentially less portable. Agreed, but I couldn't find anything special in DWARF. Thanks, Namhyung
On Tue, Feb 6, 2024 at 3:19 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Fri, Feb 2, 2024 at 7:21 PM Ian Rogers <irogers@google.com> wrote: > > > > On Fri, Feb 2, 2024 at 2:05 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > When the stack protector is enabled, compiler would generate code to > > > check stack overflow with a special value called 'stack carary' at > > > runtime. On x86_64, GCC hard-codes the stack canary as %gs:40. > > > > > > While there's a definition of fixed_percpu_data in asm/processor.h, > > > it seems that the header is not included everywhere and many places > > > it cannot find the type info. As it's in the well-known location (at > > > %gs:40), let's add a pseudo stack canary type to handle it specially. > > > > I wonder if cases like this can be handled by debug info rather than > > special cases in the tool. Special cases are fine too, but are > > potentially less portable. > > Agreed, but I couldn't find anything special in DWARF. The fs and gs selectors are commonly used for thread local storage, so could something like DW_OP_form_tls_address be used? https://dwarfstd.org/issues/110803.1.html Thanks, Ian > Thanks, > Namhyung
On Tue, Feb 6, 2024 at 3:40 PM Ian Rogers <irogers@google.com> wrote: > > On Tue, Feb 6, 2024 at 3:19 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > On Fri, Feb 2, 2024 at 7:21 PM Ian Rogers <irogers@google.com> wrote: > > > > > > On Fri, Feb 2, 2024 at 2:05 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > > > When the stack protector is enabled, compiler would generate code to > > > > check stack overflow with a special value called 'stack carary' at > > > > runtime. On x86_64, GCC hard-codes the stack canary as %gs:40. > > > > > > > > While there's a definition of fixed_percpu_data in asm/processor.h, > > > > it seems that the header is not included everywhere and many places > > > > it cannot find the type info. As it's in the well-known location (at > > > > %gs:40), let's add a pseudo stack canary type to handle it specially. > > > > > > I wonder if cases like this can be handled by debug info rather than > > > special cases in the tool. Special cases are fine too, but are > > > potentially less portable. > > > > Agreed, but I couldn't find anything special in DWARF. > > The fs and gs selectors are commonly used for thread local storage, so > could something like DW_OP_form_tls_address be used? > https://dwarfstd.org/issues/110803.1.html I'm not sure if it's the same thing. Maybe it's for variables with __thread annotation. I don't know if compilers generate a (pseudo) variable for stack canary. Thanks, Namhyung
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h index 0bfef29fa52c..e293980eb11b 100644 --- a/tools/perf/util/annotate-data.h +++ b/tools/perf/util/annotate-data.h @@ -77,6 +77,7 @@ struct annotated_data_type { extern struct annotated_data_type unknown_type; extern struct annotated_data_type stackop_type; +extern struct annotated_data_type canary_type; /** * struct data_loc_info - Data location information diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 5f3136f57c62..f2683dadf3cf 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -116,6 +116,13 @@ struct annotated_data_type stackop_type = { }, }; +struct annotated_data_type canary_type = { + .self = { + .type_name = (char *)"(stack canary)", + .children = LIST_HEAD_INIT(canary_type.self.children), + }, +}; + static int arch__grow_instructions(struct arch *arch) { struct ins *new_instructions; @@ -3764,6 +3771,17 @@ static bool is_stack_operation(struct arch *arch, struct disasm_line *dl) return false; } +static bool is_stack_canary(struct arch *arch, struct annotated_op_loc *loc) +{ + /* On x86_64, %gs:40 is used for stack canary */ + if (arch__is(arch, "x86")) { + if (loc->segment == INSN_SEG_X86_GS && loc->offset == 40) + return true; + } + + return false; +} + u64 annotate_calc_pcrel(struct map_symbol *ms, u64 ip, int offset, struct disasm_line *dl) { @@ -3938,6 +3956,12 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he) } mem_type = find_data_type(&dloc); + + if (mem_type == NULL && is_stack_canary(arch, op_loc)) { + mem_type = &canary_type; + dloc.type_offset = 0; + } + if (mem_type) istat->good++; else
When the stack protector is enabled, compiler would generate code to check stack overflow with a special value called 'stack carary' at runtime. On x86_64, GCC hard-codes the stack canary as %gs:40. While there's a definition of fixed_percpu_data in asm/processor.h, it seems that the header is not included everywhere and many places it cannot find the type info. As it's in the well-known location (at %gs:40), let's add a pseudo stack canary type to handle it specially. Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/annotate-data.h | 1 + tools/perf/util/annotate.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+)