Re: [PATCH V4 1/4] Add dso__is_pie call to identify ELF PIE

On 8/28/2024 1:23 AM, Leo Yan wrote:

On 8/28/2024 6:09 AM, Steve Clevenger wrote:

...

Add dso__is_pie global to read the .dynamic section DT_FLAGS_1 entry for the DF_1_PIE flag. This identifies position executable code.

Signed-off-by: Steve Clevenger scclevenger@os.amperecomputing.com

tools/perf/util/symbol-elf.c | 60 ++++++++++++++++++++++++++++++++++++ tools/perf/util/symbol.h | 1 + 2 files changed, 61 insertions(+)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index e398abfd13a0..0e49c1345a67 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -662,6 +662,66 @@ static int dso__synthesize_plt_got_symbols(struct dso *dso, Elf *elf, return err; }

+/*

• • Check dynamic section DT_FLAGS_1 for a Position Independent
• • Executable (PIE).
• */

+bool dso__is_pie(struct dso *dso) +{

•   Elf *elf = NULL;
  

•   Elf_Scn *scn = NULL;
  

•   GElf_Ehdr ehdr;
  

•   GElf_Shdr shdr;
  

•   bool is_pie = false;
  

•   char dso_path[PATH_MAX];
  

•   int fd = -1;
  

•   if (!dso || (elf_version(EV_CURRENT) == EV_NONE))
  

•           goto exit;      // false
  

•   dso__build_id_filename(dso, dso_path, sizeof(dso_path), false);
  

•   fd = open(dso_path, O_RDONLY);
  

•   if (fd < 0) {
  

•           pr_debug("%s: cannot read cached %s.\n", __func__, dso_path);
  

•           goto exit;      // false
  

•   }
  

•   elf = elf_begin(fd, ELF_C_READ, NULL);
  

•   gelf_getehdr(elf, &ehdr);
  

•   if (ehdr.e_type == ET_DYN) {
  

•           Elf_Data *data;
  

•           GElf_Dyn *entry;
  

•           int n_entries = shdr.sh_size / sizeof(GElf_Dyn);
  

•           scn = elf_section_by_name(elf, &ehdr, &shdr, ".dynamic", NULL);
  

•           if (!scn)
  

•                   goto exit;      // false
  

It should goto a new tag (e.g. exit_failed) so can invoke elf_end(elf) and close(fd).

...

•           data = (Elf_Data *) elf_getdata(scn, NULL);
  

•           if (!data || !data->d_buf)
  

•                   goto exit;      // false
  

Ditto.

...

•           // check DT_FLAGS_1
  

•           for (int i = 0; i < n_entries; i++) {
  

•                   entry = ((GElf_Dyn *) data->d_buf) + i;
  

•                   if (entry->d_tag == DT_FLAGS_1) {
  

•                           if ((entry->d_un.d_val & DF_1_PIE) != 0) {
  

•                                   is_pie = true;
  

•                                   break;
  

•                           }
  

•                   }
  

•           } // end for
  

•   }
  

Put 'exit_failed' tag at here.

With the changes:

Reviewed-by: Leo Yan leo.yan@arm.com

...

•   elf_end(elf);
  

•   close(fd);
  

+exit:

•   return is_pie;
  

+}

Hi Leo,

It's been a long time since I've seen goto's used as a rule rather than an exception. I see it introduced the problem you've mentioned. Is it ok to simply update the cover letter (patch 0), and this one (patch 1) as V5? If I don't hear from you, I'll resubmit all.

Thanks, Steve

...

/*

• We need to check if we have a .dynsym, so that we can handle the
• .plt, synthesizing its symbols, that aren't on the symtabs (be it

diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h index 3fb5d146d9b1..33ea2596ce31 100644 --- a/tools/perf/util/symbol.h +++ b/tools/perf/util/symbol.h @@ -127,6 +127,7 @@ void dso__insert_symbol(struct dso *dso, struct symbol *sym); void dso__delete_symbol(struct dso *dso, struct symbol *sym); +bool dso__is_pie(struct dso *dso);

struct symbol *dso__find_symbol(struct dso *dso, u64 addr); struct symbol *dso__find_symbol_nocache(struct dso *dso, u64 addr); -- 2.25.1