Silence GCC's `-Wclass-memaccess` warnings

[chandlerc]

Summary

For folks hosting with GCC, this warning is especially annoying. It is
difficult to disable on the command line in all build systems because it
is a C++ specific warning. Disabling it produces still more warnings on
C code.

Suppressing the warning can be done easily by casting the pointer to
void *, but I'm also happy with any other suggested approach to
silence this warning in the source code.

[jhenderson]

I've made two inline suggestions, but I'm not at all familiar with the other code, so can't further assist there.

llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp

1719–1720

This should work, right?

llvm/tools/llvm-nm/llvm-nm.cpp

1638–1639

Can't you do this here (like in other places earlier in the code)? Not sure the memset is actually needed with that.

[chandlerc]

llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp

1719–1720

Not necessarily?

They have at least potentially different semantics (for example, if the default constructor would leave some members uninitialized for some reason). I'm not super familiar with this code and didn't really want to actually change its meaning, just make the compiler warning stop. ;]

If you or another are confident that the code doesn't need the memset, I can switch. Stylistically, I'd suggest:

InternalInstruction Isns = {};

But that's a minor point.

[jhenderson]

llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp

1719–1720

Looking at the definition of InternalInstruction, I see usage of ArrayRef, various enum and integer types, bools and a pointer. There is no user-specified default constructor, so this should result in everything being value initialized, which with the potential exception of the ArrayRef, is the same as setting everything to zero in this context. Setting ArrayRef to zero using memset looks like it's the same as the ArrayRef default constructor too, so it should be safe, I believe.

NB: I am by no means an expert in this code area, so I might be missing something.

I'm more confident in the llvm-nm case, and the same logic applies - there is no default constructor, and the non-trivial member types all zero-initialize themselves when default constructed.

[rupprecht]

llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp

1719–1720

IMHO, we should respect the intent of that warning and not circumvent what constructors/assignment/copy constructors should be doing controlling. i.e. I prefer your suggestion of:

InternalInstruction Isns = {};

(and similar elsewhere)

As for preferring to keep this a NFC commit by silencing the warning with (void *) -- we have test coverage that should verify a semantic change is OK; we shouldn't treat these bits like haunted graveyards.

llvm/lib/CodeGen/RegisterPressure.cpp

if (AddFlagsMI != nullptr) {
     }
   }
 }
 
 /// Initialize an array of N PressureDiffs.
 void PressureDiffs::init(unsigned N) {
   Size = N;
   if (N <= Max) {
-    memset(PDiffArray, 0, N * sizeof(PressureDiff));
+    memset((void *)PDiffArray, 0, N * sizeof(PressureDiff));
     return;
   }
   Max = Size;
   free(PDiffArray);
   PDiffArray = static_cast<PressureDiff*>(safe_calloc(N, sizeof(PressureDiff)));
 }
 
 void PressureDiffs::addInstruction(unsigned Idx,

llvm/lib/ExecutionEngine/ExecutionEngine.cpp

case Type::IntegerTyID:
     break;
   case Type::FloatTyID:
     *((float*)Ptr) = Val.FloatVal;
     break;
   case Type::DoubleTyID:
     *((double*)Ptr) = Val.DoubleVal;
     break;
   case Type::X86_FP80TyID:
-    memcpy(Ptr, Val.IntVal.getRawData(), 10);
+    memcpy((void *)Ptr, Val.IntVal.getRawData(), 10);
     break;
   case Type::PointerTyID:
     // Ensure 64 bit target pointers are fully initialized on 32 bit hosts.
     if (StoreBytes != sizeof(PointerTy))
       memset(&(Ptr->PointerVal), 0, StoreBytes);
 
     *((PointerTy*)Ptr) = Val.PointerVal;
     break;

llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp

X86GenericDisassembler::X86GenericDisassembler(
   llvm_unreachable("Invalid CPU mode");
 }
 
 MCDisassembler::DecodeStatus X86GenericDisassembler::getInstruction(
     MCInst &Instr, uint64_t &Size, ArrayRef<uint8_t> Bytes, uint64_t Address,
     raw_ostream &CStream) const {
   CommentStream = &CStream;
 
   InternalInstruction Insn;
-  memset(&Insn, 0, sizeof(InternalInstruction));
+  memset((void *)&Insn, 0, sizeof(InternalInstruction));
   Insn.bytes = Bytes;
   Insn.startLocation = Address;
   Insn.readerCursor = Address;
   Insn.mode = fMode;
 
   if (Bytes.empty() || readPrefixes(&Insn) || readOpcode(&Insn) ||
       getInstructionID(&Insn, MII.get()) || Insn.instructionID == 0 ||
       readOperands(&Insn)) {

llvm/tools/llvm-nm/llvm-nm.cpp

if (DyldInfoOnly || AddDyldInfo ||
             if (SymbolList[J].Name == Entry.symbolName()) {
                found = true;
                break;
             }
           }
         }
         if (!found) {
           LastSymbolName = Entry.symbolName();
           NMSymbol W;
-          memset(&W, '\0', sizeof(NMSymbol));
+          memset((void *)&W, '\0', sizeof(NMSymbol));
           W.Name = Entry.symbolName();
           W.Address = 0;
           W.Size = 0;
           W.TypeChar = 'U';
           // There is no symbol in the nlist symbol table for this so we set
           // Sym effectivly to null and the rest of code in here must test for
           // it and not do things like Sym.getFlags() for it.
           W.Sym = BasicSymbolRef();