(UndefinedBehaviorSanitizer) GrowableArray.h: null pointer passed as argument 2, which is declared to never be null

[blockspacer]

#~/.conan/data/corrade/v2020.06/conan/stable/build/#a588aedb50aa745a2698801b1f638aec5f70dbcd/src/Corrade/Containers/#GrowableArray.h:943:22: runtime error: null pointer passed as argument 2, which is #declared to never be null
#/usr/include/string.h:43:28: note: nonnull attribute specified here
#SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ~/.conan/data/#corrade/v2020.06/conan/stable/build/a588aedb50aa745a2698801b1f638aec5f70dbcd/src/#Corrade/Containers/GrowableArray.h:943:22 in
#~/.conan/data/corrade/v2020.06/conan/stable/build/#a588aedb50aa745a2698801b1f638aec5f70dbcd/src/Corrade/Containers/#GrowableArray.h:943:22: runtime error: null pointer passed as argument 2, which is #declared to never be null
#/usr/include/string.h:43:28: note: nonnull attribute specified here
#SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ~/.conan/data/#corrade/v2020.06/conan/stable/build/a588aedb50aa745a2698801b1f638aec5f70dbcd/src/#Corrade/Containers/GrowableArray.h:943:22 in
#~/.conan/data/corrade/v2020.06/conan/stable/build/#a588aedb50aa745a2698801b1f638aec5f70dbcd/src/Corrade/Containers/#GrowableArray.h:943:22: runtime error: null pointer passed as argument 2, which is #declared to never be null
#/usr/include/string.h:43:28: note: nonnull attribute specified here
#SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ~/.conan/data/#corrade/v2020.06/conan/stable/build/a588aedb50aa745a2698801b1f638aec5f70dbcd/src/#Corrade/Containers/GrowableArray.h:943:22 in
#~/.conan/data/corrade/v2020.06/conan/stable/build/#a588aedb50aa745a2698801b1f638aec5f70dbcd/src/Corrade/Containers/#GrowableArray.h:943:22: runtime error: null pointer passed as argument 2, which is #declared to never be null
#/usr/include/string.h:43:28: note: nonnull attribute specified here
#SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ~/.conan/data/#corrade/v2020.06/conan/stable/build/a588aedb50aa745a2698801b1f638aec5f70dbcd/src/#Corrade/Containers/GrowableArray.h:943:22 in
#~/.conan/data/corrade/v2020.06/conan/stable/build/#a588aedb50aa745a2698801b1f638aec5f70dbcd/src/Corrade/Containers/#GrowableArray.h:943:22: runtime error: null pointer passed as argument 2, which is #declared to never be null
#/usr/include/string.h:43:28: note: nonnull attribute specified here
#SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ~/.conan/data/#corrade/v2020.06/conan/stable/build/a588aedb50aa745a2698801b1f638aec5f70dbcd/src/#Corrade/Containers/GrowableArray.h:943:22 in

To get error build with clang and -fsanitize=address,undefined

    -DMEMORY_TOOL_REPLACES_ALLOCATOR=1
    -D_FORTIFY_SOURCE=0
    -DUNDEFINED_SANITIZER=1
    -DUNDEFINED_BEHAVIOR_SANITIZER=1
    -g -O0
    -fPIC
    -fno-optimize-sibling-calls
    -fno-omit-frame-pointer
    -fno-stack-protector
    -fno-wrapv
    -fno-sanitize-recover=all
    -fsanitize-recover=unsigned-integer-overflow
    -fsanitize=address,undefined
    -fsanitize-trap=undefined
    -fsanitize=float-divide-by-zero
    -fno-sanitize=vptr
    -fsanitize=nullability-arg
    -fsanitize=nullability-assign
    -fsanitize=nullability-return

memcpy uses __nonnull:

extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
		     size_t __n) __THROW __nonnull ((1, 2));

NOTE: -fsanitize may require instrumented libc++, see conanfile.py in https://github.com/blockspacer/llvm_tools as example how to build instrumented libc++

Possible fix:

    if(count*sizeof(T) != NULL) // take care of nonnull attribute
    {
        std::memcpy(dst, src, count*sizeof(T));
    }

[mosra]

Hi, thank you for the detailed report!

The library is currently definitely not UBSan-clean, last time I tried (#29) I had quite a lot of false positives that obscured the real bugs. I can confirm this warning pops up (among others) when building locally and running the ContainersGrowableArrayTest.

In practice I didn't see any issues with this code on any of the platforms I'm testing on so it doesn't seem to be that critical, nonetheless that doesn't mean it's correct ;) I'll get back to fixing this (and other UBSan errors) once I find some free time.

[mosra]

This particular case and a few other I found should be fixed with 9ee4415. For me, UBSan complained about similar cases with std::memcmp() and std::memchr(), and honestly I find all this just unnecessary bullying without any real value added. Requiring all code ever to check sizes before calling those functions only increases testing burden and surface for potential bugs for no reason. I doubt there's any real C library implementation that would exploit this UB to bring some extra performance to the table.

In conclusion, the library is not UBSan-clean and probably never will be. I'll track the progress in #29 but I have no intention in going though all calls and adding stupid if(size) in front of each.