ngraph-tf
ngraph-tf copied to clipboard
Fix the implicitly constructions of ArraySlice from initializer lists
In 'ngraph_utils.cc', methods like 'const gtl::ArraySlice<DataType>& NGraphNumericDTypes()' directly construct an instance of 'gtl::ArraySlice' from an intializer list. This leads to the corruption of 'DataType' instances held inside the array slice, and therefore no 'DataType' can pass the following check within the 'TypeConstraintOk' method:
DataType dt;
if (GetNodeAttr(node->attrs(), type_attr_name, &dt) != Status::OK() ||
std::find(allowed_types.begin(), allowed_types.end(), dt) ==
allowed_types.end()) {
Since the 'allowed_types' array contains already destructed instances of 'DataType's.
The constructor of 'gtl::ArraySlice' that takes an initializer list has the following comment (see https://github.com/petewarden/tensorflow_makefile/blob/master/tensorflow/core/lib/gtl/array_slice.h):
// Implicitly constructs an ArraySlice from an initializer list. This makes it
// possible to pass a brace-enclosed initializer list to a function expecting
// an ArraySlice:
// void Process(ArraySlice<int> x);
// Process({1, 2, 3});
// The data referenced by the initializer_list must outlive this
// ArraySlice. For example, "ArraySlice<int> s={1,2};" and "return
// ArraySlice<int>({3,4});" are errors, as the resulting ArraySlice may
// reference data that is no longer valid.
So, an additional memory is required to store the held instances of the 'DataType' class. This commit introduces 'std::vector' as the store for 'DataType's. For backward compatibility, a static variable of 'ArraySlice<DataType>' is introduced in each method and the methods return references to those variables.
Signed-off-by: Pavel Samolysov [email protected]
@avijit-nervana Could you have a look at the PR, please? Maybe I wasn't able to properly titled the PR, but it is very important because without this change any DataType
s in gtl::ArraySlice
s will destructed and no one DType will be considered as compatible with NGraph, so no NGraph-nodes will be detected in ANY graph. I see this at least if NGraph and NGgraph-TF are compiled with CLang 7/8.
TESTNOW
TESTNOW
I have no access to the CI server and unfortunately am not able to see what is wrong with the commits.