[PATCH] D87755: Silence GCC's `-Wclass-memaccess` warnings

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp:1719-1720
 
   InternalInstruction Insn;
-  memset(&Insn, 0, sizeof(InternalInstruction));
+  memset((void *)&Insn, 0, sizeof(InternalInstruction));
   Insn.bytes = Bytes;
----------------
chandlerc wrote:
> jhenderson wrote:
> > This should work, right?
> 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.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87755/new/

https://reviews.llvm.org/D87755