[lldb] Add IsDebugInfoCompatible method to SBModule to allow checking compatibility between language versions

[teemperor]

Summary

In Swift there is a check if the compiled module actually fits to our Swift version in LLDB. We don't really do this
kind of version checking so far in LLDB as we only have one Clang-based TypeSystem for C languages.
C languages don't have this precise version semantic but different standards which are expressed as different language types,
so there is no precise version checking needed for them.

This adds the relevant compatibility check to SBModule and adds the relevant tests for it. The actual Swift version check is
implemented in Swift's TypeSystem class which is downstream, so we test this here with the Clang type system which is
always treating modules as compatible.

[labath]

The part that bothers me here is that this assumes that the entirety of a module (or an least the portion of it written in the same language) uses the same version of the language. Is that true for swift? Because it definitely isn't true for c++, as you can easily mix compile units built with different -std flags (and most of the time it will work). So it's not clear to me how this would work for c/c++, or any other language that offers some kind of a version-independent ABI, thus, enabling one to combine different versions in a single module. Nevertheless, since this is how swift works, it seems reasonable to have some way to access this information. However, it's not clear to me if this is the best way to do that...

Another thing on my mind is the name (type system). AFAICT, this is the first mention of these words in all of lldb/API headers, so one would get the impression that type systems are an internal implementation detail of lldb. That's why this methods sort of jumps out in your face. Are we planning to make more type system-y stuff visible at the SB level? If not, could we make this name more generic? Maybe, IsDebugInfoCompatible (with the meaning that lldb is able to understand all of debug info (of a given language) in this module)?

lldb/source/API/SBModule.cpp

700–704

Do you want to store the actual error message in type_system_or_err into sb_error?

[jingham]

In D69704#1732484, @labath wrote:

The part that bothers me here is that this assumes that the entirety of a module (or an least the portion of it written in the same language) uses the same version of the language. Is that true for swift? Because it definitely isn't true for c++, as you can easily mix compile units built with different -std flags (and most of the time it will work). So it's not clear to me how this would work for c/c++, or any other language that offers some kind of a version-independent ABI, thus, enabling one to combine different versions in a single module. Nevertheless, since this is how swift works, it seems reasonable to have some way to access this information. However, it's not clear to me if this is the best way to do that...

This call is actually being used to tell whether the type system in a given module is compatible with LLDB, not with some other compile units in the same module or other modules in the program. That is an issue for Swift because the type information for Swift is not stored in DWARF, but in the .swiftmodule file, which is a fancy dump of internal compiler state. So for lldb to be able to debug a swift module, it has to have been built with exactly the same version of the swift compiler as was built into lldb, or things go very wrong. That also answers your question about swift: you have to build all the source inputs to a Swift module with the same version of the Swift compiler. There is a version neutral text interface file, but that is only able to describe the public interface to a module. It allows you to build code against extant modules from different versions of the Swift compiler. But the information about private methods are only in the binary format, and so you still need the same version of swift to glom them all together.

If, for instance, we decided to use .pcm files as a serialization form for type info you would need the same sort of check for modules backed by these pcm's - since the clang in lldb and the clang building the pcm would have to match. BTW I am NOT suggesting that would be a good idea, and this will probably always be a no-op for anything but Swift.

Another thing on my mind is the name (type system). AFAICT, this is the first mention of these words in all of lldb/API headers, so one would get the impression that type systems are an internal implementation detail of lldb. That's why this methods sort of jumps out in your face. Are we planning to make more type system-y stuff visible at the SB level? If not, could we make this name more generic? Maybe, IsDebugInfoCompatible (with the meaning that lldb is able to understand all of debug info (of a given language) in this module)?

IsDebugInfoCompatible is a more explicit name. It would be cool at some point to write a TypeSystem as a pure plugin, but we're nowhere near that. So I agree it's better to reserve TypeSystem for lldb_private if we aren't really exposing it.

Note, Xcode already uses this interface from the Swift branch, so we'll have to be careful about changing the name.

[teemperor]

• Renamed to IsDebugInfoCompatible
• Pass on llvm::Error error message to caller.

[labath]

I think this is fine.

In D69704#1741588, @jingham wrote:

If, for instance, we decided to use .pcm files as a serialization form for type info you would need the same sort of check for modules backed by these pcm's - since the clang in lldb and the clang building the pcm would have to match. BTW I am NOT suggesting that would be a good idea, and this will probably always be a no-op for anything but Swift.

Hm... If I understand modules correctly (and I admit I know very little about them), I think this actually illustrates the my reservations about this. Assuming we were fetching type info from a pcm file etc., there is no relationship between a module (shared library) and a pcm file... I would imagine a module can reference multiple pcm files, and each one could be theoretically built with a different compiler. That then brings up a question of what does it mean for lldb to be "compatible" with the module as a whole...

lldb/source/API/SBModule.cpp

700–704

Actually, you can do that by just saying sb_error.ref() = type_system_or_err.takeError() or something along those lines. :)

[shafik]

lldb/packages/Python/lldbsuite/test/python_api/module_compability/TestModuleDebugInfoCompatible.py

61 ↗

(On Diff #228873)

I note that we don't have one for C++17 but I think that is because we just don't have the enumerator although I think we could.

[jingham]

In D69704#1742494, @labath wrote:

I think this is fine.

In D69704#1741588, @jingham wrote:

If, for instance, we decided to use .pcm files as a serialization form for type info you would need the same sort of check for modules backed by these pcm's - since the clang in lldb and the clang building the pcm would have to match. BTW I am NOT suggesting that would be a good idea, and this will probably always be a no-op for anything but Swift.

Hm... If I understand modules correctly (and I admit I know very little about them), I think this actually illustrates the my reservations about this. Assuming we were fetching type info from a pcm file etc., there is no relationship between a module (shared library) and a pcm file... I would imagine a module can reference multiple pcm files, and each one could be theoretically built with a different compiler. That then brings up a question of what does it mean for lldb to be "compatible" with the module as a whole...

It doesn't seem like these are mutually exclusive. If we were being exhaustive about this we would have two levels of test, (1) is all the debug information in the module uningestible and (2) are there some CU's in the module that have debug information that lldb can't ingest. If we had (2) then a language could decide "am I able to construct types from the debug info for a module that has some broken parts?" Deciding that depends on how the debug info is laid out. If the answer is yes, then (1) would return "false if all CU's return false from (2)." if no, (1) would return "false if any CU's return false from (2)".

Also, IRL if we introduce some incompatibility in the toolchain from compiler version -> lldb version, the vastly more common situation would be for all of a module to be built in an incompatible way. So having this larger scale check still seems to me useful. That lldb or some GUI using it can put up a dialog saying "This module is not compatible with lldb". If the whole module is not compatible it would be awkward to complain about it CU by CU.

Then historically, in the case of swift the part that causes problems is a whole-module resource and if that is not compatible with lldb, lldb can't construct a valid TypeSystem for that module. So we didn't need to implement (2), we just did (1). But that shouldn't stop anybody from implementing (2) should the need arise.

[teemperor]

Marking this as changes needed that I don't land this without applying Pavel's comment.

lldb/source/API/SBModule.cpp

700–704

Oh, TIL!

[teemperor]

• Simplified error handling (Thanks Pavel)

[labath]

In D69704#1742638, @jingham wrote:

It doesn't seem like these are mutually exclusive. If we were being exhaustive about this we would have two levels of test, (1) is all the debug information in the module uningestible and (2) are there some CU's in the module that have debug information that lldb can't ingest. If we had (2) then a language could decide "am I able to construct types from the debug info for a module that has some broken parts?" Deciding that depends on how the debug info is laid out. If the answer is yes, then (1) would return "false if all CU's return false from (2)." if no, (1) would return "false if any CU's return false from (2)".

Right, it's not that the two are mutually exclusive -- it's just that the semantics becomes fuzzier. You seem to think that the "natural" behavior would be to flag if "all" debug info is incompatible. My first though was that we should report if "any" debug info is incompatible.

Also, IRL if we introduce some incompatibility in the toolchain from compiler version -> lldb version, the vastly more common situation would be for all of a module to be built in an incompatible way. So having this larger scale check still seems to me useful. That lldb or some GUI using it can put up a dialog saying "This module is not compatible with lldb". If the whole module is not compatible it would be awkward to complain about it CU by CU.

I actually don't think this is that uncommon in the c++ world. While it's true that all *user* code in a given library will likely use the same compiler, a typical module will also contain some startup/glue code taken from the libc and/or compiler support libraries. It's fairly likely that these were built with a different compiler, or at least different flags of the same compiler. And while these things will often have no debug info, there are also a lot of cases when they will (android NDK ships debug info for everything, most linux distros enable one to install it optionally, etc.)

BTW, what is the expected effect of this method returning "false"? I would expect that one can still normally use this module everywhere, only it will appear as if it contains no debug info (or just a limited subset).. Is that so?

[teemperor]

Turns out this is actually not even used by anyone but was just (somehow?) added to the SBAPI in lldb-swift... Anyway, deleting this downstream...

lldb/include/lldb/API/SBError.h

protected:
   friend class SBBreakpoint;
   friend class SBBreakpointLocation;
   friend class SBBreakpointName;
   friend class SBCommandReturnObject;
   friend class SBCommunication;
   friend class SBData;
   friend class SBDebugger;
   friend class SBHostOS;
+  friend class SBModule;
   friend class SBPlatform;
   friend class SBProcess;
   friend class SBReproducer;
   friend class SBStructuredData;
   friend class SBTarget;
   friend class SBThread;
   friend class SBTrace;
   friend class SBValue;

lldb/include/lldb/API/SBModule.h

public:
   ///
   /// \return
   ///     A const reference to the file specification object.
   lldb::SBFileSpec GetSymbolFileSpec() const;
 
   lldb::SBAddress GetObjectFileHeaderAddress() const;
   lldb::SBAddress GetObjectFileEntryPointAddress() const;
 
+  /// Checks if the module is compatible with the type system that LLDB
+  /// implements for the given language.
+  ///
+  /// A module may be incompatible with LLDB's type system if the language
+  /// version that is used in the module isn't supported by LLDB even though
+  /// the language itself is supported. This can happen if the language itself
+  /// evolved but LLDB's only supports a newer/older version of the language.
+  ///
+  /// \param[in] language
+  ///     The language that should be checked for compability with this module.
+  ///
+  /// \return
+  ///     An error object that describes the errors that were encounterd when
+  ///     checking if LLDB's type system is compatible.
+  lldb::SBError IsTypeSystemCompatible(lldb::LanguageType language);
+
 private:
   friend class SBAddress;
   friend class SBFrame;
   friend class SBSection;
   friend class SBSymbolContext;
   friend class SBTarget;
 
   explicit SBModule(const lldb::ModuleSP &module_sp);

lldb/include/lldb/Symbol/TypeSystem.h

public:
 
   virtual bool IsVoidType(lldb::opaque_compiler_type_t type) = 0;
 
   virtual bool CanPassInRegisters(const CompilerType &type) = 0;
 
   // TypeSystems can support more than one language
   virtual bool SupportsLanguage(lldb::LanguageType language) = 0;
 
+  /// Check if the current module or target that was used to create this
+  /// type system is compatible with the TypeSystem plug-in.
+  ///
+  /// \return A Status object that is either describing any potential errors
+  ///         that make the module/target incompatible or contains no errors
+  ///         when the module/target is compatible.
+  virtual Status IsCompatible();
+
   // Type Completion
 
   virtual bool GetCompleteType(lldb::opaque_compiler_type_t type) = 0;
 
   // AST related queries
 
   virtual uint32_t GetPointerByteSize() = 0;
 

lldb/packages/Python/lldbsuite/test/python_api/module_compability/Makefile

+CXX_SOURCES := main.cpp
+
+include Makefile.rules

lldb/packages/Python/lldbsuite/test/python_api/module_compability/TestModuleTypeSystemCompatible.py

+"""
+Test SBModule's IsTypeSystemCompatible.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ModuleTypeSystemCheckTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def assert_invalid_module_err(self, error):
+        self.assertEquals("invalid module", error.GetCString())
+        self.assertFalse(error.Success())
+
+    # Py3 asserts due to a bug in SWIG.  A fix for this was upstreamed into
+    # SWIG 3.0.8.
+    @skipIf(py_version=['>=', (3, 0)], swig_version=['<', (3, 0, 8)])
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_default_module(self):
+        exe_module = lldb.SBModule()
+        self.assert_invalid_module_err(exe_module.IsTypeSystemCompatible(lldb.eLanguageTypeUnknown))
+        self.assert_invalid_module_err(error = exe_module.IsTypeSystemCompatible(lldb.eLanguageTypeC))
+
+    def assert_compatible(self, exe_module, lang):
+        error = exe_module.IsTypeSystemCompatible(lang)
+        self.assertTrue(error.Success())
+
+    # Py3 asserts due to a bug in SWIG.  A fix for this was upstreamed into
+    # SWIG 3.0.8.
+    @skipIf(py_version=['>=', (3, 0)], swig_version=['<', (3, 0, 8)])
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_c_languages(self):
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+        self.assertTrue(target.GetNumModules() > 0)
+
+        exe_module = target.GetModuleAtIndex(0)
+
+        # Check that we get an error if LLDB doesn't implement the language
+        error = exe_module.IsTypeSystemCompatible(lldb.eLanguageTypeModula2)
+        self.assertEquals("no type system for language modula2", error.GetCString())
+        self.assertFalse(error.Success())
+
+        # Check that C languages are compatible with the module.
+        self.assert_compatible(exe_module, lldb.eLanguageTypeC)
+        self.assert_compatible(exe_module, lldb.eLanguageTypeC89)
+        self.assert_compatible(exe_module, lldb.eLanguageTypeC99)
+        self.assert_compatible(exe_module, lldb.eLanguageTypeC11)
+        self.assert_compatible(exe_module, lldb.eLanguageTypeC_plus_plus)
+        self.assert_compatible(exe_module, lldb.eLanguageTypeC_plus_plus_03)
+        self.assert_compatible(exe_module, lldb.eLanguageTypeC_plus_plus_11)
+        self.assert_compatible(exe_module, lldb.eLanguageTypeC_plus_plus_14)
+
+        # 'Unknown' language gets mapped to a generic C language right now.
+        self.assert_compatible(exe_module, lldb.eLanguageTypeUnknown)

lldb/packages/Python/lldbsuite/test/python_api/module_compability/main.cpp

+int main() {}

lldb/scripts/interface/SBModule.i

public:
     GetSymbolFileSpec() const;
 
     lldb::SBAddress
     GetObjectFileHeaderAddress() const;
 
     lldb::SBAddress
     GetObjectFileEntryPointAddress() const;
 
+    lldb::SBError
+    IsTypeSystemCompatible(lldb::LanguageType language);
+
     %pythoncode %{
         def __len__(self):
             '''Return the number of symbols in a lldb.SBModule object.'''
             return self.GetNumSymbols()
 
         def __iter__(self):
             '''Iterate over all symbols in a lldb.SBModule object.'''
             return lldb_iter(self, 'GetNumSymbols', 'GetSymbolAtIndex')

lldb/source/API/SBModule.cpp

 #include "lldb/Core/Section.h"
 #include "lldb/Core/ValueObjectList.h"
 #include "lldb/Core/ValueObjectVariable.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/Symtab.h"
 #include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 SBModule::SBModule() : m_opaque_sp() {
   LLDB_RECORD_CONSTRUCTOR_NO_ARGS(SBModule);
lldb::SBAddress SBModule::GetObjectFileEntryPointAddress() const {
   if (module_sp) {
     ObjectFile *objfile_ptr = module_sp->GetObjectFile();
     if (objfile_ptr)
       sb_addr.ref() = objfile_ptr->GetEntryPointAddress();
   }
   return LLDB_RECORD_RESULT(sb_addr);
 }
 
+lldb::SBError SBModule::IsTypeSystemCompatible(lldb::LanguageType language) {
+  SBError sb_error;
+  ModuleSP module_sp(GetSP());
+  if (!module_sp) {
+    sb_error.SetErrorString("invalid module");
+    return sb_error;
+  }
+
+  auto type_system_or_err = module_sp->GetTypeSystemForLanguage(language);
+  if (!type_system_or_err) {
+    sb_error.SetErrorStringWithFormat(
+        "no type system for language %s",
+        Language::GetNameForLanguageType(language));
+    llvm::consumeError(type_system_or_err.takeError());
+    return sb_error;
+  }
+
+  sb_error.SetError(type_system_or_err->IsCompatible());
+  return sb_error;
+}
+
 namespace lldb_private {
 namespace repro {
 
 template <>
 void RegisterMethods<SBModule>(Registry &R) {
   LLDB_REGISTER_CONSTRUCTOR(SBModule, ());
   LLDB_REGISTER_CONSTRUCTOR(SBModule, (const lldb::SBModuleSpec &));
   LLDB_REGISTER_CONSTRUCTOR(SBModule, (const lldb::SBModule &));

lldb/source/Symbol/TypeSystem.cpp

 LazyBool TypeSystem::ShouldPrintAsOneLiner(void *type, ValueObject *valobj) {
   return eLazyBoolCalculate;
 }
 
 bool TypeSystem::IsMeaninglessWithoutDynamicResolution(void *type) {
   return false;
 }
 
+Status TypeSystem::IsCompatible() {
+  // Assume a language is compatible. Override this virtual function
+  // in your TypeSystem plug-in if version checking is desired.
+  return Status();
+}
+
 ConstString TypeSystem::DeclGetMangledName(void *opaque_decl) {
   return ConstString();
 }
 
 CompilerDeclContext TypeSystem::DeclGetDeclContext(void *opaque_decl) {
   return CompilerDeclContext();
 }