Page MenuHomePhabricator

[AARCH64][ELF][llvm-readobj] Add support for BTI and PAC dynamic tags
ClosedPublic

Authored by peter.smith on May 29 2019, 8:06 AM.

Details

Summary

ELF for the 64-bit Arm Architecture defines two processor-specific dynamic tags:

  • DT_AARCH64_BTI_PLT 0x70000001, d_val
  • DT_AARCH64_PAC_PLT 0x70000003, d_val

These presence of these tags indicate that PLT sequences have been protected using Branch Target Identification and Pointer Authentication respectively. The presence of both indicates that the PLT sequences have been protected with both Branch Target Identification and Pointer Authentication.

This patch adds the tags and tests for llvm-readobj and yaml2obj.

ELF for the 64-bit Arm Architecture containing definitions of the tags can be found at: https://developer.arm.com/docs/ihi0056/latest/elf-for-the-arm-64-bit-architecture-aarch64-abi-2019q1-documentation

Diff Detail

Repository
rL LLVM

Event Timeline

peter.smith created this revision.May 29 2019, 8:06 AM
ostannard added inline comments.
test/tools/llvm-readobj/aarch64-bti-pac-dynamic.test
1 ↗(On Diff #201917)

Copy-pasted comment?

15 ↗(On Diff #201917)

Why is the value for PAC printed as "0x0", but BTI is just "0"?

grimar added inline comments.May 30 2019, 2:24 AM
test/tools/llvm-readobj/aarch64-bti-pac-dynamic.test
1 ↗(On Diff #201917)

It is becoming common to start comments from ## in LLVM tools.
(see dynamic-section-arch-tags.test below, for example).

3 ↗(On Diff #201917)

Lets use yaml2obj instead of precompiled binary.

15 ↗(On Diff #201917)

FWIW it is not an issue of this patch it seems.

23 ↗(On Diff #201917)

You can probably omit dumping of other tags.

tools/llvm-readobj/ELFDumper.cpp
1466 ↗(On Diff #201917)

Excessive empty line.

peter.smith marked an inline comment as done.May 30 2019, 2:31 AM
peter.smith added inline comments.
test/tools/llvm-readobj/aarch64-bti-pac-dynamic.test
1 ↗(On Diff #201917)

Yes will fix, thanks for the spot.

15 ↗(On Diff #201917)

That is interesting, in many places in llvm-readelf the architecture specific tags are separated from each other, but in the case of printDynamicEntry they are not. So the AArch64 tags overlap with MIPS_RLD_VERSION and MIPS_ICHECKSUM. In this particular case this doesn't really matter as the value of the tag isn't important, but it may do in the future. To fix this properly will need some similar logic to isolate the cases by processor. I'll have a think.

peter.smith marked 3 inline comments as done.May 30 2019, 2:32 AM

Thanks for the comments, I hope to have an update later today.

test/tools/llvm-readobj/aarch64-bti-pac-dynamic.test
1 ↗(On Diff #201917)

Thanks, will do.

3 ↗(On Diff #201917)

Ok, will do so.

23 ↗(On Diff #201917)

Ok will do.

LukeCheeseman added inline comments.May 30 2019, 2:56 AM
test/tools/llvm-readobj/aarch64-bti-pac-dynamic.test
15 ↗(On Diff #201917)

Sounds like a similar issue to https://reviews.llvm.org/D50136. The overlapping aarch64 and sparc tags meant that the dumped attribute always came out as sparc attribute. It was arguably more of an issue there as it would dump an unrelated attribute. I wasn't too happy with the solution but there wasn't/isn't any support for target specific dumping.

Updated diff to use yaml2obj. It turns out there was already a processor specific dynamic tags test so I've added to that instead of writing a new one. I've taken the opportunity to separate out the printing of values for [DT_LOPROC, DT_AUXILIARY) so that the types don't clash.

grimar added inline comments.May 31 2019, 1:10 AM
tools/llvm-readobj/ELFDumper.cpp
1842 ↗(On Diff #202187)

I'd suggest to make the code more explicit and isolated from the switch below:

switch (ObjF->getELFFile()->getHeader()->e_machine) {
case EM_AARCH64:
  switch (Type) {
  case DT_AARCH64_BTI_PLT:
  case DT_AARCH64_PAC_PLT:
    OS << Value;
    return;
  default:
    break;
  }
case EM_HEXAGON:
  switch (Type) {
  case DT_HEXAGON_VER:
    OS << Value;
    return;
  case DT_HEXAGON_SYMSZ:
  case DT_HEXAGON_PLT:
    OS << format(ConvChar, Value);
    return;
  default:
    break;
  }
case EM_MIPS:
  switch (Type) {
  case DT_MIPS_RLD_VERSION:
  case DT_MIPS_LOCAL_GOTNO:
  case DT_MIPS_SYMTABNO:
  case DT_MIPS_UNREFEXTNO:
    <place missing mips tags here>
    OS << Value;
    return;
  case DT_MIPS_FLAGS:
    <and here (and seems one more case is also needed)>
    printFlags(Value, makeArrayRef(ElfDynamicDTMipsFlags), OS);
    return;
  default:
    break;
  }
default:
  break;
}

I.e. idea is that we try to handle target specific flags first and either return or fall back to the switch for
common tags below if flag was not handled.
No need to do checks like Type >= DT_LOPROC && Type < DT_AUXILIARY or have any printing logic in default: case I think.

Thanks for the suggestion, I'll update the patch on Monday.

I've updated printDynamicEntry as suggested. There are an awful lot of MIPS tags! I've kept them with the same value as before, although I'd think that the NO tags would probably best be output as decimal. I'll leave that decision to the MIPS maintainers.

grimar accepted this revision.Jun 4 2019, 1:11 AM

LGTM

This revision is now accepted and ready to land.Jun 4 2019, 1:11 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2019, 4:41 AM