Skip to content

Commit

Permalink
[DEV-14011] Fix memory issue detected by ASan
Browse files Browse the repository at this point in the history
The ASan report is as follows:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==74==ERROR: AddressSanitizer: SEGV on unknown address 0x6030bf3b722e (pc 0x7fc277c6ae10 bp 0x7ffe22b2c9a0 sp 0x7ffe22b2c158 T0)
==74==The signal is caused by a READ memory access.
    #0 0x7fc277c6ae10  /build/glibc-eX1tMB/glibc-2.31/string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:365
    #1 0x49d947 in __asan_memcpy (/home/docker/kiravision/test/clang-memtest+0x49d947)
    serizba#2 0x50e00a in TF_TString_ResizeUninitialized(TF_TString*, unsigned long) /opt/tensorflow/include/tensorflow/core/platform/ctstring_internal.h:278:7
    serizba#3 0x50ba34 in TF_TString_Copy(TF_TString*, char const*, unsigned long) /opt/tensorflow/include/tensorflow/core/platform/ctstring_internal.h:389:17
    serizba#4 0x50bbac in Model::restore(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/docker/kiravision/cppflow/src/Model.cpp:107:5
    serizba#5 0x4e98e2 in KiraVision::LearnedModel::LearnedModel(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/docker/kiravision/src/LearnedModel.cpp:12:13
    serizba#6 0x4e28ed in KiraVision::TableDetector::TableDetector(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, KiraVision::LogLevel const&) /home/docker/kiravision/src/TableDetector.cpp:13:24
    serizba#7 0x4d058a in main /home/docker/kiravision/test/memtest.cpp:16:31
    serizba#8 0x7fc277bd30b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    serizba#9 0x425ccd in _start (/home/docker/kiravision/test/clang-memtest+0x425ccd)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /build/glibc-eX1tMB/glibc-2.31/string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:365
==74==ABORTING

The problem was incorrect initalization of a `TF_TString`. The existing
code initialized it as `new TF_Tstring` (which is not really any kind of
initalization, just memory allocation). `TF_TString`'s have to be
initialized with `TF_TString_Initialize` before they an be used. The
fact that the offending code is using a `unique_ptr` to ensure deletion
of the `TF_TString` makes it all a little clunky.
  • Loading branch information
abertoldi committed Jun 18, 2021
1 parent 243ff2f commit 3df6559
Showing 1 changed file with 10 additions and 6 deletions.
16 changes: 10 additions & 6 deletions src/Model.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,12 @@ void Model::init() {

void Model::save(const std::string &ckpt) {
#ifdef TENSORFLOW_C_TF_TSTRING_H_
std::unique_ptr<TF_TString, decltype(&TF_TString_Dealloc)> tstr(new TF_TString, &TF_TString_Dealloc);
TF_TString_Copy(tstr.get(), ckpt.c_str(), ckpt.size());
TF_TString tstr;
TF_TString_Init(&tstr);
TF_TString_Copy(&tstr, ckpt.c_str(), ckpt.size());
std::unique_ptr<TF_TString, decltype(&TF_TString_Dealloc)> up_tstr(&tstr, &TF_TString_Dealloc);
auto deallocator = [](void* data, size_t len, void* arg) {};
TF_Tensor* t = TF_NewTensor(TF_STRING, nullptr, 0, tstr.get(), 1, deallocator, nullptr);
TF_Tensor* t = TF_NewTensor(TF_STRING, nullptr, 0, &tstr, 1, deallocator, nullptr);
#else
// Encode file_name to tensor
size_t size = 8 + TF_StringEncodedSize(ckpt.length());
Expand Down Expand Up @@ -103,10 +105,12 @@ void Model::save(const std::string &ckpt) {

void Model::restore(const std::string& ckpt) {
#ifdef TENSORFLOW_C_TF_TSTRING_H_
std::unique_ptr<TF_TString, decltype(&TF_TString_Dealloc)> tstr(new TF_TString, &TF_TString_Dealloc);
TF_TString_Copy(tstr.get(), ckpt.c_str(), ckpt.size());
TF_TString tstr;
TF_TString_Init(&tstr);
TF_TString_Copy(&tstr, ckpt.c_str(), ckpt.size());
std::unique_ptr<TF_TString, decltype(&TF_TString_Dealloc)> up_tstr(&tstr, &TF_TString_Dealloc);
auto deallocator = [](void* data, size_t len, void* arg) {};
TF_Tensor* t = TF_NewTensor(TF_STRING, nullptr, 0, tstr.get(), 1, deallocator, nullptr);
TF_Tensor* t = TF_NewTensor(TF_STRING, nullptr, 0, &tstr, 1, deallocator, nullptr);
#else
// Encode file_name to tensor
size_t size = 8 + TF_StringEncodedSize(ckpt.size());
Expand Down

0 comments on commit 3df6559

Please sign in to comment.