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

[Steve Clevenger]

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 at 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 at 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
>>