fuzzMap calls in VectorFuzzer yield inconsistent results across platf…

…orms (#3561)

Summary:
Pull Request resolved: #3561

The order arguments are evaluated in a function call in C++ is not specified.

This leads to issues when we call functions that invoke our RNG in the VectorFuzzer in two arguments, like these fuzzMap calls where we call one such function for the keys and one for the values.

This came up when trying to repro
#3419

In the CircleCI environment the values were generated before the keys when calling fuzzMap, while in my local environment the keys were generated before the values.  I was getting different results and unable to reproduce the issue.

Constructing the arguments outside the function call allows us to control the order in which their invoked.

Reviewed By: bikramSingh91, kagamiori

Differential Revision: D42178884

fbshipit-source-id: 15bc02b9b275bc0914c1538754c7260a01406090

velox/vector/fuzzer/VectorFuzzer.cpp

@@ -418,13 +418,15 @@ VectorPtr VectorFuzzer::fuzzFlat(const TypePtr& type, vector_size_t size) {
   }
   // Maps.
   else if (type->isMap()) {
-    return fuzzMap(
-        opts_.normalizeMapKeys
-            ? fuzzFlatNotNull(
-                  type->asMap().keyType(), size * opts_.containerLength)
-            : fuzzFlat(type->asMap().keyType(), size * opts_.containerLength),
-        fuzzFlat(type->asMap().valueType(), size * opts_.containerLength),
-        size);
+    // Do not initialize keys and values inline in the fuzzMap call as C++ does
+    // not specify the order they'll be called in, leading to inconsistent
+    // results across platforms.
+    auto keys = opts_.normalizeMapKeys
+        ? fuzzFlatNotNull(type->asMap().keyType(), size * opts_.containerLength)
+        : fuzzFlat(type->asMap().keyType(), size * opts_.containerLength);
+    auto values =
+        fuzzFlat(type->asMap().valueType(), size * opts_.containerLength);
+    return fuzzMap(keys, values, size);
   }
   // Rows.
   else if (type->isRow()) {
@@ -480,14 +482,17 @@ VectorPtr VectorFuzzer::fuzzComplex(const TypePtr& type, vector_size_t size) {
           fuzz(type->asArray().elementType(), size * opts_.containerLength),
           size);
 
-    case TypeKind::MAP:
-      return fuzzMap(
-          opts_.normalizeMapKeys
-              ? fuzzNotNull(
-                    type->asMap().keyType(), size * opts_.containerLength)
-              : fuzz(type->asMap().keyType(), size * opts_.containerLength),
-          fuzz(type->asMap().valueType(), size * opts_.containerLength),
-          size);
+    case TypeKind::MAP: {
+      // Do not initialize keys and values inline in the fuzzMap call as C++
+      // does not specify the order they'll be called in, leading to
+      // inconsistent results across platforms.
+      auto keys = opts_.normalizeMapKeys
+          ? fuzzNotNull(type->asMap().keyType(), size * opts_.containerLength)
+          : fuzz(type->asMap().keyType(), size * opts_.containerLength);
+      auto values =
+          fuzz(type->asMap().valueType(), size * opts_.containerLength);
+      return fuzzMap(keys, values, size);
+    }
 
     default:
       VELOX_UNREACHABLE();

velox/vector/fuzzer/VectorFuzzer.h

@@ -287,6 +287,9 @@ class VectorFuzzer {
 
   memory::MemoryPool* pool_;
 
+  // Be careful not to call any functions that use rng_ inline as arguments to a
+  // function.  C++ does not guarantee the order in which arguments are
+  // evaluated, which can lead to inconsistent results across platforms.
   FuzzerGenerator rng_;
 };