[ARM] Rewrite ARMAttributeParser

[MaskRay]

Summary

• Delete boilerplate
• Change functions to return Error
• Test parsing errors
• Update callers of ARMAttributeParser::parse() to check the Error return value.

Since this patch touches nearly everything in the file, I apply
http://llvm.org/docs/Proposals/VariableNames.html and change variable
names to lower case.

[MaskRay]

The format of .ARM.attributes can be found at https://developer.arm.com/docs/ihi0044/h/elf-for-the-arm-architecture-abi-2019q1-documentation

In BFD, some other architectures define elf_backend_obj_attrs_vendor: bfd/elf32-msp430.c bfd/elf32-tic6x.c bfd/elf32-arc.c bfd/elfnn-riscv.c.

[MaskRay]

Fix two tests

[samparker]

This is a large change and converting to lower case is making really hard to discern what the functional changes are.

[MaskRay]

In D75015#1888821, @samparker wrote:

This is a large change and converting to lower case is making really hard to discern what the functional changes are.

Pasted my reply in D74023.

For existing code, there is objection that clean-ups can clutter up the history. There is a supportive group of people putting forward a migration plan. For *AttributeParser, I think it is a middle ground: the rewrite (refactoring) will touch nearly every line (we will also face a large function move ARMAttributeParser.cpp -> ELFAttributeParser.cpp). I am in the camp of "I don't want to unnecessarily break the style of surrounding code, but if it is new code, I want to just use the ideal style." So I went forward and fixed the variable Naming in D75015.

[grimar]

In D75015#1888821, @samparker wrote:

This is a large change and converting to lower case is making really hard to discern what the functional changes are.

+1. It does not feel right that mixing of code style and functional changes happens.

llvm/lib/Support/ARMAttributeParser.cpp

500

Seems it can be:

if (sw) {
  DictScope scope(*sw, scopeName);
  if (!indicies.empty())
    sw->printList(indexName, indicies);
} 

if (Error e = parseAttributeList(size))
  return e;

llvm/tools/llvm-readobj/ELFDumper.cpp

2693–2695

TODO:

2696

Excessive curly braces around a single line.

[MaskRay]

llvm/lib/Support/ARMAttributeParser.cpp

500

It can't.

DictScope changes the indentation level. parseAttributeList must be called with DictScope in the scope.

[MaskRay]

TODO -> TODO:
Delete an excess pair of braces

[MaskRay]

invalid CPU_arch value, expected to be inside [0,22)
->
unknown CPU_arch value: 22

[compnerd]

lld/test/ELF/arm-tag-vfp-args-illegal.s

5

I think that the original error message was better here

llvm/include/llvm/Support/ARMAttributeParser.h

42

A newline before this would be nice to delimit the general handler vs the known attribute list.

78

It might be nice to use a XMACRO here. But thats not critical/important.

82

Would be nice to group this with the other general handlers.

87

static_cast<void> please.

llvm/lib/Support/ARMAttributeParser.cpp

67

Please put a trailing comma after the nodefaults handler and re-clang-format.

401

I think that the original code was clearer here. I think at the very least, we should have a comment explaining this offset.

416

Bleh, could you please check if we have a constant for this yet? IIRC, we didn't have one when I wrote this, but really this is the limit of the namespace and should really have a constant.

446

Again, I think that the original code was more readable here. Please use sizeof(length) or add a comment explaining the 4.

[MaskRay]

Address comments

[MaskRay]

Fix an lld test

[MaskRay]

llvm/lib/Support/ARMAttributeParser.cpp

401

Moved - 5 to the caller.

416

Will do afterwards.

446

Moved - 4 to the caller. I think it will be more readable.

lld/ELF/InputFiles.cpp

InputSectionBase *ObjFile<ELFT>::createInputSection(const Elf_Shdr &sec) {
   StringRef name = getSectionName(sec);
 
   switch (sec.sh_type) {
   case SHT_ARM_ATTRIBUTES: {
     if (config->emachine != EM_ARM)
       break;
     ARMAttributeParser attributes;
     ArrayRef<uint8_t> contents = check(this->getObj().getSectionContents(&sec));
-    attributes.parse(contents, config->ekind == ELF32LEKind ? support::little
-                                                            : support::big);
+    if (Error e = attributes.parse(contents, config->ekind == ELF32LEKind
+                                                 ? support::little
+                                                 : support::big)) {
+      auto *isec = make<InputSection>(*this, sec, name);
+      warn(toString(isec) + ": " + llvm::toString(std::move(e)));
+      break;
+    }
     updateSupportedARMFeatures(attributes);
     updateARMVFPArgs(attributes, this);
 
     // FIXME: Retain the first attribute section we see. The eglibc ARM
     // dynamic loaders require the presence of an attribute section for dlopen
     // to work. In a full implementation we would merge all attribute sections.
     if (in.armAttributes == nullptr) {
       in.armAttributes = make<InputSection>(*this, sec, name);

lld/test/ELF/arm-tag-vfp-args-illegal.s

 // REQUIRES:arm
 // RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %s -o %t.o
-// RUN: not ld.lld %t.o -o /dev/null 2>&1 | FileCheck %s
+// RUN: ld.lld %t.o -o /dev/null 2>&1 | FileCheck %s
 
-// CHECK: arm-tag-vfp-args-illegal.s.tmp.o: unknown Tag_ABI_VFP_args value: 5
+// CHECK: {{.*}}.o:(.ARM.attributes): unknown ABI_VFP_args value: 5
         .arch armv7-a
         .eabi_attribute 20, 1
         .eabi_attribute 21, 1
         .eabi_attribute 23, 3
         .eabi_attribute 24, 1
         .eabi_attribute 25, 1
         .eabi_attribute 26, 2
         .eabi_attribute 30, 6
         .eabi_attribute 34, 1
         .eabi_attribute 18, 4
         .eabi_attribute 28, 5 // Tag_ABI_VFP_args = 5 (Illegal value)
 
         .syntax unified
         .globl _start
         .type _start, %function
 _start: bx lr

lld/test/ELF/invalid/bad-arm-attributes.s

 # REQUIRES: arm
-# RUN: llvm-mc -filetype=obj -triple=arm-unknown-linux %s -o %t
-# RUN: ld.lld %t -o /dev/null 2>&1 | FileCheck %s
+# RUN: llvm-mc -filetype=obj -triple=arm-unknown-linux %s -o %t.o
+# RUN: ld.lld %t.o -o /dev/null 2>&1 | FileCheck %s
 
-# CHECK: invalid subsection length 0 at offset 1
+# CHECK: {{.*}}.o:(.ARM.attributes): unrecognized format-version: 0x0
 
 .section .ARM.attributes,"a",%0x70000003
   .quad 0

lld/test/ELF/invalid/bad-arm-attributes2.s

-# REQUIRES: arm
-# RUN: llvm-mc -filetype=obj -triple=arm-unknown-linux %s -o %t
-# RUN: ld.lld %t -o /dev/null 2>&1 | FileCheck %s
-
-# CHECK: invalid subsection length 4294967295 at offset 1
-
-.section .ARM.attributes,"a",%0x70000003
-  .byte 0, 0xFF, 0xFF, 0xFF, 0xFF

llvm/include/llvm/Object/ELFObjectFile.h

for (const Elf_Shdr &Sec : *SectionsOrErr) {
         auto ErrorOrContents = EF.getSectionContents(&Sec);
         if (!ErrorOrContents)
           return ErrorOrContents.takeError();
 
         auto Contents = ErrorOrContents.get();
         if (Contents[0] != ARMBuildAttrs::Format_Version || Contents.size() == 1)
           return Error::success();
 
-        Attributes.parse(Contents, ELFT::TargetEndianness);
+        if (Error E = Attributes.parse(Contents, ELFT::TargetEndianness))
+          return E;
         break;
       }
     }
     return Error::success();
   }
 
   // This flag is used for classof, to distinguish ELFObjectFile from
   // its subclass. If more subclasses will be created, this flag will

llvm/include/llvm/Support/ARMAttributeParser.h

 //===--- ARMAttributeParser.h - ARM Attribute Information Printer ---------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_ARMATTRIBUTEPARSER_H
 #define LLVM_SUPPORT_ARMATTRIBUTEPARSER_H
 
 #include "ARMBuildAttributes.h"
 #include "ScopedPrinter.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/DataExtractor.h"
 #include "llvm/Support/Endian.h"
+#include "llvm/Support/Error.h"
 
 #include <map>
 
 namespace llvm {
 class StringRef;
 
 class ARMAttributeParser {
-  ScopedPrinter *SW;
+  ScopedPrinter *sw;
 
-  std::map<unsigned, unsigned> Attributes;
+  std::map<unsigned, unsigned> attributes;
+  DataExtractor de{ArrayRef<uint8_t>{}, true, 0};
+  DataExtractor::Cursor cursor{0};
 
   struct DisplayHandler {
-    ARMBuildAttrs::AttrType Attribute;
-    void (ARMAttributeParser::*Routine)(ARMBuildAttrs::AttrType,
-                                        const uint8_t *, uint32_t &);
+    ARMBuildAttrs::AttrType attribute;
+    Error (ARMAttributeParser::*routine)(ARMBuildAttrs::AttrType);
   };
-  static const DisplayHandler DisplayRoutines[];
+  static const DisplayHandler displayRoutines[];
 
-  uint64_t ParseInteger(const uint8_t *Data, uint32_t &Offset);
-  StringRef ParseString(const uint8_t *Data, uint32_t &Offset);
+  Error parseStringAttribute(const char *name, ARMBuildAttrs::AttrType tag,
+                             const ArrayRef<const char *> array);
+  void printAttribute(unsigned tag, unsigned value, StringRef valueDesc);
+
+  Error stringAttribute(ARMBuildAttrs::AttrType tag);
+  Error CPU_arch(ARMBuildAttrs::AttrType tag);
+  Error CPU_arch_profile(ARMBuildAttrs::AttrType tag);
+  Error ARM_ISA_use(ARMBuildAttrs::AttrType tag);
+  Error THUMB_ISA_use(ARMBuildAttrs::AttrType tag);
+  Error FP_arch(ARMBuildAttrs::AttrType tag);
+  Error WMMX_arch(ARMBuildAttrs::AttrType tag);
+  Error Advanced_SIMD_arch(ARMBuildAttrs::AttrType tag);
+  Error MVE_arch(ARMBuildAttrs::AttrType tag);
+  Error PCS_config(ARMBuildAttrs::AttrType tag);
+  Error ABI_PCS_R9_use(ARMBuildAttrs::AttrType tag);
+  Error ABI_PCS_RW_data(ARMBuildAttrs::AttrType tag);
+  Error ABI_PCS_RO_data(ARMBuildAttrs::AttrType tag);
+  Error ABI_PCS_GOT_use(ARMBuildAttrs::AttrType tag);
+  Error ABI_PCS_wchar_t(ARMBuildAttrs::AttrType tag);
+  Error ABI_FP_rounding(ARMBuildAttrs::AttrType tag);
+  Error ABI_FP_denormal(ARMBuildAttrs::AttrType tag);
+  Error ABI_FP_exceptions(ARMBuildAttrs::AttrType tag);
+  Error ABI_FP_user_exceptions(ARMBuildAttrs::AttrType tag);
+  Error ABI_FP_number_model(ARMBuildAttrs::AttrType tag);
+  Error ABI_align_needed(ARMBuildAttrs::AttrType tag);
+  Error ABI_align_preserved(ARMBuildAttrs::AttrType tag);
+  Error ABI_enum_size(ARMBuildAttrs::AttrType tag);
+  Error ABI_HardFP_use(ARMBuildAttrs::AttrType tag);
+  Error ABI_VFP_args(ARMBuildAttrs::AttrType tag);
+  Error ABI_WMMX_args(ARMBuildAttrs::AttrType tag);
+  Error ABI_optimization_goals(ARMBuildAttrs::AttrType tag);
+  Error ABI_FP_optimization_goals(ARMBuildAttrs::AttrType tag);
+  Error compatibility(ARMBuildAttrs::AttrType tag);
+  Error CPU_unaligned_access(ARMBuildAttrs::AttrType tag);
+  Error FP_HP_extension(ARMBuildAttrs::AttrType tag);
+  Error ABI_FP_16bit_format(ARMBuildAttrs::AttrType tag);
+  Error MPextension_use(ARMBuildAttrs::AttrType tag);
+  Error DIV_use(ARMBuildAttrs::AttrType tag);
+  Error DSP_extension(ARMBuildAttrs::AttrType tag);
+  Error T2EE_use(ARMBuildAttrs::AttrType tag);
+  Error Virtualization_use(ARMBuildAttrs::AttrType tag);
+  Error nodefaults(ARMBuildAttrs::AttrType tag);
+
+  Error parseAttributeList(uint32_t length);
+  void parseIndexList(SmallVectorImpl<uint8_t> &indexList);
+  Error parseSubsection(uint32_t length);
 
-  void IntegerAttribute(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                        uint32_t &Offset);
-  void StringAttribute(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                       uint32_t &Offset);
-
-  void PrintAttribute(unsigned Tag, unsigned Value, StringRef ValueDesc);
-
-  void CPU_arch(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                uint32_t &Offset);
-  void CPU_arch_profile(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                        uint32_t &Offset);
-  void ARM_ISA_use(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                   uint32_t &Offset);
-  void THUMB_ISA_use(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                     uint32_t &Offset);
-  void FP_arch(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-               uint32_t &Offset);
-  void WMMX_arch(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                 uint32_t &Offset);
-  void Advanced_SIMD_arch(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                          uint32_t &Offset);
-  void MVE_arch(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                uint32_t &Offset);
-  void PCS_config(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                  uint32_t &Offset);
-  void ABI_PCS_R9_use(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                      uint32_t &Offset);
-  void ABI_PCS_RW_data(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                       uint32_t &Offset);
-  void ABI_PCS_RO_data(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                       uint32_t &Offset);
-  void ABI_PCS_GOT_use(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                       uint32_t &Offset);
-  void ABI_PCS_wchar_t(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                       uint32_t &Offset);
-  void ABI_FP_rounding(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                       uint32_t &Offset);
-  void ABI_FP_denormal(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                       uint32_t &Offset);
-  void ABI_FP_exceptions(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                         uint32_t &Offset);
-  void ABI_FP_user_exceptions(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                              uint32_t &Offset);
-  void ABI_FP_number_model(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                           uint32_t &Offset);
-  void ABI_align_needed(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                        uint32_t &Offset);
-  void ABI_align_preserved(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                           uint32_t &Offset);
-  void ABI_enum_size(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                     uint32_t &Offset);
-  void ABI_HardFP_use(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                      uint32_t &Offset);
-  void ABI_VFP_args(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                    uint32_t &Offset);
-  void ABI_WMMX_args(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                     uint32_t &Offset);
-  void ABI_optimization_goals(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                              uint32_t &Offset);
-  void ABI_FP_optimization_goals(ARMBuildAttrs::AttrType Tag,
-                                 const uint8_t *Data, uint32_t &Offset);
-  void compatibility(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                     uint32_t &Offset);
-  void CPU_unaligned_access(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                            uint32_t &Offset);
-  void FP_HP_extension(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                       uint32_t &Offset);
-  void ABI_FP_16bit_format(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                           uint32_t &Offset);
-  void MPextension_use(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                       uint32_t &Offset);
-  void DIV_use(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-               uint32_t &Offset);
-  void DSP_extension(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                     uint32_t &Offset);
-  void T2EE_use(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                uint32_t &Offset);
-  void Virtualization_use(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                          uint32_t &Offset);
-  void nodefaults(ARMBuildAttrs::AttrType Tag, const uint8_t *Data,
-                  uint32_t &Offset);
-
-  void ParseAttributeList(const uint8_t *Data, uint32_t &Offset,
-                          uint32_t Length);
-  void ParseIndexList(const uint8_t *Data, uint32_t &Offset,
-                      SmallVectorImpl<uint8_t> &IndexList);
-  void ParseSubsection(const uint8_t *Data, uint32_t Length);
 public:
-  ARMAttributeParser(ScopedPrinter *SW) : SW(SW) {}
+  ARMAttributeParser(ScopedPrinter *sw) : sw(sw) {}
+  ARMAttributeParser() : sw(nullptr) {}
+  ~ARMAttributeParser() { (void)!cursor.takeError(); }
 
-  ARMAttributeParser() : SW(nullptr) { }
+  Error parse(ArrayRef<uint8_t> section, support::endianness endian);
 
-  void parse(ArrayRef<uint8_t> Section, support::endianness E);
+  bool hasAttribute(unsigned tag) const { return attributes.count(tag); }
 
-  bool hasAttribute(unsigned Tag) const {
-    return Attributes.count(Tag);
-  }
-
-  unsigned getAttributeValue(unsigned Tag) const {
-    return Attributes.find(Tag)->second;
+  unsigned getAttributeValue(unsigned tag) const {
+    return attributes.find(tag)->second;
   }
 };
 
 }
 
 #endif
 

llvm/include/llvm/Support/DataExtractor.h

public:
   ///     data to extract all of the bytes, the offset will be left unmodified.
   ///
   /// \return
   ///     A StringRef for the extracted bytes. If the offset pointed to by
   ///     \a OffsetPtr is out of bounds, or if the offset plus the length
   ///     is out of bounds, a default-initialized StringRef will be returned.
   StringRef getBytes(uint64_t *OffsetPtr, uint64_t Length) const;
 
+  StringRef getCStrRef(Cursor &C) const { return getCStrRef(&C.Offset); }
+
   /// Extract an unsigned integer of size \a byte_size from \a
   /// *offset_ptr.
   ///
   /// Extract a single unsigned integer value and update the offset
   /// pointed to by \a offset_ptr. The size of the extracted integer
   /// is specified by the \a byte_size argument. \a byte_size should
   /// have a value greater than or equal to one and less than or equal
   /// to eight since the return value is 64 bits wide. Any

llvm/lib/Object/ELFObjectFile.cpp

if (PlatformFlags & ELF::EF_MIPS_MICROMIPS)
     Features.AddFeature("micromips");
 
   return Features;
 }
 
 SubtargetFeatures ELFObjectFileBase::getARMFeatures() const {
   SubtargetFeatures Features;
   ARMAttributeParser Attributes;
-  if (Error E = getBuildAttributes(Attributes))
+  if (Error E = getBuildAttributes(Attributes)) {
+    consumeError(std::move(E));
     return SubtargetFeatures();
+  }
 
   // both ARMv7-M and R have to support thumb hardware div
   bool isV7 = false;
   if (Attributes.hasAttribute(ARMBuildAttrs::CPU_arch))
     isV7 = Attributes.getAttributeValue(ARMBuildAttrs::CPU_arch)
       == ARMBuildAttrs::v7;
 
   if (Attributes.hasAttribute(ARMBuildAttrs::CPU_arch_profile)) {

 }
 
 // FIXME Encode from a tablegen description or target parser.
 void ELFObjectFileBase::setARMSubArch(Triple &TheTriple) const {
   if (TheTriple.getSubArch() != Triple::NoSubArch)
     return;
 
   ARMAttributeParser Attributes;
-  if (Error E = getBuildAttributes(Attributes))
+  if (Error E = getBuildAttributes(Attributes)) {
+    // TODO Propagate Error.
+    consumeError(std::move(E));
     return;
+  }
 
   std::string Triple;
   // Default to ARM, but use the triple if it's been set.
   if (TheTriple.isThumb())
     Triple = "thumb";
   else
     Triple = "arm";
 

llvm/lib/Support/ARMAttributeParser.cpp

 //===--- ARMAttributeParser.cpp - ARM Attribute Information Printer -------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/ARMAttributeParser.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 using namespace llvm;
 using namespace llvm::ARMBuildAttrs;
 
-
-static const EnumEntry<unsigned> TagNames[] = {
-  { "Tag_File", ARMBuildAttrs::File },
-  { "Tag_Section", ARMBuildAttrs::Section },
-  { "Tag_Symbol", ARMBuildAttrs::Symbol },
+static const EnumEntry<unsigned> tagNames[] = {
+    {"Tag_File", ARMBuildAttrs::File},
+    {"Tag_Section", ARMBuildAttrs::Section},
+    {"Tag_Symbol", ARMBuildAttrs::Symbol},
 };
 
-namespace llvm {
-#define ATTRIBUTE_HANDLER(Attr_)                                                \
-  { ARMBuildAttrs::Attr_, &ARMAttributeParser::Attr_ }
-
-const ARMAttributeParser::DisplayHandler
-ARMAttributeParser::DisplayRoutines[] = {
-  { ARMBuildAttrs::CPU_raw_name, &ARMAttributeParser::StringAttribute, },
-  { ARMBuildAttrs::CPU_name, &ARMAttributeParser::StringAttribute },
+#define ATTRIBUTE_HANDLER(attr)                                                \
+  { ARMBuildAttrs::attr, &ARMAttributeParser::attr }
+
+const ARMAttributeParser::DisplayHandler ARMAttributeParser::displayRoutines[] =
+    {{ARMBuildAttrs::CPU_raw_name, &ARMAttributeParser::stringAttribute},
+     {ARMBuildAttrs::CPU_name, &ARMAttributeParser::stringAttribute},
      ATTRIBUTE_HANDLER(CPU_arch),
      ATTRIBUTE_HANDLER(CPU_arch_profile),
      ATTRIBUTE_HANDLER(ARM_ISA_use),
      ATTRIBUTE_HANDLER(THUMB_ISA_use),
      ATTRIBUTE_HANDLER(FP_arch),
      ATTRIBUTE_HANDLER(WMMX_arch),
      ATTRIBUTE_HANDLER(Advanced_SIMD_arch),
      ATTRIBUTE_HANDLER(MVE_arch),
      ATTRIBUTE_HANDLER(PCS_config),
      ATTRIBUTE_HANDLER(ABI_PCS_R9_use),
      ATTRIBUTE_HANDLER(ABI_PCS_RW_data),
      ATTRIBUTE_HANDLER(ABI_PCS_RO_data),
      ATTRIBUTE_HANDLER(ABI_PCS_GOT_use),
      ATTRIBUTE_HANDLER(ABI_PCS_wchar_t),
      ATTRIBUTE_HANDLER(ABI_FP_rounding),
      ATTRIBUTE_HANDLER(ABI_FP_denormal),
      ATTRIBUTE_HANDLER(ABI_FP_exceptions),
      ATTRIBUTE_HANDLER(ABI_FP_user_exceptions),
      ATTRIBUTE_HANDLER(ABI_FP_number_model),
      ATTRIBUTE_HANDLER(ABI_align_needed),
      ATTRIBUTE_HANDLER(ABI_align_preserved),
      ATTRIBUTE_HANDLER(ABI_enum_size),
      ATTRIBUTE_HANDLER(ABI_HardFP_use),
      ATTRIBUTE_HANDLER(ABI_VFP_args),
      ATTRIBUTE_HANDLER(ABI_WMMX_args),
      ATTRIBUTE_HANDLER(ABI_optimization_goals),
      ATTRIBUTE_HANDLER(ABI_FP_optimization_goals),
      ATTRIBUTE_HANDLER(compatibility),
      ATTRIBUTE_HANDLER(CPU_unaligned_access),
      ATTRIBUTE_HANDLER(FP_HP_extension),
      ATTRIBUTE_HANDLER(ABI_FP_16bit_format),
      ATTRIBUTE_HANDLER(MPextension_use),
      ATTRIBUTE_HANDLER(DIV_use),
      ATTRIBUTE_HANDLER(DSP_extension),
      ATTRIBUTE_HANDLER(T2EE_use),
      ATTRIBUTE_HANDLER(Virtualization_use),
-  ATTRIBUTE_HANDLER(nodefaults)
-};
+     ATTRIBUTE_HANDLER(nodefaults)};
 
 #undef ATTRIBUTE_HANDLER
 
-uint64_t ARMAttributeParser::ParseInteger(const uint8_t *Data,
-                                          uint32_t &Offset) {
-  unsigned DecodeLength;
-  uint64_t Value = decodeULEB128(Data + Offset, &DecodeLength);
-  Offset += DecodeLength;
-  return Value;
-}
-
-StringRef ARMAttributeParser::ParseString(const uint8_t *Data,
-                                          uint32_t &Offset) {
-  const char *String = reinterpret_cast<const char*>(Data + Offset);
-  size_t Length = std::strlen(String);
-  Offset = Offset + Length + 1;
-  return StringRef(String, Length);
-}
-
-void ARMAttributeParser::IntegerAttribute(AttrType Tag, const uint8_t *Data,
-                                          uint32_t &Offset) {
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  Attributes.insert(std::make_pair(Tag, Value));
-
-  if (SW)
-    SW->printNumber(ARMBuildAttrs::AttrTypeAsString(Tag), Value);
-}
-
-void ARMAttributeParser::StringAttribute(AttrType Tag, const uint8_t *Data,
-                                         uint32_t &Offset) {
-  StringRef TagName = ARMBuildAttrs::AttrTypeAsString(Tag, /*TagPrefix*/false);
-  StringRef ValueDesc = ParseString(Data, Offset);
-
-  if (SW) {
-    DictScope AS(*SW, "Attribute");
-    SW->printNumber("Tag", Tag);
-    if (!TagName.empty())
-      SW->printString("TagName", TagName);
-    SW->printString("Value", ValueDesc);
+Error ARMAttributeParser::stringAttribute(AttrType tag) {
+  StringRef tagName = ARMBuildAttrs::AttrTypeAsString(tag, /*TagPrefix=*/false);
+  StringRef desc = de.getCStrRef(cursor);
+
+  if (sw) {
+    DictScope scope(*sw, "Attribute");
+    sw->printNumber("Tag", tag);
+    if (!tagName.empty())
+      sw->printString("TagName", tagName);
+    sw->printString("Value", desc);
+  }
+  return Error::success();
+}
+
+void ARMAttributeParser::printAttribute(unsigned tag, unsigned value,
+                                        StringRef valueDesc) {
+  attributes.insert(std::make_pair(tag, value));
+
+  if (sw) {
+    StringRef tagName =
+        ARMBuildAttrs::AttrTypeAsString(tag, /*TagPrefix=*/false);
+    DictScope as(*sw, "Attribute");
+    sw->printNumber("Tag", tag);
+    sw->printNumber("Value", value);
+    if (!tagName.empty())
+      sw->printString("TagName", tagName);
+    if (!valueDesc.empty())
+      sw->printString("Description", valueDesc);
   }
 }
 
-void ARMAttributeParser::PrintAttribute(unsigned Tag, unsigned Value,
-                                        StringRef ValueDesc) {
-  Attributes.insert(std::make_pair(Tag, Value));
-
-  if (SW) {
-    StringRef TagName = ARMBuildAttrs::AttrTypeAsString(Tag,
-                                                        /*TagPrefix*/false);
-    DictScope AS(*SW, "Attribute");
-    SW->printNumber("Tag", Tag);
-    SW->printNumber("Value", Value);
-    if (!TagName.empty())
-      SW->printString("TagName", TagName);
-    if (!ValueDesc.empty())
-      SW->printString("Description", ValueDesc);
+Error ARMAttributeParser::parseStringAttribute(const char *name, AttrType tag,
+                                               ArrayRef<const char *> strings) {
+  uint64_t value = de.getULEB128(cursor);
+  if (value >= strings.size()) {
+    printAttribute(tag, value, "");
+    return createStringError(errc::invalid_argument,
+                             "unknown " + Twine(name) +
+                                 " value: " + Twine(value));
   }
+  printAttribute(tag, value, strings[value]);
+  return Error::success();
 }
 
-void ARMAttributeParser::CPU_arch(AttrType Tag, const uint8_t *Data,
-                                  uint32_t &Offset) {
-  static const char *const Strings[] = {
+Error ARMAttributeParser::CPU_arch(AttrType tag) {
+  static const char *strings[] = {
     "Pre-v4", "ARM v4", "ARM v4T", "ARM v5T", "ARM v5TE", "ARM v5TEJ", "ARM v6",
     "ARM v6KZ", "ARM v6T2", "ARM v6K", "ARM v7", "ARM v6-M", "ARM v6S-M",
     "ARM v7E-M", "ARM v8", nullptr,
     "ARM v8-M Baseline", "ARM v8-M Mainline", nullptr, nullptr, nullptr,
     "ARM v8.1-M Mainline"
   };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+  return parseStringAttribute("CPU_arch", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::CPU_arch_profile(AttrType Tag, const uint8_t *Data,
-                                          uint32_t &Offset) {
-  uint64_t Encoded = ParseInteger(Data, Offset);
+Error ARMAttributeParser::CPU_arch_profile(AttrType tag) {
+  uint64_t value = de.getULEB128(cursor);
 
-  StringRef Profile;
-  switch (Encoded) {
-  default:  Profile = "Unknown"; break;
-  case 'A': Profile = "Application"; break;
-  case 'R': Profile = "Real-time"; break;
-  case 'M': Profile = "Microcontroller"; break;
-  case 'S': Profile = "Classic"; break;
-  case 0: Profile = "None"; break;
+  StringRef profile;
+  switch (value) {
+  default: profile = "Unknown"; break;
+  case 'A': profile = "Application"; break;
+  case 'R': profile = "Real-time"; break;
+  case 'M': profile = "Microcontroller"; break;
+  case 'S': profile = "Classic"; break;
+  case 0: profile = "None"; break;
   }
 
-  PrintAttribute(Tag, Encoded, Profile);
+  printAttribute(tag, value, profile);
+  return Error::success();
 }
 
-void ARMAttributeParser::ARM_ISA_use(AttrType Tag, const uint8_t *Data,
-                                     uint32_t &Offset) {
-  static const char *const Strings[] = { "Not Permitted", "Permitted" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::ARM_ISA_use(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "Permitted"};
+  return parseStringAttribute("ARM_ISA_use", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::THUMB_ISA_use(AttrType Tag, const uint8_t *Data,
-                                       uint32_t &Offset) {
-  static const char *const Strings[] = { "Not Permitted", "Thumb-1", "Thumb-2" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::THUMB_ISA_use(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "Thumb-1", "Thumb-2"};
+  return parseStringAttribute("THUMB_ISA_use", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::FP_arch(AttrType Tag, const uint8_t *Data,
-                                 uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Not Permitted", "VFPv1", "VFPv2", "VFPv3", "VFPv3-D16", "VFPv4",
-    "VFPv4-D16", "ARMv8-a FP", "ARMv8-a FP-D16"
-  };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
-}
-
-void ARMAttributeParser::WMMX_arch(AttrType Tag, const uint8_t *Data,
-                                   uint32_t &Offset) {
-  static const char *const Strings[] = { "Not Permitted", "WMMXv1", "WMMXv2" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
-}
-
-void ARMAttributeParser::Advanced_SIMD_arch(AttrType Tag, const uint8_t *Data,
-                                            uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Not Permitted", "NEONv1", "NEONv2+FMA", "ARMv8-a NEON", "ARMv8.1-a NEON"
-  };
+Error ARMAttributeParser::FP_arch(AttrType tag) {
+  static const char *strings[] = {
+      "Not Permitted", "VFPv1",     "VFPv2",      "VFPv3",         "VFPv3-D16",
+      "VFPv4",         "VFPv4-D16", "ARMv8-a FP", "ARMv8-a FP-D16"};
+  return parseStringAttribute("FP_arch", tag, makeArrayRef(strings));
+}
 
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::WMMX_arch(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "WMMXv1", "WMMXv2"};
+  return parseStringAttribute("WMMX_arch", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::MVE_arch(AttrType Tag, const uint8_t *Data,
-                                  uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Not Permitted", "MVE integer", "MVE integer and float"
-  };
+Error ARMAttributeParser::Advanced_SIMD_arch(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "NEONv1", "NEONv2+FMA",
+                                  "ARMv8-a NEON", "ARMv8.1-a NEON"};
+  return parseStringAttribute("Advanced_SIMD_arch", tag, makeArrayRef(strings));
+}
 
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::MVE_arch(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "MVE integer",
+                                  "MVE integer and float"};
+  return parseStringAttribute("MVE_arch", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::PCS_config(AttrType Tag, const uint8_t *Data,
-                                    uint32_t &Offset) {
-  static const char *const Strings[] = {
+Error ARMAttributeParser::PCS_config(AttrType tag) {
+  static const char *strings[] = {
     "None", "Bare Platform", "Linux Application", "Linux DSO", "Palm OS 2004",
-    "Reserved (Palm OS)", "Symbian OS 2004", "Reserved (Symbian OS)"
-  };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
-}
-
-void ARMAttributeParser::ABI_PCS_R9_use(AttrType Tag, const uint8_t *Data,
-                                        uint32_t &Offset) {
-  static const char *const Strings[] = { "v6", "Static Base", "TLS", "Unused" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
-}
-
-void ARMAttributeParser::ABI_PCS_RW_data(AttrType Tag, const uint8_t *Data,
-                                         uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Absolute", "PC-relative", "SB-relative", "Not Permitted"
-  };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+    "Reserved (Palm OS)", "Symbian OS 2004", "Reserved (Symbian OS)"};
+  return parseStringAttribute("PCS_config", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::ABI_PCS_RO_data(AttrType Tag, const uint8_t *Data,
-                                         uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Absolute", "PC-relative", "Not Permitted"
-  };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::ABI_PCS_R9_use(AttrType tag) {
+  static const char *strings[] = {"v6", "Static Base", "TLS", "Unused"};
+  return parseStringAttribute("ABI_PCS_R9_use", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::ABI_PCS_GOT_use(AttrType Tag, const uint8_t *Data,
-                                         uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Not Permitted", "Direct", "GOT-Indirect"
-  };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::ABI_PCS_RW_data(AttrType tag) {
+  static const char *strings[] = {"Absolute", "PC-relative", "SB-relative",
+                                  "Not Permitted"};
+  return parseStringAttribute("ABI_PCS_RW_data", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::ABI_PCS_wchar_t(AttrType Tag, const uint8_t *Data,
-                                         uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Not Permitted", "Unknown", "2-byte", "Unknown", "4-byte"
-  };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
-}
-
-void ARMAttributeParser::ABI_FP_rounding(AttrType Tag, const uint8_t *Data,
-                                         uint32_t &Offset) {
-  static const char *const Strings[] = { "IEEE-754", "Runtime" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
-}
-
-void ARMAttributeParser::ABI_FP_denormal(AttrType Tag, const uint8_t *Data,
-                                         uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Unsupported", "IEEE-754", "Sign Only"
-  };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::ABI_PCS_RO_data(AttrType tag) {
+  static const char *strings[] = {"Absolute", "PC-relative", "Not Permitted"};
+  return parseStringAttribute("ABI_PCS_RO_data", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::ABI_FP_exceptions(AttrType Tag, const uint8_t *Data,
-                                           uint32_t &Offset) {
-  static const char *const Strings[] = { "Not Permitted", "IEEE-754" };
+Error ARMAttributeParser::ABI_PCS_GOT_use(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "Direct", "GOT-Indirect"};
+  return parseStringAttribute("ABI_PCS_GOT_use", tag, makeArrayRef(strings));
+}
 
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::ABI_PCS_wchar_t(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "Unknown", "2-byte",
+                                  "Unknown", "4-byte"};
+  return parseStringAttribute("ABI_PCS_wchar_t", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::ABI_FP_user_exceptions(AttrType Tag,
-                                                const uint8_t *Data,
-                                                uint32_t &Offset) {
-  static const char *const Strings[] = { "Not Permitted", "IEEE-754" };
+Error ARMAttributeParser::ABI_FP_rounding(AttrType tag) {
+  static const char *strings[] = {"IEEE-754", "Runtime"};
+  return parseStringAttribute("ABI_FP_rounding", tag, makeArrayRef(strings));
+}
 
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::ABI_FP_denormal(AttrType tag) {
+  static const char *strings[] = {"Unsupported", "IEEE-754", "Sign Only"};
+  return parseStringAttribute("ABI_FP_denormal", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::ABI_FP_number_model(AttrType Tag, const uint8_t *Data,
-                                             uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Not Permitted", "Finite Only", "RTABI", "IEEE-754"
-  };
+Error ARMAttributeParser::ABI_FP_exceptions(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "IEEE-754"};
+  return parseStringAttribute("ABI_FP_exceptions", tag, makeArrayRef(strings));
+}
+Error ARMAttributeParser::ABI_FP_user_exceptions(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "IEEE-754"};
+  return parseStringAttribute("ABI_FP_user_exceptions", tag,
+                              makeArrayRef(strings));
+}
 
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::ABI_FP_number_model(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "Finite Only", "RTABI",
+                                  "IEEE-754"};
+  return parseStringAttribute("ABI_FP_number_model", tag,
+                              makeArrayRef(strings));
 }
 
-void ARMAttributeParser::ABI_align_needed(AttrType Tag, const uint8_t *Data,
-                                          uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Not Permitted", "8-byte alignment", "4-byte alignment", "Reserved"
-  };
+Error ARMAttributeParser::ABI_align_needed(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "8-byte alignment",
+                                  "4-byte alignment", "Reserved"};
 
-  uint64_t Value = ParseInteger(Data, Offset);
+  uint64_t value = de.getULEB128(cursor);
 
-  std::string Description;
-  if (Value < array_lengthof(Strings))
-    Description = std::string(Strings[Value]);
-  else if (Value <= 12)
-    Description = std::string("8-byte alignment, ") + utostr(1ULL << Value)
-                + std::string("-byte extended alignment");
+  std::string description;
+  if (value < array_lengthof(strings))
+    description = strings[value];
+  else if (value <= 12)
+    description = "8-byte alignment, " + utostr(1ULL << value) +
+                  "-byte extended alignment";
   else
-    Description = "Invalid";
+    description = "Invalid";
 
-  PrintAttribute(Tag, Value, Description);
+  printAttribute(tag, value, description);
+  return Error::success();
 }
 
-void ARMAttributeParser::ABI_align_preserved(AttrType Tag, const uint8_t *Data,
-                                             uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Not Required", "8-byte data alignment", "8-byte data and code alignment",
-    "Reserved"
-  };
+Error ARMAttributeParser::ABI_align_preserved(AttrType tag) {
+  static const char *strings[] = {"Not Required", "8-byte data alignment",
+                                  "8-byte data and code alignment", "Reserved"};
 
-  uint64_t Value = ParseInteger(Data, Offset);
+  uint64_t value = de.getULEB128(cursor);
 
-  std::string Description;
-  if (Value < array_lengthof(Strings))
-    Description = std::string(Strings[Value]);
-  else if (Value <= 12)
-    Description = std::string("8-byte stack alignment, ") +
-                  utostr(1ULL << Value) + std::string("-byte data alignment");
+  std::string description;
+  if (value < array_lengthof(strings))
+    description = std::string(strings[value]);
+  else if (value <= 12)
+    description = std::string("8-byte stack alignment, ") +
+                  utostr(1ULL << value) + std::string("-byte data alignment");
   else
-    Description = "Invalid";
+    description = "Invalid";
 
-  PrintAttribute(Tag, Value, Description);
+  printAttribute(tag, value, description);
+  return Error::success();
 }
 
-void ARMAttributeParser::ABI_enum_size(AttrType Tag, const uint8_t *Data,
-                                       uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Not Permitted", "Packed", "Int32", "External Int32"
-  };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::ABI_enum_size(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "Packed", "Int32",
+                                  "External Int32"};
+  return parseStringAttribute("ABI_enum_size", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::ABI_HardFP_use(AttrType Tag, const uint8_t *Data,
-                                        uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Tag_FP_arch", "Single-Precision", "Reserved", "Tag_FP_arch (deprecated)"
-  };
+Error ARMAttributeParser::ABI_HardFP_use(AttrType tag) {
+  static const char *strings[] = {"Tag_FP_arch", "Single-Precision", "Reserved",
+                                  "Tag_FP_arch (deprecated)"};
+  return parseStringAttribute("ABI_HardFP_use", tag, makeArrayRef(strings));
+}
 
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::ABI_VFP_args(AttrType tag) {
+  static const char *strings[] = {"AAPCS", "AAPCS VFP", "Custom",
+                                  "Not Permitted"};
+  return parseStringAttribute("ABI_VFP_args", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::ABI_VFP_args(AttrType Tag, const uint8_t *Data,
-                                      uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "AAPCS", "AAPCS VFP", "Custom", "Not Permitted"
-  };
+Error ARMAttributeParser::ABI_WMMX_args(AttrType tag) {
+  static const char *strings[] = {"AAPCS", "iWMMX", "Custom"};
+  return parseStringAttribute("ABI_WMMX_args", tag, makeArrayRef(strings));
+}
 
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
-}
-
-void ARMAttributeParser::ABI_WMMX_args(AttrType Tag, const uint8_t *Data,
-                                       uint32_t &Offset) {
-  static const char *const Strings[] = { "AAPCS", "iWMMX", "Custom" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
-}
-
-void ARMAttributeParser::ABI_optimization_goals(AttrType Tag,
-                                                const uint8_t *Data,
-                                                uint32_t &Offset) {
-  static const char *const Strings[] = {
+Error ARMAttributeParser::ABI_optimization_goals(AttrType tag) {
+  static const char *strings[] = {
     "None", "Speed", "Aggressive Speed", "Size", "Aggressive Size", "Debugging",
     "Best Debugging"
   };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+  return parseStringAttribute("ABI_optimization_goals", tag,
+                              makeArrayRef(strings));
 }
 
-void ARMAttributeParser::ABI_FP_optimization_goals(AttrType Tag,
-                                                   const uint8_t *Data,
-                                                   uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "None", "Speed", "Aggressive Speed", "Size", "Aggressive Size", "Accuracy",
-    "Best Accuracy"
-  };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
-}
-
-void ARMAttributeParser::compatibility(AttrType Tag, const uint8_t *Data,
-                                       uint32_t &Offset) {
-  uint64_t Integer = ParseInteger(Data, Offset);
-  StringRef String = ParseString(Data, Offset);
-
-  if (SW) {
-    DictScope AS(*SW, "Attribute");
-    SW->printNumber("Tag", Tag);
-    SW->startLine() << "Value: " << Integer << ", " << String << '\n';
-    SW->printString("TagName", AttrTypeAsString(Tag, /*TagPrefix*/false));
-    switch (Integer) {
+Error ARMAttributeParser::ABI_FP_optimization_goals(AttrType tag) {
+  static const char *strings[] = {
+      "None",     "Speed",        "Aggressive Speed", "Size", "Aggressive Size",
+      "Accuracy", "Best Accuracy"};
+  return parseStringAttribute("ABI_FP_optimization_goals", tag,
+                              makeArrayRef(strings));
+}
+
+Error ARMAttributeParser::compatibility(AttrType tag) {
+  uint64_t integer = de.getULEB128(cursor);
+  StringRef string = de.getCStrRef(cursor);
+
+  if (sw) {
+    DictScope scope(*sw, "Attribute");
+    sw->printNumber("Tag", tag);
+    sw->startLine() << "Value: " << integer << ", " << string << '\n';
+    sw->printString("TagName", AttrTypeAsString(tag, /*TagPrefix*/ false));
+    switch (integer) {
     case 0:
-      SW->printString("Description", StringRef("No Specific Requirements"));
+      sw->printString("Description", StringRef("No Specific Requirements"));
       break;
     case 1:
-      SW->printString("Description", StringRef("AEABI Conformant"));
+      sw->printString("Description", StringRef("AEABI Conformant"));
       break;
     default:
-      SW->printString("Description", StringRef("AEABI Non-Conformant"));
+      sw->printString("Description", StringRef("AEABI Non-Conformant"));
       break;
     }
   }
+  return Error::success();
 }
 
-void ARMAttributeParser::CPU_unaligned_access(AttrType Tag, const uint8_t *Data,
-                                              uint32_t &Offset) {
-  static const char *const Strings[] = { "Not Permitted", "v6-style" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::CPU_unaligned_access(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "v6-style"};
+  return parseStringAttribute("CPU_unaligned_access", tag,
+                              makeArrayRef(strings));
 }
 
-void ARMAttributeParser::FP_HP_extension(AttrType Tag, const uint8_t *Data,
-                                         uint32_t &Offset) {
-  static const char *const Strings[] = { "If Available", "Permitted" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::FP_HP_extension(AttrType tag) {
+  static const char *strings[] = {"If Available", "Permitted"};
+  return parseStringAttribute("FP_HP_extension", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::ABI_FP_16bit_format(AttrType Tag, const uint8_t *Data,
-                                             uint32_t &Offset) {
-  static const char *const Strings[] = { "Not Permitted", "IEEE-754", "VFPv3" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::ABI_FP_16bit_format(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "IEEE-754", "VFPv3"};
+  return parseStringAttribute("ABI_FP_16bit_format", tag,
+                              makeArrayRef(strings));
 }
 
-void ARMAttributeParser::MPextension_use(AttrType Tag, const uint8_t *Data,
-                                         uint32_t &Offset) {
-  static const char *const Strings[] = { "Not Permitted", "Permitted" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::MPextension_use(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "Permitted"};
+  return parseStringAttribute("MPextension_use", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::DIV_use(AttrType Tag, const uint8_t *Data,
-                                 uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "If Available", "Not Permitted", "Permitted"
-  };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::DIV_use(AttrType tag) {
+  static const char *strings[] = {"If Available", "Not Permitted", "Permitted"};
+  return parseStringAttribute("DIV_use", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::DSP_extension(AttrType Tag, const uint8_t *Data,
-                                       uint32_t &Offset) {
-  static const char *const Strings[] = { "Not Permitted", "Permitted" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::DSP_extension(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "Permitted"};
+  return parseStringAttribute("DSP_extension", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::T2EE_use(AttrType Tag, const uint8_t *Data,
-                                  uint32_t &Offset) {
-  static const char *const Strings[] = { "Not Permitted", "Permitted" };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::T2EE_use(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "Permitted"};
+  return parseStringAttribute("T2EE_use", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::Virtualization_use(AttrType Tag, const uint8_t *Data,
-                                            uint32_t &Offset) {
-  static const char *const Strings[] = {
-    "Not Permitted", "TrustZone", "Virtualization Extensions",
-    "TrustZone + Virtualization Extensions"
-  };
-
-  uint64_t Value = ParseInteger(Data, Offset);
-  StringRef ValueDesc =
-    (Value < array_lengthof(Strings)) ? Strings[Value] : nullptr;
-  PrintAttribute(Tag, Value, ValueDesc);
+Error ARMAttributeParser::Virtualization_use(AttrType tag) {
+  static const char *strings[] = {"Not Permitted", "TrustZone",
+                                  "Virtualization Extensions",
+                                  "TrustZone + Virtualization Extensions"};
+  return parseStringAttribute("Virtualization_use", tag, makeArrayRef(strings));
 }
 
-void ARMAttributeParser::nodefaults(AttrType Tag, const uint8_t *Data,
-                                    uint32_t &Offset) {
-  uint64_t Value = ParseInteger(Data, Offset);
-  PrintAttribute(Tag, Value, "Unspecified Tags UNDEFINED");
+Error ARMAttributeParser::nodefaults(AttrType tag) {
+  uint64_t value = de.getULEB128(cursor);
+  printAttribute(tag, value, "Unspecified Tags UNDEFINED");
+  return Error::success();
 }
 
-void ARMAttributeParser::ParseIndexList(const uint8_t *Data, uint32_t &Offset,
-                                        SmallVectorImpl<uint8_t> &IndexList) {
+void ARMAttributeParser::parseIndexList(SmallVectorImpl<uint8_t> &indexList) {
   for (;;) {
-    unsigned DecodeLength;
-    uint64_t Value = decodeULEB128(Data + Offset, &DecodeLength);
-    Offset += DecodeLength;
-    if (Value == 0)
+    uint64_t value = de.getULEB128(cursor);
+    if (!cursor || !value)
       break;
-    IndexList.push_back(Value);
+    indexList.push_back(value);
   }
 }
 
-void ARMAttributeParser::ParseAttributeList(const uint8_t *Data,
-                                            uint32_t &Offset, uint32_t Length) {
-  while (Offset < Length) {
-    unsigned DecodeLength;
-    uint64_t Tag = decodeULEB128(Data + Offset, &DecodeLength);
-    Offset += DecodeLength;
-
-    bool Handled = false;
-    for (unsigned AHI = 0, AHE = array_lengthof(DisplayRoutines);
-         AHI != AHE && !Handled; ++AHI) {
-      if (uint64_t(DisplayRoutines[AHI].Attribute) == Tag) {
-        (this->*DisplayRoutines[AHI].Routine)(ARMBuildAttrs::AttrType(Tag),
-                                              Data, Offset);
-        Handled = true;
+Error ARMAttributeParser::parseAttributeList(uint32_t length) {
+  uint64_t pos;
+  uint64_t end = cursor.tell() - 5 + length;
+  while ((pos = cursor.tell()) < end) {
+    uint64_t tag = de.getULEB128(cursor);
+    bool handled = false;
+    for (unsigned AHI = 0, AHE = array_lengthof(displayRoutines);
+         AHI != AHE && !handled; ++AHI) {
+      if (uint64_t(displayRoutines[AHI].attribute) == tag) {
+        if (Error e = (this->*displayRoutines[AHI].routine)(
+                ARMBuildAttrs::AttrType(tag)))
+          return e;
+        handled = true;
         break;
       }
     }
-    if (!Handled) {
-      if (Tag < 32) {
-        errs() << "unhandled AEABI Tag " << Tag
-               << " (" << ARMBuildAttrs::AttrTypeAsString(Tag) << ")\n";
-        continue;
-      }
+    if (!handled) {
+      if (tag < 32)
+        return createStringError(errc::invalid_argument,
+                                 "invalid AEABI tag 0x" +
+                                     Twine::utohexstr(tag) + " at offset 0x" +
+                                     Twine::utohexstr(pos));
+
+      if (tag % 2 == 0) {
+        uint64_t value = de.getULEB128(cursor);
+        attributes.insert(std::make_pair(tag, value));
+        if (sw)
+          sw->printNumber(ARMBuildAttrs::AttrTypeAsString(tag), value);
+      } else {
+        StringRef tagName =
+            ARMBuildAttrs::AttrTypeAsString(tag, /*TagPrefix=*/false);
+        StringRef desc = de.getCStrRef(cursor);
 
-      if (Tag % 2 == 0)
-        IntegerAttribute(ARMBuildAttrs::AttrType(Tag), Data, Offset);
-      else
-        StringAttribute(ARMBuildAttrs::AttrType(Tag), Data, Offset);
+        if (sw) {
+          DictScope scope(*sw, "Attribute");
+          sw->printNumber("Tag", tag);
+          if (!tagName.empty())
+            sw->printString("TagName", tagName);
+          sw->printString("Value", desc);
         }
       }
     }
-
-void ARMAttributeParser::ParseSubsection(const uint8_t *Data, uint32_t Length) {
-  uint32_t Offset = sizeof(uint32_t); /* SectionLength */
-
-  const char *VendorName = reinterpret_cast<const char*>(Data + Offset);
-  size_t VendorNameLength = std::strlen(VendorName);
-  Offset = Offset + VendorNameLength + 1;
-
-  if (SW) {
-    SW->printNumber("SectionLength", Length);
-    SW->printString("Vendor", StringRef(VendorName, VendorNameLength));
   }
-
-  if (StringRef(VendorName, VendorNameLength).lower() != "aeabi") {
-    return;
-  }
+  return Error::success();
 
-  while (Offset < Length) {
-    /// Tag_File | Tag_Section | Tag_Symbol   uleb128:byte-size
-    uint8_t Tag = Data[Offset];
-    Offset = Offset + sizeof(Tag);
-
-    uint32_t Size =
-      *reinterpret_cast<const support::ulittle32_t*>(Data + Offset);
-    Offset = Offset + sizeof(Size);
-
-    if (SW) {
-      SW->printEnum("Tag", Tag, makeArrayRef(TagNames));
-      SW->printNumber("Size", Size);
-    }
+Error ARMAttributeParser::parseSubsection(uint32_t length) {
+  uint64_t end = cursor.tell() - 4 + length;
+  StringRef vendorName = de.getCStrRef(cursor);
+  if (sw) {
+    sw->printNumber("SectionLength", length);
+    sw->printString("Vendor", vendorName);
 
-    if (Size > Length) {
-      errs() << "subsection length greater than section length\n";
-      return;
+  // Ignore unrecognized vendor-name.
+  if (vendorName.lower() != "aeabi")
+    return createStringError(errc::invalid_argument,
+                             "unrecognized vendor-name: " + vendorName);
+
+  while (cursor.tell() < end) {
+    /// Tag_File | Tag_Section | Tag_Symbol   uleb128:byte-size
+    uint8_t tag = de.getU8(cursor);
+    uint32_t size = de.getU32(cursor);
+    if (!cursor)
+      return cursor.takeError();
+
+    if (sw) {
+      sw->printEnum("Tag", tag, makeArrayRef(tagNames));
+      sw->printNumber("Size", size);
     }
 
-    StringRef ScopeName, IndexName;
-    SmallVector<uint8_t, 8> Indicies;
-    switch (Tag) {
+    StringRef scopeName, indexName;
+    SmallVector<uint8_t, 8> indicies;
+    switch (tag) {
     case ARMBuildAttrs::File:
-      ScopeName = "FileAttributes";
+      scopeName = "FileAttributes";
       break;
     case ARMBuildAttrs::Section:
-      ScopeName = "SectionAttributes";
-      IndexName = "Sections";
-      ParseIndexList(Data, Offset, Indicies);
+      scopeName = "SectionAttributes";
+      indexName = "Sections";
+      parseIndexList(indicies);
       break;
     case ARMBuildAttrs::Symbol:
-      ScopeName = "SymbolAttributes";
-      IndexName = "Symbols";
-      ParseIndexList(Data, Offset, Indicies);
+      scopeName = "SymbolAttributes";
+      indexName = "Symbols";
+      parseIndexList(indicies);
       break;
     default:
-      errs() << "unrecognised tag: 0x" << Twine::utohexstr(Tag) << '\n';
-      return;
-    }
-
-    if (SW) {
-      DictScope ASS(*SW, ScopeName);
-      if (!Indicies.empty())
-        SW->printList(IndexName, Indicies);
-      ParseAttributeList(Data, Offset, Length);
-    } else {
-      ParseAttributeList(Data, Offset, Length);
-    }
+      return createStringError(errc::invalid_argument,
+                               "unrecognized tag 0x" + Twine::utohexstr(tag) +
+                                   " at offset 0x" +
+                                   Twine::utohexstr(cursor.tell() - 5));
+    }
+
+    if (sw) {
+      DictScope scope(*sw, scopeName);
+      if (!indicies.empty())
+        sw->printList(indexName, indicies);
+      if (Error e = parseAttributeList(size))
+        return e;
+    } else if (Error e = parseAttributeList(size))
+      return e;
+  }
+  return Error::success();
+}
+
+Error ARMAttributeParser::parse(ArrayRef<uint8_t> section,
+                                support::endianness endian) {
+  unsigned sectionNumber = 0;
+  de = DataExtractor(section, endian == support::little, 0);
+
+  // For early returns, we have more specific errors, consume the Error in
+  // cursor.
+  struct ClearCursorError {
+    DataExtractor::Cursor &cursor;
+    ~ClearCursorError() { consumeError(cursor.takeError()); }
+  } clear{cursor};
+
+  // Unrecognized format-version.
+  uint8_t formatVersion = de.getU8(cursor);
+  if (formatVersion != 'A')
+    return createStringError(errc::invalid_argument,
+                             "unrecognized format-version: 0x" +
+                                 utohexstr(formatVersion));
+
+  while (!de.eof(cursor)) {
+    uint32_t sectionLength = de.getU32(cursor);
+    if (!cursor)
+      return cursor.takeError();
+
+    if (sw) {
+      sw->startLine() << "Section " << ++sectionNumber << " {\n";
+      sw->indent();
+    }
+
+    if (sectionLength < 4 || cursor.tell() - 4 + sectionLength > section.size())
+      return createStringError(errc::invalid_argument,
+                               "invalid subsection length " +
+                                   Twine(sectionLength) + " at offset 0x" +
+                                   utohexstr(cursor.tell() - 4));
+    if (Error e = parseSubsection(sectionLength))
+      return e;
+    if (sw) {
+      sw->unindent();
+      sw->startLine() << "}\n";
     }
   }
 
-void ARMAttributeParser::parse(ArrayRef<uint8_t> Section,
-                               support::endianness E) {
-  uint64_t Offset = 1;
-  unsigned SectionNumber = 0;
-
-  while (Offset < Section.size()) {
-    uint32_t SectionLength =
-        support::endian::read32(Section.data() + Offset, E);
-
-    if (SW) {
-      SW->startLine() << "Section " << ++SectionNumber << " {\n";
-      SW->indent();
-    }
-
-    if (SectionLength == 0 || (SectionLength + Offset) > Section.size()) {
-      errs() << "invalid subsection length " << SectionLength << " at offset "
-             << Offset << "\n";
-      return;
-    }
-
-    ParseSubsection(Section.data() + Offset, SectionLength);
-    Offset = Offset + SectionLength;
-
-    if (SW) {
-      SW->unindent();
-      SW->startLine() << "}\n";
-    }
-  }
-}
+  return cursor.takeError();
 }

llvm/tools/llvm-readobj/ELFDumper.cpp

if (Contents[0] != ARMBuildAttrs::Format_Version) {
              << Twine::utohexstr(Contents[0]) << '\n';
       continue;
     }
 
     W.printHex("FormatVersion", Contents[0]);
     if (Contents.size() == 1)
       continue;
 
-    ARMAttributeParser(&W).parse(Contents, support::little);
+    // TODO: Print error and delete the redundant FormatVersion check above.
+    if (Error E = ARMAttributeParser(&W).parse(Contents, support::little))
+      consumeError(std::move(E));
   }
 }
 
 template <class ELFT> class MipsGOTParser {
 public:
   TYPEDEF_ELF_TYPES(ELFT)
   using Entry = typename ELFO::Elf_Addr;
   using Entries = ArrayRef<Entry>;
 

llvm/unittests/Support/ARMAttributeParser.cpp

bool testBuildAttr(unsigned Tag, unsigned Value,
   std::string buffer;
   raw_string_ostream OS(buffer);
   AttributeSection Section(Tag, Value);
   Section.write(OS);
   ArrayRef<uint8_t> Bytes(
     reinterpret_cast<const uint8_t*>(OS.str().c_str()), OS.str().size());
 
   ARMAttributeParser Parser;
-  Parser.parse(Bytes, support::little);
+  cantFail(Parser.parse(Bytes, support::little));
 
   return (Parser.hasAttribute(ExpectedTag) &&
     Parser.getAttributeValue(ExpectedTag) == ExpectedValue);
 }
 
+void testParseError(ArrayRef<uint8_t> bytes, const char *msg) {
+  ARMAttributeParser parser;
+  Error e = parser.parse(bytes, support::little);
+  EXPECT_STREQ(toString(std::move(e)).c_str(), msg);
+}
+
 bool testTagString(unsigned Tag, const char *name) {
   return ARMBuildAttrs::AttrTypeAsString(Tag).str() == name;
 }
 
+TEST(ARMAttributeParser, UnrecognizedFormatVersion) {
+  static const uint8_t bytes[] = {1};
+  testParseError(bytes, "unrecognized format-version: 0x1");
+}
+
+TEST(ARMAttributeParser, InvalidSubsectionLength) {
+  static const uint8_t bytes[] = {'A', 3, 0, 0, 0};
+  testParseError(bytes, "invalid subsection length 3 at offset 0x1");
+}
+
+TEST(ARMAttributeParser, UnrecognizedVendorName) {
+  static const uint8_t bytes[] = {'A', 7, 0, 0, 0, 'x', 'y', 0};
+  testParseError(bytes, "unrecognized vendor-name: xy");
+}
+
+TEST(ARMAttributeParser, UnrecognizedTag) {
+  static const uint8_t bytes[] = {'A', 15,  0, 0, 0, 'a', 'e', 'a',
+                                  'b', 'i', 0, 4, 5, 0,   0,   0};
+  testParseError(bytes, "unrecognized tag 0x4 at offset 0xb");
+}
+
+TEST(ARMAttributeParser, UnknownCPU_arch) {
+  static const uint8_t bytes[] = {'A', 15, 0, 0, 0, 'a', 'e', 'a', 'b',
+                                  'i', 0,  1, 7, 0, 0,   0,   6,   22};
+  testParseError(bytes, "unknown CPU_arch value: 22");
+}
+
 TEST(CPUArchBuildAttr, testBuildAttr) {
   EXPECT_TRUE(testTagString(6, "Tag_CPU_arch"));
 
   EXPECT_TRUE(testBuildAttr(6, 0, ARMBuildAttrs::CPU_arch,
                             ARMBuildAttrs::Pre_v4));
   EXPECT_TRUE(testBuildAttr(6, 1, ARMBuildAttrs::CPU_arch,
                             ARMBuildAttrs::v4));
   EXPECT_TRUE(testBuildAttr(6, 2, ARMBuildAttrs::CPU_arch,