ClimateGlobalChange / tempestextremes

Extreme weather detection and characterization
84 stars 30 forks source link

StitchNodes crashes on exit with new-delete-type-mismatch due to lack of virtual destructor for ColumnData #71

Closed mjwillson closed 6 months ago

mjwillson commented 6 months ago

I see consistent crashes from StitchNodes just before exit with the following error:

1508903 third_party/tcmalloc/tcmalloc.cc:905] size check failed for 0x10d67fe4d1c0: claimed 8, actual 32, class 1
1508903 third_party/tcmalloc/tcmalloc.cc:850] CHECK in do_free_with_size: CorrectSize(ptr, size, align) (false) 

Compiling with ASAN enabled and re-running reveals the problem is in the PathNode destructor:

==1520578==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x50300001ed00 in thread T0:
  object passed to delete has wrong type:
  size of the allocated type:   32 bytes;
  size of the deallocated type: 8 bytes.
    #0 0x5615d3c24462 in operator delete(void*, unsigned long) third_party/llvm/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:155:3
    #1 0x5615d3c3d1b6 in PathNode::clear() third_party/tempestextremes/base/NodeFileUtilities.h:469:4
    #2 0x5615d3c3cfc4 in ~PathNode third_party/tempestextremes/base/NodeFileUtilities.h:478:3
    #3 0x5615d3c3cfc4 in __destroy_at<PathNode, 0> third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/construct_at.h:67:11
    #4 0x5615d3c3cfc4 in destroy<PathNode, void, 0> third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/allocator_traits.h:340:5
    #5 0x5615d3c3cfc4 in __base_destruct_at_end third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/vector:927:7
    #6 0x5615d3c3cfc4 in __clear third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/vector:921:5
    #7 0x5615d3c3cfc4 in std::__u::vector<PathNode, std::__u::allocator<PathNode>>::__destroy_vector::operator()() third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/vector:491:16
    #8 0x5615d3c3ef5f in ~vector third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/vector:502:67
    #9 0x5615d3c3ef5f in __destroy_at<Path, 0> third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/construct_at.h:67:11
    #10 0x5615d3c3ef5f in destroy<Path, void, 0> third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/allocator_traits.h:340:5
    #11 0x5615d3c3ef5f in __base_destruct_at_end third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/vector:927:7
    #12 0x5615d3c3ef5f in __clear third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/vector:921:5
    #13 0x5615d3c3ef5f in std::__u::vector<Path, std::__u::allocator<Path>>::__destroy_vector::operator()() third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/vector:491:16
    #14 0x5615d3c34eb2 in ~vector third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/vector:502:67
    #15 0x5615d3c34eb2 in NodeFile::~NodeFile() third_party/tempestextremes/base/NodeFileUtilities.h:670:7
    #16 0x5615d3c2e3e4 in main third_party/tempestextremes/nodes/StitchNodes.cpp:1363:2
    #17 0x7faa8d2fe3d3 in __libc_start_main (/usr/grte/v5/lib64/libc.so.6+0x613d3) (BuildId: 9a996398ce14a94560b0c642eb4f6e94)
    #18 0x5615d3b4fea9 in _start /usr/grte/v5/debug-src/src/csu/../sysdeps/x86_64/start.S:120

0x50300001ed00 is located 0 bytes inside of 32-byte region [0x50300001ed00,0x50300001ed20)
allocated by thread T0 here:
    #0 0x5615d3c237dd in operator new(unsigned long) third_party/llvm/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:86:3
    #1 0x5615d3c2c297 in main third_party/tempestextremes/nodes/StitchNodes.cpp:1348:7
    #2 0x7faa8d2fe3d3 in __libc_start_main (/usr/grte/v5/lib64/libc.so.6+0x613d3) (BuildId: 9a996398ce14a94560b0c642eb4f6e94)
    #3 0x5615d3b4fea9 in _start /usr/grte/v5/debug-src/src/csu/../sysdeps/x86_64/start.S:120

SUMMARY: AddressSanitizer: new-delete-type-mismatch third_party/tempestextremes/base/NodeFileUtilities.h:469:4 in PathNode::clear()

Specifically PathNode::clear() is delete-ing a ColumnData *, but ColumnData has multiple subclasses (ColumnDataString, ColumnDataDouble) and no virtual destructor, so it won't know how much memory to free when deleting a pointer to the superclass.

I've confirmed the following fixes it:

diff --git a/google3/third_party/tempestextremes/base/NodeFileUtilities.h b/tempestextremes/base/NodeFileUtilities.h
--- a/tempestextremes/base/NodeFileUtilities.h
+++ b/tempestextremes/base/NodeFileUtilities.h
@@ -177,6 +177,8 @@ public:
        virtual ColumnData * Duplicate() const {
                return new ColumnData();
        }
+
+       virtual ~ColumnData() {}
 };

 ///////////////////////////////////////////////////////////////////////////////
paullric commented 6 months ago

Fixed in commit 8d36066

mjwillson commented 6 months ago

That was quick, thanks!