[14/14] perf annotate-data: Add stack canary type

Message ID 20240202220459.527138-15-namhyung@kernel.org
State New
Headers
Series perf tools: Remaining bits of data type profiling (v5) |

Commit Message

Namhyung Kim Feb. 2, 2024, 10:04 p.m. UTC
  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(+)
  

Comments

Ian Rogers Feb. 3, 2024, 3:21 a.m. UTC | #1
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
>
  
Namhyung Kim Feb. 6, 2024, 11:18 p.m. UTC | #2
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
  
Ian Rogers Feb. 6, 2024, 11:40 p.m. UTC | #3
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
  
Namhyung Kim Feb. 7, 2024, 7:08 p.m. UTC | #4
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
  

Patch

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