Update ARM and x86 ArchHandler to match 64bits counterparts. NFC

[Jean-Daniel]

Summary

Define an explicit type for arch specific reference kind and use it in switch statement to make the compiler emit warnings if some case is not cover.
It will help to catch such errors when we add new mach-o reference kind.

[shankarke]

not sure if its common coding practice to use return instead of break in switch statements.

lib/ReaderWriter/MachO/ArchHandler_arm.cpp

916

There is an assert at line 911, that takes care of making sure all the kinds are specific to ARM, no ?

[Jean-Daniel]

In D7612#123205, @shankarke wrote:

not sure if its common coding practice to use return instead of break in switch statements.

I don't know if this is a common practice but it matches the x86_64 and ARM64 ArchHandlers.

lib/ReaderWriter/MachO/ArchHandler_arm.cpp

916

assert are runtime error and are not a replacement for proper compile time warning IMHO.

[Jean-Daniel]

lib/ReaderWriter/MachO/ArchHandler_arm.cpp

916

Sorry I misread the comment.
The assert check the reference kind family, but it does not guarantee that the following switch has a case to handle it. The problem is when we add new reference kinds, we want to make sure that they are properly handled in all switch() statements.

[shankarke]

Please wait for Nick's review too.

lib/ReaderWriter/MachO/ArchHandler_arm.cpp

916

LGTM.

[kledzik]

LGTM

lib/ReaderWriter/MachO/ArchHandler_arm.cpp

 
 private:
   friend class Thumb2ToArmShimAtom;
   friend class ArmToThumbShimAtom;
 
   static const Registry::KindStrings _sKindStrings[];
   static const StubInfo              _sStubInfoArmPIC;
 
-  enum : Reference::KindValue {
+  enum Arm_Kinds : Reference::KindValue {
     invalid,               /// for error condition
 
     modeThumbCode,         /// Content starting at this offset is thumb.
     modeArmCode,           /// Content starting at this offset is arm.
     modeData,              /// Content starting at this offset is data.
 
     // Kinds found in mach-o .o files:
     thumb_bl22,            /// ex: bl _foo

 //  ArchHandler_arm
 //===----------------------------------------------------------------------===//
 
 ArchHandler_arm::ArchHandler_arm() { }
 
 ArchHandler_arm::~ArchHandler_arm() { }
 
 const Registry::KindStrings ArchHandler_arm::_sKindStrings[] = {
+  LLD_KIND_STRING_ENTRY(invalid),
   LLD_KIND_STRING_ENTRY(modeThumbCode),
   LLD_KIND_STRING_ENTRY(modeArmCode),
   LLD_KIND_STRING_ENTRY(modeData),
   LLD_KIND_STRING_ENTRY(thumb_bl22),
   LLD_KIND_STRING_ENTRY(thumb_b22),
   LLD_KIND_STRING_ENTRY(thumb_movw),
   LLD_KIND_STRING_ENTRY(thumb_movt),
   LLD_KIND_STRING_ENTRY(thumb_movw_funcRel),
void ArchHandler_arm::applyFixupFinal(const Reference &ref, uint8_t *loc,
                                       bool &thumbMode, bool targetIsThumb) {
   if (ref.kindNamespace() != Reference::KindNamespace::mach_o)
     return;
   assert(ref.kindArch() == Reference::KindArch::ARM);
   ulittle32_t *loc32 = reinterpret_cast<ulittle32_t *>(loc);
   int32_t displacement;
   uint16_t value16;
   uint32_t value32;
-  switch (ref.kindValue()) {
+  switch (static_cast<Arm_Kinds>(ref.kindValue())) {
   case modeThumbCode:
     thumbMode = true;
     break;
   case modeArmCode:
     thumbMode = false;
     break;
   case modeData:
     break;
void ArchHandler_arm::applyFixupRelocatable(const Reference &ref, uint8_t *loc,
                                             bool &thumbMode,
                                             bool targetIsThumb) {
   bool useExternalReloc = useExternalRelocationTo(*ref.target());
   ulittle32_t *loc32 = reinterpret_cast<ulittle32_t *>(loc);
   int32_t displacement;
   uint16_t value16;
   uint32_t value32;
   bool targetIsUndef = isa<UndefinedAtom>(ref.target());
-  switch (ref.kindValue()) {
+  switch (static_cast<Arm_Kinds>(ref.kindValue())) {
   case modeThumbCode:
     thumbMode = true;
-    break;
+    return;
   case modeArmCode:
     thumbMode = false;
-    break;
+    return;
   case modeData:
-    break;
+    return;
   case thumb_b22:
   case thumb_bl22:
     assert(thumbMode);
     if (useExternalReloc)
       displacement = (ref.addend() - (fixupAddress + 4));
     else
       displacement = (targetAddress - (fixupAddress + 4)) + ref.addend();
     value32 = setDisplacementInThumbBranch(*loc32, fixupAddress,
                                            displacement,
                                            targetIsUndef || targetIsThumb);
     *loc32 = value32;
-    break;
+    return;
   case thumb_movw:
     assert(thumbMode);
     if (useExternalReloc)
       value16 = ref.addend() & 0xFFFF;
     else
       value16 = (targetAddress + ref.addend()) & 0xFFFF;
     *loc32 = setWordFromThumbMov(*loc32, value16);
-    break;
+    return;
   case thumb_movt:
     assert(thumbMode);
     if (useExternalReloc)
       value16 = ref.addend() >> 16;
     else
       value16 = (targetAddress + ref.addend()) >> 16;
     *loc32 = setWordFromThumbMov(*loc32, value16);
-    break;
+    return;
   case thumb_movw_funcRel:
     assert(thumbMode);
     value16 = (targetAddress - inAtomAddress + ref.addend()) & 0xFFFF;
     *loc32 = setWordFromThumbMov(*loc32, value16);
-    break;
+    return;
   case thumb_movt_funcRel:
     assert(thumbMode);
     value16 = (targetAddress - inAtomAddress + ref.addend()) >> 16;
     *loc32 = setWordFromThumbMov(*loc32, value16);
-    break;
+    return;
   case arm_b24:
   case arm_bl24:
     assert(!thumbMode);
     if (useExternalReloc)
       displacement = (ref.addend() - (fixupAddress + 8));
     else
       displacement = (targetAddress - (fixupAddress + 8)) + ref.addend();
     value32 = setDisplacementInArmBranch(*loc32, displacement,
                                          targetIsThumb);
     *loc32 = value32;
-    break;
+    return;
   case arm_movw:
     assert(!thumbMode);
     if (useExternalReloc)
       value16 = ref.addend() & 0xFFFF;
     else
       value16 = (targetAddress + ref.addend()) & 0xFFFF;
     *loc32 = setWordFromArmMov(*loc32, value16);
-    break;
+    return;
   case arm_movt:
     assert(!thumbMode);
     if (useExternalReloc)
       value16 = ref.addend() >> 16;
     else
       value16 = (targetAddress + ref.addend()) >> 16;
     *loc32 = setWordFromArmMov(*loc32, value16);
-    break;
+    return;
   case arm_movw_funcRel:
     assert(!thumbMode);
     value16 = (targetAddress - inAtomAddress + ref.addend()) & 0xFFFF;
     *loc32 = setWordFromArmMov(*loc32, value16);
-    break;
+    return;
   case arm_movt_funcRel:
     assert(!thumbMode);
     value16 = (targetAddress - inAtomAddress + ref.addend()) >> 16;
     *loc32 = setWordFromArmMov(*loc32, value16);
-    break;
+    return;
   case pointer32:
     *loc32 = targetAddress + ref.addend();
-    break;
+    return;
   case delta32:
     *loc32 = targetAddress - fixupAddress + ref.addend();
-    break;
+    return;
   case lazyPointer:
   case lazyImmediateLocation:
     // do nothing
-    break;
-  default:
-    llvm_unreachable("invalid ARM Reference Kind");
+    return;
+  case invalid:
+    // Fall into llvm_unreachable().
     break;
   }
+  llvm_unreachable("invalid ARM Reference Kind");
 }
 
 void ArchHandler_arm::appendSectionRelocations(
                                    const DefinedAtom &atom,
                                    uint64_t atomSectionOffset,
                                    const Reference &ref,
                                    FindSymbolIndexForAtom symbolIndexForAtom,
                                    FindSectionIndexForAtom sectionIndexForAtom,
                                    FindAddressForAtom addressForAtom,
                                    normalized::Relocations &relocs) {
   if (ref.kindNamespace() != Reference::KindNamespace::mach_o)
     return;
   assert(ref.kindArch() == Reference::KindArch::ARM);
   uint32_t sectionOffset = atomSectionOffset + ref.offsetInAtom();
   bool useExternalReloc = useExternalRelocationTo(*ref.target());
   uint32_t targetAtomAddress;
   uint32_t fromAtomAddress;
   uint16_t other16;
-  switch (ref.kindValue()) {
+  switch (static_cast<Arm_Kinds>(ref.kindValue())) {
   case modeThumbCode:
   case modeArmCode:
   case modeData:
-    break;
     // Do nothing.
-    break;
+    return;
   case thumb_b22:
   case thumb_bl22:
     if (useExternalReloc) {
       appendReloc(relocs, sectionOffset, symbolIndexForAtom(*ref.target()), 0,
                   ARM_THUMB_RELOC_BR22 | rExtern    | rPcRel | rLength4);
     } else {
       if (ref.addend() != 0)
         appendReloc(relocs, sectionOffset, 0, addressForAtom(*ref.target()),
                   ARM_THUMB_RELOC_BR22 | rScattered | rPcRel | rLength4);
       else
         appendReloc(relocs, sectionOffset, sectionIndexForAtom(*ref.target()),0,
                   ARM_THUMB_RELOC_BR22 |              rPcRel | rLength4);
     }
-    break;
+    return;
   case thumb_movw:
     if (useExternalReloc) {
       other16 = ref.addend() >> 16;
       appendReloc(relocs, sectionOffset, symbolIndexForAtom(*ref.target()), 0,
                   ARM_RELOC_HALF | rExtern    | rLenThmbLo);
       appendReloc(relocs, other16, 0, 0,
                   ARM_RELOC_PAIR              | rLenThmbLo);
     } else {
       targetAtomAddress = addressForAtom(*ref.target());
       if (ref.addend() != 0) {
         other16 = (targetAtomAddress + ref.addend()) >> 16;
         appendReloc(relocs, sectionOffset, 0, targetAtomAddress,
                   ARM_RELOC_HALF | rScattered | rLenThmbLo);
         appendReloc(relocs, other16, 0, 0,
                   ARM_RELOC_PAIR              | rLenThmbLo);
       } else {
         other16 = (targetAtomAddress + ref.addend()) >> 16;
         appendReloc(relocs, sectionOffset, sectionIndexForAtom(*ref.target()),0,
                   ARM_RELOC_HALF              | rLenThmbLo);
         appendReloc(relocs, other16, 0, 0,
                   ARM_RELOC_PAIR              | rLenThmbLo);
       }
     }
-    break;
+    return;
   case thumb_movt:
     if (useExternalReloc) {
       other16 = ref.addend() & 0xFFFF;
       appendReloc(relocs, sectionOffset, symbolIndexForAtom(*ref.target()), 0,
                   ARM_RELOC_HALF | rExtern    | rLenThmbHi);
       appendReloc(relocs, other16, 0, 0,
                   ARM_RELOC_PAIR              | rLenThmbHi);
     } else {
       targetAtomAddress = addressForAtom(*ref.target());
       if (ref.addend() != 0) {
         other16 = (targetAtomAddress + ref.addend()) & 0xFFFF;
         appendReloc(relocs, sectionOffset, 0, targetAtomAddress,
                     ARM_RELOC_HALF | rScattered | rLenThmbHi);
         appendReloc(relocs, other16, 0, 0,
                     ARM_RELOC_PAIR              | rLenThmbHi);
       } else {
         other16 = (targetAtomAddress + ref.addend()) & 0xFFFF;
         appendReloc(relocs, sectionOffset, sectionIndexForAtom(*ref.target()),0,
                     ARM_RELOC_HALF              | rLenThmbHi);
         appendReloc(relocs, other16, 0, 0,
                     ARM_RELOC_PAIR              | rLenThmbHi);
       }
     }
-    break;
+    return;
   case thumb_movw_funcRel:
     fromAtomAddress = addressForAtom(atom);
     targetAtomAddress = addressForAtom(*ref.target());
     other16 = (targetAtomAddress - fromAtomAddress + ref.addend()) >> 16;
     appendReloc(relocs, sectionOffset, 0, targetAtomAddress,
                 ARM_RELOC_HALF_SECTDIFF | rScattered | rLenThmbLo);
     appendReloc(relocs, other16, 0, fromAtomAddress,
                 ARM_RELOC_PAIR          | rScattered | rLenThmbLo);
-    break;
+    return;
   case thumb_movt_funcRel:
     fromAtomAddress = addressForAtom(atom);
     targetAtomAddress = addressForAtom(*ref.target());
     other16 = (targetAtomAddress - fromAtomAddress + ref.addend()) & 0xFFFF;
     appendReloc(relocs, sectionOffset, 0, targetAtomAddress,
                 ARM_RELOC_HALF_SECTDIFF | rScattered | rLenThmbHi);
     appendReloc(relocs, other16, 0, fromAtomAddress,
                 ARM_RELOC_PAIR          | rScattered | rLenThmbHi);
-    break;
+    return;
   case arm_b24:
   case arm_bl24:
     if (useExternalReloc) {
       appendReloc(relocs, sectionOffset, symbolIndexForAtom(*ref.target()), 0,
                   ARM_RELOC_BR24 | rExtern    | rPcRel | rLength4);
     } else {
       if (ref.addend() != 0)
         appendReloc(relocs, sectionOffset, 0, addressForAtom(*ref.target()),
                   ARM_RELOC_BR24 | rScattered | rPcRel | rLength4);
       else
         appendReloc(relocs, sectionOffset, sectionIndexForAtom(*ref.target()),0,
                   ARM_RELOC_BR24 |              rPcRel | rLength4);
     }
-    break;
+    return;
   case arm_movw:
     if (useExternalReloc) {
       other16 = ref.addend() >> 16;
       appendReloc(relocs, sectionOffset, symbolIndexForAtom(*ref.target()), 0,
                   ARM_RELOC_HALF | rExtern    | rLenArmLo);
       appendReloc(relocs, other16, 0, 0,
                   ARM_RELOC_PAIR              | rLenArmLo);
     } else {
       targetAtomAddress = addressForAtom(*ref.target());
       if (ref.addend() != 0) {
         other16 = (targetAtomAddress + ref.addend()) >> 16;
         appendReloc(relocs, sectionOffset, 0, targetAtomAddress,
                   ARM_RELOC_HALF | rScattered | rLenArmLo);
         appendReloc(relocs, other16, 0, 0,
                   ARM_RELOC_PAIR              | rLenArmLo);
       } else {
         other16 = (targetAtomAddress + ref.addend()) >> 16;
         appendReloc(relocs, sectionOffset, sectionIndexForAtom(*ref.target()),0,
                   ARM_RELOC_HALF              | rLenArmLo);
         appendReloc(relocs, other16, 0, 0,
                   ARM_RELOC_PAIR              | rLenArmLo);
       }
     }
-    break;
+    return;
   case arm_movt:
     if (useExternalReloc) {
       other16 = ref.addend() & 0xFFFF;
       appendReloc(relocs, sectionOffset, symbolIndexForAtom(*ref.target()), 0,
                   ARM_RELOC_HALF | rExtern    | rLenArmHi);
       appendReloc(relocs, other16, 0, 0,
                   ARM_RELOC_PAIR              | rLenArmHi);
     } else {
       targetAtomAddress = addressForAtom(*ref.target());
       if (ref.addend() != 0) {
         other16 = (targetAtomAddress + ref.addend()) & 0xFFFF;
         appendReloc(relocs, sectionOffset, 0, targetAtomAddress,
                   ARM_RELOC_HALF | rScattered | rLenArmHi);
         appendReloc(relocs, other16, 0, 0,
                   ARM_RELOC_PAIR              | rLenArmHi);
       } else {
         other16 = (targetAtomAddress + ref.addend()) & 0xFFFF;
         appendReloc(relocs, sectionOffset, sectionIndexForAtom(*ref.target()),0,
                   ARM_RELOC_HALF              | rLenArmHi);
         appendReloc(relocs, other16, 0, 0,
                   ARM_RELOC_PAIR              | rLenArmHi);
       }
     }
-    break;
+    return;
   case arm_movw_funcRel:
     fromAtomAddress = addressForAtom(atom);
     targetAtomAddress = addressForAtom(*ref.target());
     other16 = (targetAtomAddress - fromAtomAddress + ref.addend()) >> 16;
     appendReloc(relocs, sectionOffset, 0, targetAtomAddress,
                 ARM_RELOC_HALF_SECTDIFF | rScattered | rLenArmLo);
     appendReloc(relocs, other16, 0, fromAtomAddress,
                 ARM_RELOC_PAIR          | rScattered | rLenArmLo);
-    break;
+    return;
   case arm_movt_funcRel:
     fromAtomAddress = addressForAtom(atom);
     targetAtomAddress = addressForAtom(*ref.target());
     other16 = (targetAtomAddress - fromAtomAddress + ref.addend()) & 0xFFFF;
     appendReloc(relocs, sectionOffset, 0, targetAtomAddress,
                 ARM_RELOC_HALF_SECTDIFF | rScattered | rLenArmHi);
     appendReloc(relocs, other16, 0, fromAtomAddress,
                 ARM_RELOC_PAIR          | rScattered | rLenArmHi);
-    break;
+    return;
   case pointer32:
     if (useExternalReloc) {
       appendReloc(relocs, sectionOffset, symbolIndexForAtom(*ref.target()),  0,
                 ARM_RELOC_VANILLA |    rExtern     |  rLength4);
     }
     else {
       if (ref.addend() != 0)
         appendReloc(relocs, sectionOffset, 0, addressForAtom(*ref.target()),
                 ARM_RELOC_VANILLA |    rScattered  |  rLength4);
       else
         appendReloc(relocs, sectionOffset, sectionIndexForAtom(*ref.target()),0,
                 ARM_RELOC_VANILLA |                   rLength4);
     }
-    break;
+    return;
   case delta32:
     appendReloc(relocs, sectionOffset, 0, addressForAtom(*ref.target()),
               ARM_RELOC_SECTDIFF  |  rScattered    | rLength4);
     appendReloc(relocs, sectionOffset, 0, addressForAtom(atom) +
                                                            ref.offsetInAtom(),
               ARM_RELOC_PAIR      |  rScattered    | rLength4);
-    break;
+    return;
   case lazyPointer:
   case lazyImmediateLocation:
     // do nothing
-    break;
-  default:
-    llvm_unreachable("invalid ARM Reference Kind");
+    return;
+  case invalid:
+    // Fall into llvm_unreachable().
     break;
   }
+  llvm_unreachable("invalid ARM Reference Kind");
 }
 
 void ArchHandler_arm::addAdditionalReferences(MachODefinedAtom &atom) {
   if (atom.isThumb()) {
     atom.addReference(0, modeThumbCode, &atom, 0, Reference::KindArch::ARM);
   }
 }
 

lib/ReaderWriter/MachO/ArchHandler_x86.cpp

Reference::KindValue dataInCodeTransitionEnd(
                                         const MachODefinedAtom &atom) override {
     return modeCode;
   }
 
 private:
   static const Registry::KindStrings _sKindStrings[];
   static const StubInfo              _sStubInfo;
 
-  enum : Reference::KindValue {
+  enum X86_Kinds : Reference::KindValue {
     invalid,               /// for error condition
 
     modeCode,              /// Content starting at this offset is code.
     modeData,              /// Content starting at this offset is data.
 
     // Kinds found in mach-o .o files:
     branch32,              /// ex: call _foo
     branch16,              /// ex: callw _foo

 void ArchHandler_x86::applyFixupFinal(const Reference &ref, uint8_t *loc,
                                       uint64_t fixupAddress,
                                       uint64_t targetAddress,
                                       uint64_t inAtomAddress) {
   if (ref.kindNamespace() != Reference::KindNamespace::mach_o)
     return;
   assert(ref.kindArch() == Reference::KindArch::x86);
   ulittle32_t *loc32 = reinterpret_cast<ulittle32_t *>(loc);
-  switch (ref.kindValue()) {
+  switch (static_cast<X86_Kinds>(ref.kindValue())) {
   case branch32:
     *loc32 = (targetAddress - (fixupAddress + 4)) + ref.addend();
-    break;
+    return;
   case branch16:
     *loc32 = (targetAddress - (fixupAddress + 2)) + ref.addend();
-    break;
+    return;
   case pointer32:
   case abs32:
     *loc32 = targetAddress + ref.addend();
-    break;
+    return;
   case funcRel32:
     *loc32 = targetAddress - inAtomAddress + ref.addend();
-    break;
+    return;
   case delta32:
     *loc32 = targetAddress - fixupAddress + ref.addend();
-    break;
+    return;
   case negDelta32:
     *loc32 = fixupAddress - targetAddress + ref.addend();
-    break;
+    return;
   case modeCode:
   case modeData:
   case lazyPointer:
     // do nothing
-    break;
+    return;
   case lazyImmediateLocation:
     *loc32 = ref.addend();
-    break;
-  default:
-    llvm_unreachable("invalid x86 Reference Kind");
+    return;
+  case invalid:
+    // Fall into llvm_unreachable().
     break;
   }
+  llvm_unreachable("invalid x86 Reference Kind");
 }
 
 void ArchHandler_x86::applyFixupRelocatable(const Reference &ref,
                                                uint8_t *loc,
                                                uint64_t fixupAddress,
                                                uint64_t targetAddress,
                                                uint64_t inAtomAddress) {
   bool useExternalReloc = useExternalRelocationTo(*ref.target());
   ulittle16_t *loc16 = reinterpret_cast<ulittle16_t *>(loc);
   ulittle32_t *loc32 = reinterpret_cast<ulittle32_t *>(loc);
-  switch (ref.kindValue()) {
+  switch (static_cast<X86_Kinds>(ref.kindValue())) {
   case branch32:
     if (useExternalReloc)
       *loc32 = ref.addend() - (fixupAddress + 4);
     else
       *loc32  =(targetAddress - (fixupAddress+4)) + ref.addend();
-    break;
+    return;
   case branch16:
     if (useExternalReloc)
       *loc16 = ref.addend() - (fixupAddress + 2);
     else
       *loc16 = (targetAddress - (fixupAddress+2)) + ref.addend();
-    break;
+    return;
   case pointer32:
   case abs32:
     *loc32 = targetAddress + ref.addend();
-    break;
+    return;
   case funcRel32:
     *loc32 = targetAddress - inAtomAddress + ref.addend(); // FIXME
-    break;
+    return;
   case delta32:
     *loc32 = targetAddress - fixupAddress + ref.addend();
-    break;
+    return;
   case negDelta32:
     *loc32 = fixupAddress - targetAddress + ref.addend();
-    break;
+    return;
   case modeCode:
   case modeData:
   case lazyPointer:
   case lazyImmediateLocation:
     // do nothing
-    break;
-  default:
-    llvm_unreachable("invalid x86 Reference Kind");
+    return;
+  case invalid:
+    // Fall into llvm_unreachable().
     break;
   }
+  llvm_unreachable("invalid x86 Reference Kind");
 }
 
 bool ArchHandler_x86::useExternalRelocationTo(const Atom &target) {
   // Undefined symbols are referenced via external relocations.
   if (isa<UndefinedAtom>(&target))
     return true;
   if (const DefinedAtom *defAtom = dyn_cast<DefinedAtom>(&target)) {
      switch (defAtom->merge()) {
void ArchHandler_x86::appendSectionRelocations(
                                    FindSectionIndexForAtom sectionIndexForAtom,
                                    FindAddressForAtom addressForAtom,
                                    normalized::Relocations &relocs) {
   if (ref.kindNamespace() != Reference::KindNamespace::mach_o)
     return;
   assert(ref.kindArch() == Reference::KindArch::x86);
   uint32_t sectionOffset = atomSectionOffset + ref.offsetInAtom();
   bool useExternalReloc = useExternalRelocationTo(*ref.target());
-  switch (ref.kindValue()) {
+  switch (static_cast<X86_Kinds>(ref.kindValue())) {
   case modeCode:
   case modeData:
     break;
   case branch32:
     if (useExternalReloc) {
       appendReloc(relocs, sectionOffset, symbolIndexForAtom(*ref.target()), 0,
                   GENERIC_RELOC_VANILLA | rExtern    | rPcRel | rLength4);
     } else {
       if (ref.addend() != 0)
         appendReloc(relocs, sectionOffset, 0, addressForAtom(*ref.target()),
                   GENERIC_RELOC_VANILLA | rScattered | rPcRel |  rLength4);
       else
         appendReloc(relocs, sectionOffset, sectionIndexForAtom(*ref.target()),0,
                   GENERIC_RELOC_VANILLA |              rPcRel | rLength4);
     }
-    break;
+    return;
   case branch16:
     if (useExternalReloc) {
       appendReloc(relocs, sectionOffset, symbolIndexForAtom(*ref.target()), 0,
                   GENERIC_RELOC_VANILLA | rExtern    | rPcRel | rLength2);
     } else {
       if (ref.addend() != 0)
         appendReloc(relocs, sectionOffset, 0, addressForAtom(*ref.target()),
                   GENERIC_RELOC_VANILLA | rScattered | rPcRel |  rLength2);
       else
         appendReloc(relocs, sectionOffset, sectionIndexForAtom(*ref.target()),0,
                   GENERIC_RELOC_VANILLA |              rPcRel | rLength2);
     }
-    break;
+    return;
   case pointer32:
   case abs32:
     if (useExternalReloc)
       appendReloc(relocs, sectionOffset, symbolIndexForAtom(*ref.target()),  0,
                 GENERIC_RELOC_VANILLA |    rExtern     |  rLength4);
     else {
       if (ref.addend() != 0)
         appendReloc(relocs, sectionOffset, 0, addressForAtom(*ref.target()),
                 GENERIC_RELOC_VANILLA |    rScattered  |  rLength4);
       else
         appendReloc(relocs, sectionOffset, sectionIndexForAtom(*ref.target()), 0,
                 GENERIC_RELOC_VANILLA |                   rLength4);
     }
-    break;
+    return;
   case funcRel32:
     appendReloc(relocs, sectionOffset, 0, addressForAtom(*ref.target()),
               GENERIC_RELOC_SECTDIFF |  rScattered    | rLength4);
     appendReloc(relocs, sectionOffset, 0, addressForAtom(atom) - ref.addend(),
               GENERIC_RELOC_PAIR     |  rScattered    | rLength4);
-    break;
+    return;
   case delta32:
     appendReloc(relocs, sectionOffset, 0, addressForAtom(*ref.target()),
               GENERIC_RELOC_SECTDIFF |  rScattered    | rLength4);
     appendReloc(relocs, sectionOffset, 0, addressForAtom(atom) +
                                                            ref.offsetInAtom(),
               GENERIC_RELOC_PAIR     |  rScattered    | rLength4);
-    break;
+    return;
   case negDelta32:
     appendReloc(relocs, sectionOffset, 0, addressForAtom(atom) +
                                                            ref.offsetInAtom(),
               GENERIC_RELOC_SECTDIFF |  rScattered    | rLength4);
     appendReloc(relocs, sectionOffset, 0, addressForAtom(*ref.target()),
               GENERIC_RELOC_PAIR     |  rScattered    | rLength4);
-    break;
+    return;
   case lazyPointer:
   case lazyImmediateLocation:
     llvm_unreachable("lazy reference kind implies Stubs pass was run");
+    return;
+  case invalid:
+    // Fall into llvm_unreachable().
     break;
-  default:
-    llvm_unreachable("unknown x86 Reference Kind");
-    break;
-
   }
+  llvm_unreachable("unknown x86 Reference Kind");
 }
 
 
 std::unique_ptr<mach_o::ArchHandler> ArchHandler::create_x86() {
   return std::unique_ptr<mach_o::ArchHandler>(new ArchHandler_x86());
 }
 
 } // namespace mach_o
 } // namespace lld