C++: Better fix for void type on buffer access

Fixes issue https://github.com/github/codeql-c-analysis-team/issues/20

This change undoes the workaround in github#2736, and replaces it with a fix for the underlying cause. The problem was that the IR construction code for side effects incorrectly assumed that `BufferAccessOpcode` included `SizedBufferAccessOpcode`. I think that was actually a perfectly reasonable assumption to make, so I changed the `Opcode` hierarchy to make it true.

Author: Dave Bartolomeo

cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll

@@ -213,23 +213,29 @@ abstract class IndirectReadOpcode extends IndirectMemoryAccessOpcode {
 }
 
 /**
- * An opcode that accesses a memory buffer of unknown size.
+ * An opcode that accesses a memory buffer.
  */
 abstract class BufferAccessOpcode extends Opcode {
   final override predicate hasAddressOperand() { any() }
 }
 
+/**
+ * An opcode that accesses a memory buffer of unknown size.
+ */
+abstract class UnsizedBufferAccessOpcode extends BufferAccessOpcode {
+}
+
 /**
  * An opcode that writes to a memory buffer of unknown size.
  */
-abstract class BufferWriteOpcode extends BufferAccessOpcode {
+abstract class UnsizedBufferWriteOpcode extends UnsizedBufferAccessOpcode {
   final override MemoryAccessKind getWriteMemoryAccess() { result instanceof BufferMemoryAccess }
 }
 
 /**
  * An opcode that reads from a memory buffer of unknown size.
  */
-abstract class BufferReadOpcode extends BufferAccessOpcode {
+abstract class UnsizedBufferReadOpcode extends UnsizedBufferAccessOpcode {
   final override MemoryAccessKind getReadMemoryAccess() { result instanceof BufferMemoryAccess }
 }
 
@@ -261,9 +267,7 @@ abstract class EntireAllocationReadOpcode extends EntireAllocationAccessOpcode {
 /**
  * An opcode that accesses a memory buffer whose size is determined by a `BufferSizeOperand`.
  */
-abstract class SizedBufferAccessOpcode extends Opcode {
-  final override predicate hasAddressOperand() { any() }
-
+abstract class SizedBufferAccessOpcode extends BufferAccessOpcode {
   final override predicate hasBufferSizeOperand() { any() }
 }
 
@@ -666,16 +670,16 @@ module Opcode {
     final override string toString() { result = "IndirectMayWriteSideEffect" }
   }
 
-  class BufferReadSideEffect extends ReadSideEffectOpcode, BufferReadOpcode, TBufferReadSideEffect {
+  class BufferReadSideEffect extends ReadSideEffectOpcode, UnsizedBufferReadOpcode, TBufferReadSideEffect {
     final override string toString() { result = "BufferReadSideEffect" }
   }
 
-  class BufferMustWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode,
+  class BufferMustWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode,
     TBufferMustWriteSideEffect {
     final override string toString() { result = "BufferMustWriteSideEffect" }
   }
 
-  class BufferMayWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode, MayWriteOpcode,
+  class BufferMayWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode, MayWriteOpcode,
     TBufferMayWriteSideEffect {
     final override string toString() { result = "BufferMayWriteSideEffect" }
   }

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll

@@ -26,7 +26,7 @@ private predicate hasResultMemoryAccess(
     type = languageType.getIRType() and
     isIndirectOrBufferMemoryAccess(instr.getResultMemoryAccess()) and
     (if instr.hasResultMayMemoryAccess() then isMayAccess = true else isMayAccess = false) and
-    if type.getByteSize() > 0
+    if exists(type.getByteSize())
     then endBitOffset = Ints::add(startBitOffset, Ints::mul(type.getByteSize(), 8))
     else endBitOffset = Ints::unknown()
   )
@@ -43,7 +43,7 @@ private predicate hasOperandMemoryAccess(
     type = languageType.getIRType() and
     isIndirectOrBufferMemoryAccess(operand.getMemoryAccess()) and
     (if operand.hasMayReadMemoryAccess() then isMayAccess = true else isMayAccess = false) and
-    if type.getByteSize() > 0
+    if exists(type.getByteSize())
     then endBitOffset = Ints::add(startBitOffset, Ints::mul(type.getByteSize(), 8))
     else endBitOffset = Ints::unknown()
   )

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedCall.qll

@@ -487,7 +487,7 @@ class TranslatedSideEffect extends TranslatedElement, TTranslatedArgumentSideEff
   }
 
   override CppType getInstructionOperandType(InstructionTag tag, TypedOperandTag operandTag) {
-    if hasSpecificReadSideEffect(any(Opcode::BufferReadSideEffect op))
+    if hasSpecificReadSideEffect(any(BufferAccessOpcode op))
     then
       result = getUnknownType() and
       tag instanceof OnlyInstructionTag and

csharp/ql/src/semmle/code/csharp/ir/implementation/Opcode.qll

@@ -213,23 +213,29 @@ abstract class IndirectReadOpcode extends IndirectMemoryAccessOpcode {
 }
 
 /**
- * An opcode that accesses a memory buffer of unknown size.
+ * An opcode that accesses a memory buffer.
  */
 abstract class BufferAccessOpcode extends Opcode {
   final override predicate hasAddressOperand() { any() }
 }
 
+/**
+ * An opcode that accesses a memory buffer of unknown size.
+ */
+abstract class UnsizedBufferAccessOpcode extends BufferAccessOpcode {
+}
+
 /**
  * An opcode that writes to a memory buffer of unknown size.
  */
-abstract class BufferWriteOpcode extends BufferAccessOpcode {
+abstract class UnsizedBufferWriteOpcode extends UnsizedBufferAccessOpcode {
   final override MemoryAccessKind getWriteMemoryAccess() { result instanceof BufferMemoryAccess }
 }
 
 /**
  * An opcode that reads from a memory buffer of unknown size.
  */
-abstract class BufferReadOpcode extends BufferAccessOpcode {
+abstract class UnsizedBufferReadOpcode extends UnsizedBufferAccessOpcode {
   final override MemoryAccessKind getReadMemoryAccess() { result instanceof BufferMemoryAccess }
 }
 
@@ -261,9 +267,7 @@ abstract class EntireAllocationReadOpcode extends EntireAllocationAccessOpcode {
 /**
  * An opcode that accesses a memory buffer whose size is determined by a `BufferSizeOperand`.
  */
-abstract class SizedBufferAccessOpcode extends Opcode {
-  final override predicate hasAddressOperand() { any() }
-
+abstract class SizedBufferAccessOpcode extends BufferAccessOpcode {
   final override predicate hasBufferSizeOperand() { any() }
 }
 
@@ -666,16 +670,16 @@ module Opcode {
     final override string toString() { result = "IndirectMayWriteSideEffect" }
   }
 
-  class BufferReadSideEffect extends ReadSideEffectOpcode, BufferReadOpcode, TBufferReadSideEffect {
+  class BufferReadSideEffect extends ReadSideEffectOpcode, UnsizedBufferReadOpcode, TBufferReadSideEffect {
     final override string toString() { result = "BufferReadSideEffect" }
   }
 
-  class BufferMustWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode,
+  class BufferMustWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode,
     TBufferMustWriteSideEffect {
     final override string toString() { result = "BufferMustWriteSideEffect" }
   }
 
-  class BufferMayWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode, MayWriteOpcode,
+  class BufferMayWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode, MayWriteOpcode,
     TBufferMayWriteSideEffect {
     final override string toString() { result = "BufferMayWriteSideEffect" }
   }