Merge pull request #1550 from jbj/array-aggregate-perf

dave-bartolomeo · web-flow · commit 19d2f3a54e2b · 2019-08-12T06:53:16.000-07:00
C++ IR: Fix performance of large value-initialized arrays
diff --git a/cpp/ql/src/semmle/code/cpp/exprs/Literal.qll b/cpp/ql/src/semmle/code/cpp/exprs/Literal.qll
@@ -255,9 +255,10 @@ class ArrayAggregateLiteral extends AggregateLiteral {
    * list, either explicitly with an expression, or implicitly value
    * initialized.
    */
-  pragma[inline]
+  bindingset[elementIndex]
   predicate isInitialized(int elementIndex) {
-    elementIndex in [0..arrayType.getArraySize() - 1]
+    elementIndex >= 0 and
+    elementIndex < arrayType.getArraySize()
   }
 
   /**
@@ -268,7 +269,7 @@ class ArrayAggregateLiteral extends AggregateLiteral {
    * of an object to `false`, `0`, `nullptr`, or by calling the default
    * constructor, as appropriate to the type.
    */
-  pragma[inline]
+  bindingset[elementIndex]
   predicate isValueInitialized(int elementIndex) {
     isInitialized(elementIndex) and
     not exists(getElementExpr(elementIndex))
diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/InstructionTag.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/InstructionTag.qll
@@ -1,20 +1,5 @@
 private import cpp
 
-private predicate fieldIsInitialized(Field field) {
-  exists(ClassAggregateLiteral initList |
-    initList.isInitialized(field)
-  ) or
-  exists(ConstructorFieldInit init |
-    field = init.getTarget()
-  )
-}
-
-private predicate elementIsInitialized(int elementIndex) {
-  exists(ArrayAggregateLiteral initList |
-    initList.isInitialized(elementIndex)
-  )
-}
-
 newtype TInstructionTag =
   OnlyInstructionTag() or  // Single instruction (not including implicit Load)
   InitializeThisTag() or
@@ -64,27 +49,13 @@ newtype TInstructionTag =
   ThrowTag() or
   UnwindTag() or
   InitializerUninitializedTag() or
-  InitializerFieldAddressTag(Field field) {
-    fieldIsInitialized(field)
-  } or
-  InitializerFieldDefaultValueTag(Field field) {
-    fieldIsInitialized(field)
-  } or
-  InitializerFieldDefaultValueStoreTag(Field field) {
-    fieldIsInitialized(field)
-  } or
-  InitializerElementIndexTag(int elementIndex) {
-    elementIsInitialized(elementIndex)
-  } or
-  InitializerElementAddressTag(int elementIndex) {
-    elementIsInitialized(elementIndex)
-  } or
-  InitializerElementDefaultValueTag(int elementIndex) {
-    elementIsInitialized(elementIndex)
-  } or
-  InitializerElementDefaultValueStoreTag(int elementIndex) {
-    elementIsInitialized(elementIndex)
-  } or
+  InitializerFieldAddressTag() or
+  InitializerFieldDefaultValueTag() or
+  InitializerFieldDefaultValueStoreTag() or
+  InitializerElementIndexTag() or
+  InitializerElementAddressTag() or
+  InitializerElementDefaultValueTag() or
+  InitializerElementDefaultValueStoreTag() or
   AsmTag() or
   AsmInputTag(int elementIndex) {
     exists(AsmStmt asm |
@@ -150,24 +121,13 @@ string getInstructionTagId(TInstructionTag tag) {
   tag = CatchTag() and result = "Catch" or
   tag = ThrowTag() and result = "Throw" or
   tag = UnwindTag() and result = "Unwind" or
-  exists(Field field, Class cls, int index, string tagName |
-    field = cls.getCanonicalMember(index) and
-    (
-      tag = InitializerFieldAddressTag(field) and tagName = "InitFieldAddr" or
-      tag = InitializerFieldDefaultValueTag(field) and tagName = "InitFieldDefVal" or
-      tag = InitializerFieldDefaultValueStoreTag(field) and tagName = "InitFieldDefValStore"
-    ) and
-    result = tagName + "(" + index + ")"
-  ) or
-  exists(int index, string tagName |
-    (
-      tag = InitializerElementIndexTag(index) and tagName = "InitElemIndex" or
-      tag = InitializerElementAddressTag(index) and tagName = "InitElemAddr" or
-      tag = InitializerElementDefaultValueTag(index) and tagName = "InitElemDefVal" or
-      tag = InitializerElementDefaultValueStoreTag(index) and tagName = "InitElemDefValStore"
-    ) and
-    result = tagName + "(" + index + ")"
-  ) or
+  tag = InitializerFieldAddressTag() and result = "InitFieldAddr" or
+  tag = InitializerFieldDefaultValueTag() and result = "InitFieldDefVal" or
+  tag = InitializerFieldDefaultValueStoreTag() and result = "InitFieldDefValStore" or
+  tag = InitializerElementIndexTag() and result = "InitElemIndex" or
+  tag = InitializerElementAddressTag() and result = "InitElemAddr" or
+  tag = InitializerElementDefaultValueTag() and result = "InitElemDefVal" or
+  tag = InitializerElementDefaultValueStoreTag() and result = "InitElemDefValStore" or
   tag = AsmTag() and result = "Asm" or
   exists(int index |
     tag = AsmInputTag(index) and result = "AsmInputTag(" + index + ")"
diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedElement.qll
@@ -441,7 +441,7 @@ private predicate isFirstValueInitializedElementInRange(
   initList.isValueInitialized(elementIndex) and
   (
     elementIndex = 0 or
-    not initList.isValueInitialized(elementIndex - 1)
+    exists(initList.getElementExpr(elementIndex - 1))
   )
 }
 
diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedInitialization.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/TranslatedInitialization.qll
@@ -497,7 +497,7 @@ abstract class TranslatedFieldInitialization extends TranslatedElement {
   }
 
   final InstructionTag getFieldAddressTag() {
-    result = InitializerFieldAddressTag(field)
+    result = InitializerFieldAddressTag()
   }
 
   final Field getField() {
@@ -625,11 +625,11 @@ class TranslatedFieldValueInitialization extends TranslatedFieldInitialization,
   }
 
   private InstructionTag getFieldDefaultValueTag() {
-    result = InitializerFieldDefaultValueTag(field)
+    result = InitializerFieldDefaultValueTag()
   }
 
   private InstructionTag getFieldDefaultValueStoreTag() {
-    result = InitializerFieldDefaultValueStoreTag(field)
+    result = InitializerFieldDefaultValueStoreTag()
   }
 }
 
@@ -699,11 +699,11 @@ abstract class TranslatedElementInitialization extends TranslatedElement {
   abstract int getElementIndex();
 
   final InstructionTag getElementAddressTag() {
-    result = InitializerElementAddressTag(getElementIndex())
+    result = InitializerElementAddressTag()
   }
 
   final InstructionTag getElementIndexTag() {
-    result = InitializerElementIndexTag(getElementIndex())
+    result = InitializerElementIndexTag()
   }
 
   final ArrayAggregateLiteral getInitList() {
@@ -861,11 +861,11 @@ class TranslatedElementValueInitialization extends TranslatedElementInitializati
   }
   
   private InstructionTag getElementDefaultValueTag() {
-    result = InitializerElementDefaultValueTag(elementIndex)
+    result = InitializerElementDefaultValueTag()
   }
 
   private InstructionTag getElementDefaultValueStoreTag() {
-    result = InitializerElementDefaultValueStoreTag(elementIndex)
+    result = InitializerElementDefaultValueStoreTag()
   }
 
   private Type getDefaultValueType() {
diff --git a/cpp/ql/test/library-tests/ir/ir/PrintAST.expected b/cpp/ql/test/library-tests/ir/ir/PrintAST.expected
@@ -6,6 +6,10 @@
 #-----|   params: 
 #-----|     0: [Parameter] p#0
 #-----|         Type = [RValueReferenceType] __va_list_tag &&
+#-----| [Operator,TopLevelFunction] void operator delete(void*)
+#-----|   params: 
+#-----|     0: [Parameter] p#0
+#-----|         Type = [VoidPointerType] void *
 #-----| [Operator,TopLevelFunction] void operator delete(void*, unsigned long)
 #-----|   params: 
 #-----|     0: [Parameter] p#0
@@ -8001,3 +8005,49 @@ ir.cpp:
 # 1147|       2: [Handler] <handler>
 # 1147|         0: [CatchBlock] { ... }
 # 1149|     1: [ReturnStmt] return ...
+perf-regression.cpp:
+#    4| [CopyAssignmentOperator] Big& Big::operator=(Big const&)
+#    4|   params: 
+#-----|     0: [Parameter] p#0
+#-----|         Type = [LValueReferenceType] const Big &
+#    4| [MoveAssignmentOperator] Big& Big::operator=(Big&&)
+#    4|   params: 
+#-----|     0: [Parameter] p#0
+#-----|         Type = [RValueReferenceType] Big &&
+#    4| [CopyConstructor] void Big::Big(Big const&)
+#    4|   params: 
+#-----|     0: [Parameter] p#0
+#-----|         Type = [LValueReferenceType] const Big &
+#    4| [MoveConstructor] void Big::Big(Big&&)
+#    4|   params: 
+#-----|     0: [Parameter] p#0
+#-----|         Type = [RValueReferenceType] Big &&
+#    6| [Constructor] void Big::Big()
+#    6|   params: 
+#    6|   initializations: 
+#    6|     0: [ConstructorFieldInit] constructor init of field buffer
+#    6|         Type = [ArrayType] char[1073741824]
+#    6|         ValueCategory = prvalue
+#    6|       0: [ArrayAggregateLiteral] {...}
+#    6|           Type = [ArrayType] char[1073741824]
+#    6|           ValueCategory = prvalue
+#    6|   body: [Block] { ... }
+#    6|     0: [ReturnStmt] return ...
+#    9| [TopLevelFunction] int main()
+#    9|   params: 
+#    9|   body: [Block] { ... }
+#   10|     0: [DeclStmt] declaration
+#   10|       0: [VariableDeclarationEntry] definition of big
+#   10|           Type = [PointerType] Big *
+#   10|         init: [Initializer] initializer for big
+#   10|           expr: [NewExpr] new
+#   10|               Type = [PointerType] Big *
+#   10|               ValueCategory = prvalue
+#   10|             1: [ConstructorCall] call to Big
+#   10|                 Type = [VoidType] void
+#   10|                 ValueCategory = prvalue
+#   12|     1: [ReturnStmt] return ...
+#   12|       0: [Literal,Zero] 0
+#   12|           Type = [IntType] int
+#   12|           Value = [Literal,Zero] 0
+#   12|           ValueCategory = prvalue
diff --git a/cpp/ql/test/library-tests/ir/ir/perf-regression.cpp b/cpp/ql/test/library-tests/ir/ir/perf-regression.cpp
@@ -0,0 +1,13 @@
+// This test ensures that we can efficiently generate IR for a large
+// value-initialized array.
+
+struct Big {
+  char buffer[1 << 30]; // 1 GiB
+  Big() : buffer() {} // This explicit init of `buffer` makes it value-initialized
+};
+
+int main() {
+  Big *big = new Big;
+
+  return 0;
+}
diff --git a/cpp/ql/test/library-tests/ir/ir/raw_ir.expected b/cpp/ql/test/library-tests/ir/ir/raw_ir.expected
@@ -5269,3 +5269,43 @@ ir.cpp:
 # 1149|     v13_0(void) = NoOp       : 
 # 1133|     v13_1(void) = ReturnVoid : 
 #-----|   Goto -> Block 1
+
+perf-regression.cpp:
+#    6| void Big::Big()
+#    6|   Block 0
+#    6|     v0_0(void)                    = EnterFunction        : 
+#    6|     mu0_1(unknown)                = AliasedDefinition    : 
+#    6|     mu0_2(unknown)                = UnmodeledDefinition  : 
+#    6|     r0_3(glval<Big>)              = InitializeThis       : 
+#    6|     r0_4(glval<char[1073741824]>) = FieldAddress[buffer] : r0_3
+#    6|     r0_5(int)                     = Constant[0]          : 
+#    6|     r0_6(glval<char>)             = PointerAdd           : r0_4, r0_5
+#    6|     r0_7(unknown[1073741824])     = Constant[0]          : 
+#    6|     mu0_8(unknown[1073741824])    = Store                : &:r0_6, r0_7
+#    6|     v0_9(void)                    = NoOp                 : 
+#    6|     v0_10(void)                   = ReturnVoid           : 
+#    6|     v0_11(void)                   = UnmodeledUse         : mu*
+#    6|     v0_12(void)                   = ExitFunction         : 
+
+#    9| int main()
+#    9|   Block 0
+#    9|     v0_0(void)           = EnterFunction                 : 
+#    9|     mu0_1(unknown)       = AliasedDefinition             : 
+#    9|     mu0_2(unknown)       = UnmodeledDefinition           : 
+#   10|     r0_3(glval<Big *>)   = VariableAddress[big]          : 
+#   10|     r0_4(glval<unknown>) = FunctionAddress[operator new] : 
+#   10|     r0_5(unsigned long)  = Constant[1073741824]          : 
+#   10|     r0_6(void *)         = Call                          : func:r0_4, 0:r0_5
+#   10|     mu0_7(unknown)       = ^CallSideEffect               : ~mu0_2
+#   10|     r0_8(Big *)          = Convert                       : r0_6
+#   10|     r0_9(glval<unknown>) = FunctionAddress[Big]          : 
+#   10|     v0_10(void)          = Call                          : func:r0_9, this:r0_8
+#   10|     mu0_11(unknown)      = ^CallSideEffect               : ~mu0_2
+#   10|     mu0_12(Big *)        = Store                         : &:r0_3, r0_8
+#   12|     r0_13(glval<int>)    = VariableAddress[#return]      : 
+#   12|     r0_14(int)           = Constant[0]                   : 
+#   12|     mu0_15(int)          = Store                         : &:r0_13, r0_14
+#    9|     r0_16(glval<int>)    = VariableAddress[#return]      : 
+#    9|     v0_17(void)          = ReturnValue                   : &:r0_16, ~mu0_2
+#    9|     v0_18(void)          = UnmodeledUse                  : mu*
+#    9|     v0_19(void)          = ExitFunction                  : 
diff --git a/cpp/ql/test/library-tests/literals/aggregate_literals/arrays_value_init.ql b/cpp/ql/test/library-tests/literals/aggregate_literals/arrays_value_init.ql
@@ -2,5 +2,6 @@ import cpp
 
 from ArrayType a, ArrayAggregateLiteral al, int i
 where a = al.getType()
+  and i = [0 .. al.getUnspecifiedType().(ArrayType).getArraySize()]
   and al.isValueInitialized(i)
 select al, a, i

Original file line number	Diff line number	Diff line change
`@@ -441,7 +441,7 @@ private predicate isFirstValueInitializedElementInRange(`
`441`	`441`	`initList.isValueInitialized(elementIndex) and`
`442`	`442`	`(`
`443`	`443`	`elementIndex = 0 or`
`444`		`- not initList.isValueInitialized(elementIndex - 1)`
	`444`	`+ exists(initList.getElementExpr(elementIndex - 1))`
`445`	`445`	`)`
`446`	`446`	`}`
`447`	`447`
Original file line number	Diff line number	Diff line change
`@@ -497,7 +497,7 @@ abstract class TranslatedFieldInitialization extends TranslatedElement {`
`497`	`497`	`}`
`498`	`498`
`499`	`499`	`final InstructionTag getFieldAddressTag() {`
`500`		`- result = InitializerFieldAddressTag(field)`
	`500`	`+ result = InitializerFieldAddressTag()`
`501`	`501`	`}`
`502`	`502`
`503`	`503`	`final Field getField() {`
`@@ -625,11 +625,11 @@ class TranslatedFieldValueInitialization extends TranslatedFieldInitialization,`
`625`	`625`	`}`
`626`	`626`
`627`	`627`	`private InstructionTag getFieldDefaultValueTag() {`
`628`		`- result = InitializerFieldDefaultValueTag(field)`
	`628`	`+ result = InitializerFieldDefaultValueTag()`
`629`	`629`	`}`
`630`	`630`
`631`	`631`	`private InstructionTag getFieldDefaultValueStoreTag() {`
`632`		`- result = InitializerFieldDefaultValueStoreTag(field)`
	`632`	`+ result = InitializerFieldDefaultValueStoreTag()`
`633`	`633`	`}`
`634`	`634`	`}`
`635`	`635`
`@@ -699,11 +699,11 @@ abstract class TranslatedElementInitialization extends TranslatedElement {`
`699`	`699`	`abstract int getElementIndex();`
`700`	`700`
`701`	`701`	`final InstructionTag getElementAddressTag() {`
`702`		`- result = InitializerElementAddressTag(getElementIndex())`
	`702`	`+ result = InitializerElementAddressTag()`
`703`	`703`	`}`
`704`	`704`
`705`	`705`	`final InstructionTag getElementIndexTag() {`
`706`		`- result = InitializerElementIndexTag(getElementIndex())`
	`706`	`+ result = InitializerElementIndexTag()`
`707`	`707`	`}`
`708`	`708`
`709`	`709`	`final ArrayAggregateLiteral getInitList() {`
`@@ -861,11 +861,11 @@ class TranslatedElementValueInitialization extends TranslatedElementInitializati`
`861`	`861`	`}`
`862`	`862`
`863`	`863`	`private InstructionTag getElementDefaultValueTag() {`
`864`		`- result = InitializerElementDefaultValueTag(elementIndex)`
	`864`	`+ result = InitializerElementDefaultValueTag()`
`865`	`865`	`}`
`866`	`866`
`867`	`867`	`private InstructionTag getElementDefaultValueStoreTag() {`
`868`		`- result = InitializerElementDefaultValueStoreTag(elementIndex)`
	`868`	`+ result = InitializerElementDefaultValueStoreTag()`
`869`	`869`	`}`
`870`	`870`
`871`	`871`	`private Type getDefaultValueType() {`