Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix segfault when attempting to convert string to float16.
To make sure this gets fixed, add test for converting string to any numeric type.

PiperOrigin-RevId: 286650886
Change-Id: I81f770ec2bbd33a863e8057ce198c679912fa8e0
  • Loading branch information
mihaimaruseac authored and tensorflower-gardener committed Dec 20, 2019
1 parent b787df8 commit 5ac1b9e
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 12 deletions.
11 changes: 11 additions & 0 deletions tensorflow/python/BUILD
Expand Up @@ -1839,6 +1839,17 @@ py_library(
],
)

tf_py_test(
name = "framework_constant_op_test",
size = "small",
srcs = ["framework/constant_op_test.py"],
main = "framework/constant_op_test.py",
python_version = "PY3",
deps = [
":constant_op",
],
)

tf_py_test(
name = "framework_registry_test",
size = "small",
Expand Down
61 changes: 61 additions & 0 deletions tensorflow/python/framework/constant_op_test.py
@@ -0,0 +1,61 @@
# Copyright 2020 The TensorFlow Authors. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
# ==============================================================================
"""Tests for tensorflow.python.framework.constant_op."""

from __future__ import absolute_import
from __future__ import division
from __future__ import print_function

from absl.testing import parameterized

from tensorflow.python.framework import constant_op
from tensorflow.python.framework import dtypes
from tensorflow.python.framework import ops
from tensorflow.python.platform import test


class ConstantOpTest(test.TestCase, parameterized.TestCase):

@parameterized.parameters(
dtypes.bfloat16,
dtypes.complex128,
dtypes.complex64,
dtypes.double,
dtypes.float16,
dtypes.float32,
dtypes.float64,
dtypes.half,
dtypes.int16,
dtypes.int32,
dtypes.int64,
dtypes.int8,
dtypes.qint16,
dtypes.qint32,
dtypes.qint8,
dtypes.quint16,
dtypes.quint8,
dtypes.uint16,
dtypes.uint32,
dtypes.uint64,
dtypes.uint8,
)
def test_convert_string_to_number(self, dtype):
with self.assertRaises(TypeError):
constant_op.constant("hello", dtype)


if __name__ == "__main__":
ops.enable_eager_execution()
test.main()
35 changes: 23 additions & 12 deletions tensorflow/python/lib/core/py_seq_tensor.cc
Expand Up @@ -21,6 +21,7 @@ limitations under the License.
#include "tensorflow/core/lib/core/errors.h"
#include "tensorflow/core/lib/core/stringpiece.h"
#include "tensorflow/core/lib/strings/str_util.h"
#include "tensorflow/core/platform/macros.h"
#include "tensorflow/core/platform/types.h"
#include "tensorflow/python/lib/core/numpy.h"
#include "tensorflow/python/lib/core/py_util.h"
Expand Down Expand Up @@ -396,6 +397,21 @@ typedef Converter<int32> Int32Converter;

// Floating-point support

// Returns `true` if `out` overflows when converted from `as_double`.
template <class T>
static inline bool CheckForOverflow(double as_double, T* out) {
return (sizeof(T) < sizeof(double) && std::isinf(*out) &&
std::isfinite(as_double));
}

// There is no `std::isinf` that takes `Eigen::half` as argument but Eigen
// provides `Eigen::half_impl::isinf` instead.
template <>
inline bool CheckForOverflow<Eigen::half>(double as_double, Eigen::half* out) {
return (sizeof(Eigen::half) < sizeof(double) &&
Eigen::half_impl::isinf(*out) && std::isfinite(as_double));
}

template <class T>
static const char* ConvertOneFloat(PyObject* v, T* out) {
if (PyErr_Occurred()) {
Expand All @@ -405,20 +421,19 @@ static const char* ConvertOneFloat(PyObject* v, T* out) {
const double as_double = PyFloat_AS_DOUBLE(v);
*out = static_cast<T>(as_double);
// Check for overflow
if (TF_PREDICT_FALSE(sizeof(T) < sizeof(double) && std::isinf(*out) &&
std::isfinite(as_double))) {
if (TF_PREDICT_FALSE(CheckForOverflow<T>(as_double, out))) {
return ErrorOutOfRangeDouble;
}
return nullptr;
}
#if PY_MAJOR_VERSION < 3
if (PyInt_Check(v)) {
*out = PyInt_AS_LONG(v);
*out = static_cast<T>(PyInt_AS_LONG(v));
return nullptr;
}
#endif
if (PyLong_Check(v)) {
*out = PyLong_AsDouble(v);
*out = static_cast<T>(PyLong_AsDouble(v));
if (PyErr_Occurred()) return ErrorOutOfRangeDouble;
return nullptr;
}
Expand Down Expand Up @@ -467,13 +482,7 @@ struct ConverterTraits<Eigen::half> {
static const tensorflow::DataType kTypeEnum = DT_HALF;

static const char* ConvertScalar(PyObject* v, Eigen::half* out) {
// NOTE(nareshmodi): Is there a way to convert to C double without the
// intermediate Python double? This will help with ConvertOneFloat as well.
Safe_PyObjectPtr as_float = make_safe(PyNumber_Float(v));
double v_double = PyFloat_AS_DOUBLE(as_float.get());
*out = Eigen::half(v_double);

return nullptr;
return ConvertOneFloat<Eigen::half>(v, out);
}
};

Expand Down Expand Up @@ -613,7 +622,9 @@ Status PySeqToTensor(PyObject* obj, DataType dtype, Tensor* ret) {
break;

case DT_HALF:
RETURN_STRING_AS_STATUS(NumpyHalfConverter::Convert(obj, &state, ret));
if (NumpyHalfConverter::Convert(obj, &state, ret) == nullptr)
return Status::OK();
break;

case DT_INT64:
if (Int64Converter::Convert(obj, &state, ret) == nullptr)
Expand Down

0 comments on commit 5ac1b9e

Please sign in to comment.