This is an archive of the discontinued LLVM Phabricator instance.

Use uniform mechanism for OOM errors handling
ClosedPublic

Authored by sepavloff on May 28 2018, 12:31 AM.

Details

Summary

In r325551 many calls of malloc/calloc/realloc were replaces with calls of
their safe counterparts defined in the namespace llvm. There functions
generate crash if memory cannot be allocated, such behavior facilitates
handling of out of memory errors on Windows.

If the result of *alloc function were checked for success, the function was
not replaced with the safe variant. In these cases the calling function made
the error handling, like:

T *NewElts = static_cast<T*>(malloc(NewCapacity*sizeof(T)));
if (NewElts == nullptr)
  report_bad_alloc_error("Allocation of SmallVector element failed.");

Actually knowledge about the function where OOM occurred is useless. Moreover
having a single entry point for OOM handling is convenient for investigation
of memory problems. This change removes custom OOM errors handling and
replaces them with calls to functions llvm::safe_*alloc.

Declarations of safe_*alloc are moved to a separate include file, to avoid
cyclic dependency in SmallVector.h

Diff Detail

Repository
rL LLVM

Event Timeline

sepavloff created this revision.May 28 2018, 12:31 AM
rnk accepted this revision.May 28 2018, 9:58 AM

lgtm

Factoring all this checking out seems like a win.

include/llvm/Support/MemAlloc.h
25 ↗(On Diff #148784)

I'm not totally happy with the name "safe" here. They're really no "safer", they just have a different failure mode that is hookable. I kind of like mallocOrDie or malloc_or_die, which is the naming used by the sanitizer libraries. Let's leave it alone for now, though.

This revision is now accepted and ready to land.May 28 2018, 9:58 AM
This revision was automatically updated to reflect the committed changes.

It would seem this is slightly incorrect.

$ ninja -j1
[1/4533] Linking CXX shared library lib/libLLVMDemangle.so.7svn
FAILED: lib/libLLVMDemangle.so.7svn 
: && /usr/bin/clang++-6.0 -fPIC -g0 -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -g0  -Wl,-z,defs -Wl,-z,nodelete -fuse-ld=lld -Wl,--color-diagnostics   -Wl,-O3 -Wl,--gc-sections -shared -Wl,-soname,libLLVMDemangle.so.7 -o lib/libLLVMDemangle.so.7svn lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o  -Wl,-rpath,"\$ORIGIN/../lib" && :
/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:(llvm::itaniumDemangle(char const*, char*, unsigned long*, int*))

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:(llvm::printNode((anonymous namespace)::Node*, char*, unsigned long*))

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:(llvm::ItaniumPartialDemangler::getFunctionDeclContextName(char*, unsigned long*) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:(llvm::ItaniumPartialDemangler::getFunctionDeclContextName(char*, unsigned long*) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:(llvm::ItaniumPartialDemangler::getFunctionDeclContextName(char*, unsigned long*) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:(llvm::ItaniumPartialDemangler::getFunctionParameters(char*, unsigned long*) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:(llvm::ItaniumPartialDemangler::getFunctionParameters(char*, unsigned long*) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:(llvm::ItaniumPartialDemangler::getFunctionParameters(char*, unsigned long*) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:(llvm::ItaniumPartialDemangler::getFunctionParameters(char*, unsigned long*) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:(llvm::ItaniumPartialDemangler::getFunctionReturnType(char*, unsigned long*) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:((anonymous namespace)::PODSmallVector<(anonymous namespace)::Node*, 32ul>::push_back((anonymous namespace)::Node* const&))

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:((anonymous namespace)::PODSmallVector<(anonymous namespace)::Node*, 32ul>::push_back((anonymous namespace)::Node* const&))

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:((anonymous namespace)::SpecialName::printLeft((anonymous namespace)::OutputStream&) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:((anonymous namespace)::CtorVtableSpecialName::printLeft((anonymous namespace)::OutputStream&) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:((anonymous namespace)::CtorVtableSpecialName::printLeft((anonymous namespace)::OutputStream&) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:((anonymous namespace)::Db::parseTemplateArgs(bool))

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:((anonymous namespace)::Db::parseTemplateArgs(bool))

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:((anonymous namespace)::Db::parseTemplateParam())

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:((anonymous namespace)::Db::parseTemplateParam())

/usr/lib/llvm-6.0/bin/ld.lld: error: undefined symbol: llvm::report_bad_alloc_error(char const*, bool)
>>> referenced by ItaniumDemangle.cpp
>>>               lib/Demangle/CMakeFiles/LLVMDemangle.dir/ItaniumDemangle.cpp.o:((anonymous namespace)::NameType::printLeft((anonymous namespace)::OutputStream&) const)

/usr/lib/llvm-6.0/bin/ld.lld: error: too many errors emitted, stopping now (use -error-limit=0 to see all errors)
clang: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

If i try to fix it:

diff --git a/lib/Demangle/CMakeLists.txt b/lib/Demangle/CMakeLists.txt
index 74e37654f07..e03b555e3a6 100644
--- a/lib/Demangle/CMakeLists.txt
+++ b/lib/Demangle/CMakeLists.txt
@@ -1,3 +1,6 @@
 add_llvm_library(LLVMDemangle
   ItaniumDemangle.cpp
+
+  LINK_LIBS
+  LLVMSupport
 )

It does not work:

Configuring done
CMake Error: The inter-target dependency graph contains the following strongly connected component (cycle):
  "LLVMDemangle" of type SHARED_LIBRARY
    depends on "LLVMSupport" (weak)
  "LLVMSupport" of type SHARED_LIBRARY
    depends on "LLVMDemangle" (weak)
At least one of these targets is not a STATIC_LIBRARY.  Cyclic dependencies are allowed only among static libraries.
CMake Project parsing failed.

So this somehow introduced a cycle into dependency graph?

@dblaikie,@rnk,@sepavloff Thoughts on temporarily reverting for now?

rnk added a comment.May 29 2018, 11:57 AM

I would revert the changes to the demangler library only, if possible, and
just leave that as a special case. I assume we do some weird stuff to call
report_fatal_error without introducing a circular dependency.