dim created this revision. dim added reviewers: aprantl, emaste, JDevlieghere, teemperor. Herald added subscribers: krytarowski, arichardson. dim requested review of this revision. Herald added a project: LLDB.
When assertions are turned off, the `llvm::Error` value created at the start of this function is overwritten using the move-assignment operator, but the success value is never checked. Whenever a TypeSystem cannot be found or created, this can lead to lldb core dumping with: Program aborted due to an unhandled Error: Error value was Success. (Note: Success values must still be checked prior to being destroyed). Fix this by not creating a `llvm::Error` value in advance, and directly returning the result of `llvm::make_error` instead, whenever an error is encountered. See also: https://bugs.freebsd.org/253881 and https://bugs.freebsd.org/257829. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D108088 Files: lldb/source/Symbol/TypeSystem.cpp
Index: lldb/source/Symbol/TypeSystem.cpp =================================================================== --- lldb/source/Symbol/TypeSystem.cpp +++ lldb/source/Symbol/TypeSystem.cpp @@ -223,62 +223,32 @@ llvm::Expected<TypeSystem &> TypeSystemMap::GetTypeSystemForLanguage( lldb::LanguageType language, llvm::Optional<CreateCallback> create_callback) { - llvm::Error error = llvm::Error::success(); - assert(!error); // Check the success value when assertions are enabled std::lock_guard<std::mutex> guard(m_mutex); - if (m_clear_in_progress) { - error = llvm::make_error<llvm::StringError>( + if (m_clear_in_progress) + return llvm::make_error<llvm::StringError>( "Unable to get TypeSystem because TypeSystemMap is being cleared", llvm::inconvertibleErrorCode()); - } else { - collection::iterator pos = m_map.find(language); - if (pos != m_map.end()) { - auto *type_system = pos->second.get(); - if (type_system) { - llvm::consumeError(std::move(error)); - return *type_system; - } - error = llvm::make_error<llvm::StringError>( - "TypeSystem for language " + - llvm::StringRef(Language::GetNameForLanguageType(language)) + - " doesn't exist", - llvm::inconvertibleErrorCode()); - return std::move(error); - } - for (const auto &pair : m_map) { - if (pair.second && pair.second->SupportsLanguage(language)) { - // Add a new mapping for "language" to point to an already existing - // TypeSystem that supports this language - m_map[language] = pair.second; - if (pair.second.get()) { - llvm::consumeError(std::move(error)); - return *pair.second.get(); - } - error = llvm::make_error<llvm::StringError>( - "TypeSystem for language " + - llvm::StringRef(Language::GetNameForLanguageType(language)) + - " doesn't exist", - llvm::inconvertibleErrorCode()); - return std::move(error); - } - } + collection::iterator pos = m_map.find(language); + if (pos != m_map.end()) { + auto *type_system = pos->second.get(); + if (type_system) + return *type_system; + return llvm::make_error<llvm::StringError>( + "TypeSystem for language " + + llvm::StringRef(Language::GetNameForLanguageType(language)) + + " doesn't exist", + llvm::inconvertibleErrorCode()); + } - if (!create_callback) { - error = llvm::make_error<llvm::StringError>( - "Unable to find type system for language " + - llvm::StringRef(Language::GetNameForLanguageType(language)), - llvm::inconvertibleErrorCode()); - } else { - // Cache even if we get a shared pointer that contains a null type system - // back - TypeSystemSP type_system_sp = (*create_callback)(); - m_map[language] = type_system_sp; - if (type_system_sp.get()) { - llvm::consumeError(std::move(error)); - return *type_system_sp.get(); - } - error = llvm::make_error<llvm::StringError>( + for (const auto &pair : m_map) { + if (pair.second && pair.second->SupportsLanguage(language)) { + // Add a new mapping for "language" to point to an already existing + // TypeSystem that supports this language + m_map[language] = pair.second; + if (pair.second.get()) + return *pair.second.get(); + return llvm::make_error<llvm::StringError>( "TypeSystem for language " + llvm::StringRef(Language::GetNameForLanguageType(language)) + " doesn't exist", @@ -286,7 +256,23 @@ } } - return std::move(error); + if (!create_callback) + return llvm::make_error<llvm::StringError>( + "Unable to find type system for language " + + llvm::StringRef(Language::GetNameForLanguageType(language)), + llvm::inconvertibleErrorCode()); + + // Cache even if we get a shared pointer that contains a null type system + // back + TypeSystemSP type_system_sp = (*create_callback)(); + m_map[language] = type_system_sp; + if (type_system_sp.get()) + return *type_system_sp.get(); + return llvm::make_error<llvm::StringError>( + "TypeSystem for language " + + llvm::StringRef(Language::GetNameForLanguageType(language)) + + " doesn't exist", + llvm::inconvertibleErrorCode()); } llvm::Expected<TypeSystem &>
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits