[lldb] Avoid unhandled Error in TypeSystemMap::GetTypeSystemForLanguage

[dim]

Summary

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.

[teemperor]

Thanks for fixing this (and cleaning up the code), LGTM. I wrote some regression tests for this in D108121 that I'll land as a follow-up.

lldb/source/Symbol/TypeSystem.cpp

 //===-- TypeSystem.cpp ----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Symbol/CompilerType.h"
 #include "lldb/Target/Language.h"
 
 #include <set>
 
 using namespace lldb_private;
if (type_system && !visited.count(type_system)) {
         break;
     }
   }
 }
 
 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);
-    }
+
+    if (type_system)
+    return 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()) {
-          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);
-      }
-    }
+      if (pair.second.get())
+      return llvm::make_error<llvm::StringError>(
 
-    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>(
-          "TypeSystem for language " +
-              llvm::StringRef(Language::GetNameForLanguageType(language)) +
-              " doesn't exist",
-          llvm::inconvertibleErrorCode());
-    }
-  }
-
-  return std::move(error);
-}
+  if (!create_callback)
+    return llvm::make_error<llvm::StringError>(
+
+  if (type_system_sp.get())
+  return llvm::make_error<llvm::StringError>(
 
 llvm::Expected<TypeSystem &>
 TypeSystemMap::GetTypeSystemForLanguage(lldb::LanguageType language,
                                         Module *module, bool can_create) {
   if (can_create) {
     return GetTypeSystemForLanguage(
         language, llvm::Optional<CreateCallback>([language, module]() {
           return TypeSystem::CreateInstance(language, module);