Received-SPF: pass (google.com: domain of linux-kernel+bounces-50891-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240202220459.527138-1-namhyung@kernel.org> <20240202220459.527138-15-namhyung@kernel.org>
In-Reply-To: <20240202220459.527138-15-namhyung@kernel.org>
From: Ian Rogers <irogers@google.com>
Date: Fri, 2 Feb 2024 19:21:09 -0800
Message-ID: <CAP-5=fUWaKW7d6_YkET0o26=fjBwX6PPJ1gXQ9wndQM_Oi2O3A@mail.gmail.com>
Subject: Re: [PATCH 14/14] perf annotate-data: Add stack canary type
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>, Jiri Olsa <jolsa@kernel.org>, 
	Adrian Hunter <adrian.hunter@intel.com>, Peter Zijlstra <peterz@infradead.org>, 
	Ingo Molnar <mingo@kernel.org>, LKML <linux-kernel@vger.kernel.org>, 
	linux-perf-users@vger.kernel.org, 
	Linus Torvalds <torvalds@linux-foundation.org>, Stephane Eranian <eranian@google.com>, 
	Masami Hiramatsu <mhiramat@kernel.org>, linux-toolchains@vger.kernel.org, 
	linux-trace-devel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 2, 2024 at 2:05=E2=80=AFPM Namhyung Kim <namhyung@kernel.org> w=
rote:
>
> 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-d=
ata.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 =3D {
>         },
>  };
>
> +struct annotated_data_type canary_type =3D {
> +       .self =3D {
> +               .type_name =3D (char *)"(stack canary)",
> +               .children =3D 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 =3D=3D INSN_SEG_X86_GS && loc->offset =
=3D=3D 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_t=
ype(struct hist_entry *he)
>                 }
>
>                 mem_type =3D find_data_type(&dloc);
> +
> +               if (mem_type =3D=3D NULL && is_stack_canary(arch, op_loc)=
) {
> +                       mem_type =3D &canary_type;
> +                       dloc.type_offset =3D 0;
> +               }
> +
>                 if (mem_type)
>                         istat->good++;
>                 else
> --
> 2.43.0.594.gd9cf4e227d-goog
>