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 https://github.com/Semmle/ql/pull/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.
This commit is contained in:
Dave Bartolomeo 2020-02-21 18:46:32 -07:00
Родитель de66841263
Коммит 170331b105
4 изменённых файлов: 29 добавлений и 21 удалений

Просмотреть файл

@ -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 { abstract class BufferAccessOpcode extends Opcode {
final override predicate hasAddressOperand() { any() } 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. * 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 } final override MemoryAccessKind getWriteMemoryAccess() { result instanceof BufferMemoryAccess }
} }
/** /**
* An opcode that reads from a memory buffer of unknown size. * 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 } 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`. * An opcode that accesses a memory buffer whose size is determined by a `BufferSizeOperand`.
*/ */
abstract class SizedBufferAccessOpcode extends Opcode { abstract class SizedBufferAccessOpcode extends BufferAccessOpcode {
final override predicate hasAddressOperand() { any() }
final override predicate hasBufferSizeOperand() { any() } final override predicate hasBufferSizeOperand() { any() }
} }
@ -666,16 +670,16 @@ module Opcode {
final override string toString() { result = "IndirectMayWriteSideEffect" } final override string toString() { result = "IndirectMayWriteSideEffect" }
} }
class BufferReadSideEffect extends ReadSideEffectOpcode, BufferReadOpcode, TBufferReadSideEffect { class BufferReadSideEffect extends ReadSideEffectOpcode, UnsizedBufferReadOpcode, TBufferReadSideEffect {
final override string toString() { result = "BufferReadSideEffect" } final override string toString() { result = "BufferReadSideEffect" }
} }
class BufferMustWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode, class BufferMustWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode,
TBufferMustWriteSideEffect { TBufferMustWriteSideEffect {
final override string toString() { result = "BufferMustWriteSideEffect" } final override string toString() { result = "BufferMustWriteSideEffect" }
} }
class BufferMayWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode, MayWriteOpcode, class BufferMayWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode, MayWriteOpcode,
TBufferMayWriteSideEffect { TBufferMayWriteSideEffect {
final override string toString() { result = "BufferMayWriteSideEffect" } final override string toString() { result = "BufferMayWriteSideEffect" }
} }

Просмотреть файл

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

Просмотреть файл

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

Просмотреть файл

@ -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 { abstract class BufferAccessOpcode extends Opcode {
final override predicate hasAddressOperand() { any() } 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. * 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 } final override MemoryAccessKind getWriteMemoryAccess() { result instanceof BufferMemoryAccess }
} }
/** /**
* An opcode that reads from a memory buffer of unknown size. * 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 } 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`. * An opcode that accesses a memory buffer whose size is determined by a `BufferSizeOperand`.
*/ */
abstract class SizedBufferAccessOpcode extends Opcode { abstract class SizedBufferAccessOpcode extends BufferAccessOpcode {
final override predicate hasAddressOperand() { any() }
final override predicate hasBufferSizeOperand() { any() } final override predicate hasBufferSizeOperand() { any() }
} }
@ -666,16 +670,16 @@ module Opcode {
final override string toString() { result = "IndirectMayWriteSideEffect" } final override string toString() { result = "IndirectMayWriteSideEffect" }
} }
class BufferReadSideEffect extends ReadSideEffectOpcode, BufferReadOpcode, TBufferReadSideEffect { class BufferReadSideEffect extends ReadSideEffectOpcode, UnsizedBufferReadOpcode, TBufferReadSideEffect {
final override string toString() { result = "BufferReadSideEffect" } final override string toString() { result = "BufferReadSideEffect" }
} }
class BufferMustWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode, class BufferMustWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode,
TBufferMustWriteSideEffect { TBufferMustWriteSideEffect {
final override string toString() { result = "BufferMustWriteSideEffect" } final override string toString() { result = "BufferMustWriteSideEffect" }
} }
class BufferMayWriteSideEffect extends WriteSideEffectOpcode, BufferWriteOpcode, MayWriteOpcode, class BufferMayWriteSideEffect extends WriteSideEffectOpcode, UnsizedBufferWriteOpcode, MayWriteOpcode,
TBufferMayWriteSideEffect { TBufferMayWriteSideEffect {
final override string toString() { result = "BufferMayWriteSideEffect" } final override string toString() { result = "BufferMayWriteSideEffect" }
} }