Replace `T(x)` with `reinterpret_cast<T>(x)` everywhere it means reinterpret_cast. No functional change

[• Quuxplusone]

Summary

I wrote a Clang warning [not pictured] to diagnose any use of T(x) which was not equivalent to static_cast<T>(x). Then I built Clang with that warning turned on. Here are all of the instances of "unsafe" T(x) that it identified in the codebase. Every single one is a cast from some pointer type to uintptr_t.

N.B.: The code in llvm/IR/SymbolTableListTraits.h looks absolutely crazy, and this will be the first time it's been touched in over a decade.

[lebedev.ri]

Passing-by remark:

I wrote a Clang warning [not pictured] to diagnose any use of T(x) which was not equivalent to static_cast<T>(x).

I'm not sure whether or not this will pass the bar for a clang diagnostic,
but that does sound like a good clang-tidy readability check.

I also have a bit of pet peeve with T(x) (i.e. including those that should be static_cast<>),
so if you do go with clang-tidy direction, it may be good to generalize it to handle all T(x),
but diagnose only those cast types that are specified in the config.

[grandinj]

Libreoffice has a similar clang-tidy-style cast-checker here:

https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/cstylecast.cxx
https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/test/cstylecast.cxx

[jhenderson]

@Quuxplusone - did you see that there are several clang-format warnings reported against these changes? Please could you fix them.

[• Quuxplusone]

@jhenderson: Updated! clang-format's suggested linebreaks look horrendous to me, but if this makes that buildbot happy, okay... I guess the moral of the story is "deeply nested expressions that also use lots of reinterpret_casts, should be avoided," and I can't argue with that. ;)

[• Quuxplusone]

In D76572#1936191, @grandinj wrote:

Libreoffice has a similar clang-tidy-style cast-checker [for C-style casts] here:

https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/cstylecast.cxx
https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang/test/cstylecast.cxx

Nice. Does LibreOffice have anything (either in clang-tidy or in a paper guideline) against T(x)-style casts? E.g.

struct PB {};
struct D : private PB {};
using FloatRef = float&;
using IntPtr = int*;
using PBPtr = PB*;
int i = 42;
const int *cpi = &i;
D *pd = nullptr;
float& f = FloatRef(i);  // reinterpret_cast
int *pi = IntPtr(cpi);  // const_cast
PB *pb = PBPtr(pd);  // cast to private base

[grandinj]

In D76572#1936861, @Quuxplusone wrote:

Nice. Does LibreOffice have anything (either in clang-tidy or in a paper guideline) against T(x)-style casts? E.g.

No, we don't have very many of those in our codebase, so we have left them alone.
Our plugin is designed to convert c-style casts to modern C++ casts.

[rsmith]

Changes LGTM on the Clang side.

In D76572#1935791, @lebedev.ri wrote:

Passing-by remark:

I wrote a Clang warning [not pictured] to diagnose any use of T(x) which was not equivalent to static_cast<T>(x).

I'm not sure whether or not this will pass the bar for a clang diagnostic

I'd like to try it out on a larger codebase, but it sounds at least potentially good to me. There's a simple syntactic workaround (use (T)x instead of T(x)), and there's a high likelihood that the code doesn't mean what the programmer intended.

Does the warning catch the cases where the code is equivalent to static_cast<T>(x) except that it ignores access? That seems like a really good thing to warn on regardless of whether we warn on the general case.

[• Quuxplusone]

In D76572#1937558, @rsmith wrote:

In D76572#1935791, @lebedev.ri wrote:

Passing-by remark:

I wrote a Clang warning [not pictured] to diagnose any use of T(x) which was not equivalent to static_cast<T>(x).

I'm not sure whether or not this will pass the bar for a clang diagnostic

I'd like to try it out on a larger codebase, but it sounds at least potentially good to me. There's a simple syntactic workaround (use (T)x instead of T(x)), and there's a high likelihood that the code doesn't mean what the programmer intended.

Does the warning catch the cases where the code is equivalent to static_cast<T>(x) except that it ignores access? That seems like a really good thing to warn on regardless of whether we warn on the general case.

Yes, I verified with some simple test cases that it catches the equivalents of (1) reinterpret_cast, (2) const_cast, (3) const_cast plus static_cast, (4) upcast to private base, and (5) downcast to private base.
However, the thing I wrote is 100% not ready for prime time. It basically just inserts a call to TryStaticCast from inside CheckCXXCStyleCast. Unfortunately TryStaticCast is one of these functions that Clang is full of, where it both "checks if X is possible" and also "does X" and also "prints error messages to stdout," all from within the same function. So my patch ends up being morally equivalent to just treating T(x) as static_cast<T>(x); it emits some errors that shouldn't be there if it's supposed to just warn, and I have no idea how to shut up those errors except by completely refactoring TryXXXXCast. (E.g., it could take an extra parameter Mode that indicated whether to actually do the conversion, or just report whether it was possible, or emit notes about why it was impossible.) I wrestled unsuccessfully with similar issues when I did -Wreturn-std-move.

I 100% want to see someone like you tackle this issue, but take my word for it, you don't want to start with the code I wrote the other day. :)

[rsmith]

In D76572#1937641, @Quuxplusone wrote:

[...] take my word for it, you don't want to start with the code I wrote the other day. :)

Understood, thanks :) (FWIW, I had a stab at implementing essentially this last year and hit exactly the issues you highlighted.)

[jhenderson]

LGTM, with nit, with regards to the llvm-readobj and libObject parts. I haven't looked at the other bits though.

llvm/lib/CodeGen/AsmPrinter/OcamlGCPrinter.cpp

150

Nit: I think you can fold these two string literals together. Not sure why they were separate before.

[• Quuxplusone]

Btw, @jhenderson and @rsmith, if you wanted to land your own files' pieces of this patch while waiting on whoever-it-is to approve the pieces in other files, that'd be cool. As I mentioned initially, I don't have the access necessary to land any part of it myself.

llvm/lib/CodeGen/AsmPrinter/OcamlGCPrinter.cpp

150

I think the current code is intentionally parallel to what's on line 162, so I'd prefer to leave it alone and let whoever maintains it reflow it if they want.

[jhenderson]

In D76572#1942024, @Quuxplusone wrote:

Btw, @jhenderson and @rsmith, if you wanted to land your own files' pieces of this patch while waiting on whoever-it-is to approve the pieces in other files, that'd be cool. As I mentioned initially, I don't have the access necessary to land any part of it myself.

From my point of view, I don't see any particular rush to land this, so let's just keep it all as one, and wait for the other bits to be reviewed.

[sberg]

In D76572#1936935, @grandinj wrote:

In D76572#1936861, @Quuxplusone wrote:

Nice. Does LibreOffice have anything (either in clang-tidy or in a paper guideline) against T(x)-style casts? E.g.

No, we don't have very many of those in our codebase, so we have left them alone.
Our plugin is designed to convert c-style casts to modern C++ casts.

Heh, I've meanwhile improved LibreOffice's cstylecast check now (after I'd read only the first few initial comments here), to also flag function-style casts that should be const_/reinterpret_cast, and it indeed only found a handful of cases that would better be reinterpret_cast (https://gerrit.libreoffice.org/plugins/gitiles/core/+/1ebeacb20ad0165e399629fcfd7795ad0da3edf8%5E!/ "Extend loplugin:cstylecast to certain function-style casts"). Though in two of those places it conveniently unearthed code that should rather do without any kind of casting (addressed with follow-up https://gerrit.libreoffice.org/plugins/gitiles/core/+/741d30b5e1b0dcdbafb300ed7c7ad46756ffd946%5E!/ "Simplify pointer equality comparison" and https://gerrit.libreoffice.org/plugins/gitiles/core/+/e3196f3dddad6e7825db3b35e8196be35b466fd9%5E!/ "Fix pointer equality comparision").

[• Quuxplusone]

@DiggerLin @ahatanak @espindola @reames Gentle ping!

[hubert.reinterpretcast]

I am guessing @DiggerLin was pinged with regards to XCOFFObjectFile.cpp. @jhenderson already reviewed and approved the changes to that file (it falls under libObject).

[• Quuxplusone]

Ah. Then I'm not sure who else to ping, or even which pieces remain in need of review.
CODE_OWNERS.TXT isn't being very helpful here...
@kparzysz for HexagonCommonGEP.cpp ("Hexagon backend")?
@Bigcheese for Binary.h ("Object")?

[lebedev.ri]

All these changes are rather NFCI, i'm not sure each one needs to be reviewed.

[Bigcheese]

LGTM for llvm/{lib,include/llvm}/Object/* and llvm/tools/llvm-readobj/*.

[ahatanak]

I'm not sure which part I'm supposed to look at, but the changes LGTM.

[• Quuxplusone]

Rebase in preparation for finally pushing this thing if the buildbots seem happy.

[lebedev.ri]

Do you plan to follow-up with the diagnostic itself, in some form?

[• Quuxplusone]

In D76572#2472221, @lebedev.ri wrote:

Do you plan to follow-up with the diagnostic itself, in some form?

No, I don't. My diagnostic code was just an ad-hoc noisy hack with no hope of ever passing Clang's test suite.
But @oneraynyday's D89765 will be of interest to you.

[lebedev.ri]

In D76572#2472256, @Quuxplusone wrote:

In D76572#2472221, @lebedev.ri wrote:

Do you plan to follow-up with the diagnostic itself, in some form?

No, I don't. My diagnostic code was just an ad-hoc noisy hack with no hope of ever passing Clang's test suite.

Huh.

But @oneraynyday's D89765 will be of interest to you.

Personally, i don't think that patch has hope of being upstreamed in it's current form,
introducing a new tool is directly counter-productive for usefulness/user adoption.

clang/lib/CodeGen/CGCall.h

class CGCallee {
   }
 
 public:
   CGCallee() : KindOrFunctionPointer(SpecialKind::Invalid) {}
 
   /// Construct a callee.  Call this constructor directly when this
   /// isn't a direct call.
   CGCallee(const CGCalleeInfo &abstractInfo, llvm::Value *functionPtr)
-      : KindOrFunctionPointer(SpecialKind(uintptr_t(functionPtr))) {
+      : KindOrFunctionPointer(
+            SpecialKind(reinterpret_cast<uintptr_t>(functionPtr))) {
     AbstractInfo = abstractInfo;
     assert(functionPtr && "configuring callee without function pointer");
     assert(functionPtr->getType()->isPointerTy());
     assert(functionPtr->getType()->getPointerElementType()->isFunctionTy());
   }
 
   static CGCallee forBuiltin(unsigned builtinID,
                              const FunctionDecl *builtinDecl) {
CGCalleeInfo getAbstractInfo() const {
     return AbstractInfo;
   }
   llvm::Value *getFunctionPointer() const {
     assert(isOrdinary());
     return reinterpret_cast<llvm::Value *>(uintptr_t(KindOrFunctionPointer));
   }
   void setFunctionPointer(llvm::Value *functionPtr) {
     assert(isOrdinary());
-    KindOrFunctionPointer = SpecialKind(uintptr_t(functionPtr));
+    KindOrFunctionPointer =
+        SpecialKind(reinterpret_cast<uintptr_t>(functionPtr));
   }
 
   bool isVirtual() const {
     return KindOrFunctionPointer == SpecialKind::Virtual;
   }
   const CallExpr *getVirtualCallExpr() const {
     assert(isVirtual());
     return VirtualInfo.CE;

llvm/include/llvm/IR/SymbolTableListTraits.h

 
 public:
   SymbolTableListTraits() = default;
 
 private:
   /// getListOwner - Return the object that owns this list.  If this is a list
   /// of instructions, it returns the BasicBlock that owns them.
   ItemParentClass *getListOwner() {
-    size_t Offset(size_t(&((ItemParentClass*)nullptr->*ItemParentClass::
-                           getSublistAccess(static_cast<ValueSubClass*>(nullptr)))));
-    ListTy *Anchor(static_cast<ListTy *>(this));
+    size_t Offset = reinterpret_cast<size_t>(
+        &((ItemParentClass *)nullptr->*ItemParentClass::getSublistAccess(
+                                           static_cast<ValueSubClass *>(
+                                               nullptr))));
+    ListTy *Anchor = static_cast<ListTy *>(this);
     return reinterpret_cast<ItemParentClass*>(reinterpret_cast<char*>(Anchor)-
                                               Offset);
   }
 
   static ListTy &getList(ItemParentClass *Par) {
     return Par->*(Par->getSublistAccess((ValueSubClass*)nullptr));
   }
 

llvm/include/llvm/Object/Binary.h

Triple::ObjectFormatType getTripleObjectFormat() const {
     if (isELF())
       return Triple::ELF;
     return Triple::UnknownObjectFormat;
   }
 
   static Error checkOffset(MemoryBufferRef M, uintptr_t Addr,
                            const uint64_t Size) {
     if (Addr + Size < Addr || Addr + Size < Size ||
-        Addr + Size > uintptr_t(M.getBufferEnd()) ||
-        Addr < uintptr_t(M.getBufferStart())) {
+        Addr + Size > reinterpret_cast<uintptr_t>(M.getBufferEnd()) ||
+        Addr < reinterpret_cast<uintptr_t>(M.getBufferStart())) {
       return errorCodeToError(object_error::unexpected_eof);
     }
     return Error::success();
   }
 };
 
 // Create wrappers for C Binding types (see CBindingWrapping.h).
 DEFINE_ISA_CONVERSION_FUNCTIONS(Binary, LLVMBinaryRef)

llvm/lib/CodeGen/AsmPrinter/OcamlGCPrinter.cpp

if (FI.getStrategy().getName() != getStrategy().getName())
       continue;
 
     uint64_t FrameSize = FI.getFrameSize();
     if (FrameSize >= 1 << 16) {
       // Very rude!
       report_fatal_error("Function '" + FI.getFunction().getName() +
                          "' is too large for the ocaml GC! "
                          "Frame size " +
-                         Twine(FrameSize) + ">= 65536.\n"
-                                            "(" +
-                         Twine(uintptr_t(&FI)) + ")");
+                         Twine(FrameSize) +
+                         ">= 65536.\n"
+                         Twine(reinterpret_cast<uintptr_t>(&FI)) + ")");
     }
 
     AP.OutStreamer->AddComment("live roots for " +
                                Twine(FI.getFunction().getName()));
     AP.OutStreamer->AddBlankLine();
 
     for (GCFunctionInfo::iterator J = FI.begin(), JE = FI.end(); J != JE; ++J) {
       size_t LiveCount = FI.live_size(J);

llvm/lib/Object/COFFObjectFile.cpp

static bool checkSize(MemoryBufferRef M, std::error_code &EC, uint64_t Size) {
   return true;
 }
 
 // Sets Obj unless any bytes in [addr, addr + size) fall outsize of m.
 // Returns unexpected_eof if error.
 template <typename T>
 static Error getObject(const T *&Obj, MemoryBufferRef M, const void *Ptr,
                        const uint64_t Size = sizeof(T)) {
-  uintptr_t Addr = uintptr_t(Ptr);
+  uintptr_t Addr = reinterpret_cast<uintptr_t>(Ptr);
   if (Error E = Binary::checkOffset(M, Addr, Size))
     return E;
   Obj = reinterpret_cast<const T *>(Addr);
   return Error::success();
 }
 
 // Decode a string table entry in base 64 (//AAAAAA). Expects \arg Str without
 // prefixed slashes.

 
 // The .debug sections are the only debug sections for COFF
 // (\see MCObjectFileInfo.cpp).
 bool COFFObjectFile::isDebugSection(StringRef SectionName) const {
   return SectionName.startswith(".debug");
 }
 
 unsigned COFFObjectFile::getSectionID(SectionRef Sec) const {
-  uintptr_t Offset =
-      uintptr_t(Sec.getRawDataRefImpl().p) - uintptr_t(SectionTable);
+  uintptr_t Offset = reinterpret_cast<uintptr_t>(Sec.getRawDataRefImpl().p) -
+                     reinterpret_cast<uintptr_t>(SectionTable);
   assert((Offset % sizeof(coff_section)) == 0);
   return (Offset / sizeof(coff_section)) + 1;
 }
 
 bool COFFObjectFile::isSectionVirtual(DataRefImpl Ref) const {
   const coff_section *Sec = toSec(Ref);
   // In COFF, a virtual section won't have any in-file
   // content, so the file pointer to the content will be zero.
if (!NumRelocs)
     return nullptr;
   auto begin = reinterpret_cast<const coff_relocation *>(
       Base + Sec->PointerToRelocations);
   if (Sec->hasExtendedRelocations()) {
     // Skip the first relocation entry repurposed to store the number of
     // relocations.
     begin++;
   }
-  if (auto E = Binary::checkOffset(M, uintptr_t(begin),
+  if (auto E = Binary::checkOffset(M, reinterpret_cast<uintptr_t>(begin),
                                    sizeof(coff_relocation) * NumRelocs)) {
     consumeError(std::move(E));
     return nullptr;
   }
   return begin;
 }
 
 relocation_iterator COFFObjectFile::section_rel_begin(DataRefImpl Ref) const {

 // Returns the file offset for the given RVA.
 Error COFFObjectFile::getRvaPtr(uint32_t Addr, uintptr_t &Res) const {
   for (const SectionRef &S : sections()) {
     const coff_section *Section = getCOFFSection(S);
     uint32_t SectionStart = Section->VirtualAddress;
     uint32_t SectionEnd = Section->VirtualAddress + Section->VirtualSize;
     if (SectionStart <= Addr && Addr < SectionEnd) {
       uint32_t Offset = Addr - SectionStart;
-      Res = uintptr_t(base()) + Section->PointerToRawData + Offset;
+      Res = reinterpret_cast<uintptr_t>(base()) + Section->PointerToRawData +
+            Offset;
       return Error::success();
     }
   }
   return errorCodeToError(object_error::parse_failed);
 }
 
 Error COFFObjectFile::getRvaAndSizeAsBytes(uint32_t RVA, uint32_t Size,
                                            ArrayRef<uint8_t> &Contents) const {
   for (const SectionRef &S : sections()) {
     const coff_section *Section = getCOFFSection(S);
     uint32_t SectionStart = Section->VirtualAddress;
     // Check if this RVA is within the section bounds. Be careful about integer
     // overflow.
     uint32_t OffsetIntoSection = RVA - SectionStart;
     if (SectionStart <= RVA && OffsetIntoSection < Section->VirtualSize &&
         Size <= Section->VirtualSize - OffsetIntoSection) {
-      uintptr_t Begin =
-          uintptr_t(base()) + Section->PointerToRawData + OffsetIntoSection;
+      uintptr_t Begin = reinterpret_cast<uintptr_t>(base()) +
+                        Section->PointerToRawData + OffsetIntoSection;
       Contents =
           ArrayRef<uint8_t>(reinterpret_cast<const uint8_t *>(Begin), Size);
       return Error::success();
     }
   }
   return errorCodeToError(object_error::parse_failed);
 }
 
Error COFFObjectFile::getSectionContents(const coff_section *Sec,
                                          ArrayRef<uint8_t> &Res) const {
   // In COFF, a virtual section won't have any in-file
   // content, so the file pointer to the content will be zero.
   if (Sec->PointerToRawData == 0)
     return Error::success();
   // The only thing that we need to verify is that the contents is contained
   // within the file bounds. We don't need to make sure it doesn't cover other
   // data, as there's nothing that says that is not allowed.
-  uintptr_t ConStart = uintptr_t(base()) + Sec->PointerToRawData;
+  uintptr_t ConStart =
+      reinterpret_cast<uintptr_t>(base()) + Sec->PointerToRawData;
   uint32_t SectionSize = getSectionSize(Sec);
   if (Error E = checkOffset(Data, ConStart, SectionSize))
     return E;
   Res = makeArrayRef(reinterpret_cast<const uint8_t *>(ConStart), SectionSize);
   return Error::success();
 }
 
 const coff_relocation *COFFObjectFile::toRel(DataRefImpl Rel) const {

llvm/lib/Object/ELFObjectFile.cpp

createPtr(MemoryBufferRef Object, bool InitContent) {
   return std::make_unique<ELFObjectFile<ELFT>>(std::move(*Ret));
 }
 
 Expected<std::unique_ptr<ObjectFile>>
 ObjectFile::createELFObjectFile(MemoryBufferRef Obj, bool InitContent) {
   std::pair<unsigned char, unsigned char> Ident =
       getElfArchType(Obj.getBuffer());
   std::size_t MaxAlignment =
-      1ULL << countTrailingZeros(uintptr_t(Obj.getBufferStart()));
+      1ULL << countTrailingZeros(
+          reinterpret_cast<uintptr_t>(Obj.getBufferStart()));
 
   if (MaxAlignment < 2)
     return createError("Insufficient alignment");
 
   if (Ident.first == ELF::ELFCLASS32) {
     if (Ident.second == ELF::ELFDATA2LSB)
       return createPtr<ELF32LE>(Obj, InitContent);
     else if (Ident.second == ELF::ELFDATA2MSB)

llvm/lib/Object/XCOFFObjectFile.cpp

 static const uint8_t SymTypeMask = 0x07;
 static const uint16_t NoRelMask = 0x0001;
 
 // Checks that [Ptr, Ptr + Size) bytes fall inside the memory buffer
 // 'M'. Returns a pointer to the underlying object on success.
 template <typename T>
 static Expected<const T *> getObject(MemoryBufferRef M, const void *Ptr,
                                      const uint64_t Size = sizeof(T)) {
-  uintptr_t Addr = uintptr_t(Ptr);
+  uintptr_t Addr = reinterpret_cast<uintptr_t>(Ptr);
   if (Error E = Binary::checkOffset(M, Addr, Size))
     return std::move(E);
   return reinterpret_cast<const T *>(Addr);
 }
 
 static uintptr_t getWithOffset(uintptr_t Base, ptrdiff_t Offset) {
   return reinterpret_cast<uintptr_t>(reinterpret_cast<const char *>(Base) +
                                      Offset);
XCOFFObjectFile::getSectionContents(DataRefImpl Sec) const {
   uint64_t OffsetToRaw;
   if (is64Bit())
     OffsetToRaw = toSection64(Sec)->FileOffsetToRawData;
   else
     OffsetToRaw = toSection32(Sec)->FileOffsetToRawData;
 
   const uint8_t * ContentStart = base() + OffsetToRaw;
   uint64_t SectionSize = getSectionSize(Sec);
-  if (checkOffset(Data, uintptr_t(ContentStart), SectionSize))
+  if (checkOffset(Data, reinterpret_cast<uintptr_t>(ContentStart), SectionSize))
     return make_error<BinaryError>();
 
   return makeArrayRef(ContentStart,SectionSize);
 }
 
 uint64_t XCOFFObjectFile::getSectionAlignment(DataRefImpl Sec) const {
   uint64_t Result = 0;
   llvm_unreachable("Not yet implemented!");

llvm/lib/Target/Hexagon/HexagonCommonGEP.cpp

for (NodeSymRel::iterator I = Rel.begin(), E = Rel.end(); I != E; ++I)
         return &*I;
     return nullptr;
 }
 
   // Create an ordered pair of GepNode pointers. The pair will be used in
   // determining equality. The only purpose of the ordering is to eliminate
   // duplication due to the commutativity of equality/non-equality.
 static NodePair node_pair(GepNode *N1, GepNode *N2) {
-    uintptr_t P1 = uintptr_t(N1), P2 = uintptr_t(N2);
-    if (P1 <= P2)
-      return std::make_pair(N1, N2);
-    return std::make_pair(N2, N1);
+  uintptr_t P1 = reinterpret_cast<uintptr_t>(N1);
+  uintptr_t P2 = reinterpret_cast<uintptr_t>(N2);
 }
 
 static unsigned node_hash(GepNode *N) {
     // Include everything except flags and parent.
     FoldingSetNodeID ID;
     ID.AddPointer(N->Idx);
     ID.AddPointer(N->PTy);
     return ID.ComputeHash();

llvm/tools/llvm-readobj/ELFDumper.cpp

getLinkAsSymtab(const ELFFile<ELFT> &Obj, const typename ELFT::Shdr &Sec,
   return std::make_pair(*SymsOrErr, *StrTabOrErr);
 }
 
 template <class ELFT>
 Expected<ArrayRef<typename ELFT::Versym>>
 ELFDumper<ELFT>::getVersionTable(const Elf_Shdr &Sec, ArrayRef<Elf_Sym> *SymTab,
                                  StringRef *StrTab) const {
   assert((!SymTab && !StrTab) || (SymTab && StrTab));
-  if (uintptr_t(Obj.base() + Sec.sh_offset) % sizeof(uint16_t) != 0)
+  if (reinterpret_cast<uintptr_t>(Obj.base() + Sec.sh_offset) %
+          sizeof(uint16_t) !=
+      0)
     return createError("the " + describe(Sec) + " is misaligned");
 
   Expected<ArrayRef<Elf_Versym>> VersionsOrErr =
       Obj.template getSectionContentsAsArray<Elf_Versym>(Sec);
   if (!VersionsOrErr)
     return createError("cannot read content of " + describe(Sec) + ": " +
                        toString(VersionsOrErr.takeError()));
 
ELFDumper<ELFT>::getVersionDefinitions(const Elf_Shdr &Sec) const {
 
   std::vector<VerDef> Ret;
   const uint8_t *VerdefBuf = Start;
   for (unsigned I = 1; I <= /*VerDefsNum=*/Sec.sh_info; ++I) {
     if (VerdefBuf + sizeof(Elf_Verdef) > End)
       return createError("invalid " + describe(Sec) + ": version definition " +
                          Twine(I) + " goes past the end of the section");
 
-    if (uintptr_t(VerdefBuf) % sizeof(uint32_t) != 0)
+    if (reinterpret_cast<uintptr_t>(VerdefBuf) % sizeof(uint32_t) != 0)
       return createError(
           "invalid " + describe(Sec) +
           ": found a misaligned version definition entry at offset 0x" +
           Twine::utohexstr(VerdefBuf - Start));
 
     unsigned Version = *reinterpret_cast<const Elf_Half *>(VerdefBuf);
     if (Version != 1)
       return createError("unable to dump " + describe(Sec) + ": version " +
                          Twine(Version) + " is not yet supported");
 
     const Elf_Verdef *D = reinterpret_cast<const Elf_Verdef *>(VerdefBuf);
     VerDef &VD = *Ret.emplace(Ret.end());
     VD.Offset = VerdefBuf - Start;
     VD.Version = D->vd_version;
     VD.Flags = D->vd_flags;
     VD.Ndx = D->vd_ndx;
     VD.Cnt = D->vd_cnt;
     VD.Hash = D->vd_hash;
 
     const uint8_t *VerdauxBuf = VerdefBuf + D->vd_aux;
     for (unsigned J = 0; J < D->vd_cnt; ++J) {
-      if (uintptr_t(VerdauxBuf) % sizeof(uint32_t) != 0)
+      if (reinterpret_cast<uintptr_t>(VerdauxBuf) % sizeof(uint32_t) != 0)
         return createError("invalid " + describe(Sec) +
                            ": found a misaligned auxiliary entry at offset 0x" +
                            Twine::utohexstr(VerdauxBuf - Start));
 
       Expected<VerdAux> AuxOrErr = ExtractNextAux(VerdauxBuf, I);
       if (!AuxOrErr)
         return AuxOrErr.takeError();
 
ELFDumper<ELFT>::getVersionDependencies(const Elf_Shdr &Sec) const {
   const uint8_t *VerneedBuf = Start;
 
   std::vector<VerNeed> Ret;
   for (unsigned I = 1; I <= /*VerneedNum=*/Sec.sh_info; ++I) {
     if (VerneedBuf + sizeof(Elf_Verdef) > End)
       return createError("invalid " + describe(Sec) + ": version dependency " +
                          Twine(I) + " goes past the end of the section");
 
-    if (uintptr_t(VerneedBuf) % sizeof(uint32_t) != 0)
+    if (reinterpret_cast<uintptr_t>(VerneedBuf) % sizeof(uint32_t) != 0)
       return createError(
           "invalid " + describe(Sec) +
           ": found a misaligned version dependency entry at offset 0x" +
           Twine::utohexstr(VerneedBuf - Start));
 
     unsigned Version = *reinterpret_cast<const Elf_Half *>(VerneedBuf);
     if (Version != 1)
       return createError("unable to dump " + describe(Sec) + ": version " +
for (unsigned I = 1; I <= /*VerneedNum=*/Sec.sh_info; ++I) {
 
     if (Verneed->vn_file < StrTab.size())
       VN.File = std::string(StrTab.drop_front(Verneed->vn_file));
     else
       VN.File = "<corrupt vn_file: " + to_string(Verneed->vn_file) + ">";
 
     const uint8_t *VernauxBuf = VerneedBuf + Verneed->vn_aux;
     for (unsigned J = 0; J < Verneed->vn_cnt; ++J) {
-      if (uintptr_t(VernauxBuf) % sizeof(uint32_t) != 0)
+      if (reinterpret_cast<uintptr_t>(VernauxBuf) % sizeof(uint32_t) != 0)
         return createError("invalid " + describe(Sec) +
                            ": found a misaligned auxiliary entry at offset 0x" +
                            Twine::utohexstr(VernauxBuf - Start));
 
       if (VernauxBuf + sizeof(Elf_Vernaux) > End)
         return createError(
             "invalid " + describe(Sec) + ": version dependency " + Twine(I) +
             " refers to an auxiliary entry that goes past the end "