[lldb] Adjust UpdateExternalModuleListIfNeeded method for the case of *.dwo

[alexander-shaposhnikov]

Summary

A *.dwo file itself can have DW_AT_GNU_dwo_name (but no DW_AT_comp_dir)
(clang 4.0 generates such DWOs). In this case there is no need to try to get a new module,
and, more over, if we try (below) the method ModuleList::GetSharedModule will return
an error + nullptr since the path is ill-formed (because comp_dir is null (since DW_AT_comp_dir is not present)).

Test plan: built a toy example + "br set --name f" + ... + "p LocalVariable" (lldb will search recursively for types and will call UpdateExternalModuleListIfNeeded)

[clayborg]

Can you explain the issue with an example?

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

1644–1656 ↗

(On Diff #113249)

So this is checking if the extension is ".dwo" and if the object file's path is the same as the .dwo file's path? Can you give an example of what fails? I'm having a hard time understanding what this is doing. Skipping a dwo file because it doesn't have a DW_AT_comp_dir? If the DW_AT_GNU_dwo_name is relative, then there really should be a DW_AT_comp_dir. Otherwise unless we run from the directory things are relative to, we don't find the .dwo file.

[alexander-shaposhnikov]

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

1644–1656 ↗

(On Diff #113249)

> So this is checking if the extension is ".dwo" and if the object file's path is the same as the .dwo
file's path?
yes!

okay, i will try to explain, sorry for the long text in advance.
LLDB tries to load "external modules" (recursively) (this is called, for example, when lldb is searching for types). The way this code determines if it needs to try to create an "external" module (and add it to m_external_type_modules) is via looking at the presence of DW_AT_GNU_dwo_name (lines 1627 - 1630).
However, when the already created module (corresponding to .dwo itself) is being processed,
it will see the presence of DW_AT_GNU_dwo_name (which contains the name of itself) and will try to call ModuleList::GetSharedModule again. Since .dwo has only DW_AT_GNU_dwo_name and doesn't have DW_AT_comp_dir the method ModuleList::GetSharedModule will fail (it will check if the file actually exists - and since the path is not correct, that check will fail) (and actually it doesn't make any sense to try to load the same dwo file again). As a result for each such case the warning (lines 1661 - 1665) will be printed.

minimal reduced example where i can reproduce the issue is the following:

~/fission:

build/
      build.sh 
      out/
            A.dwo  
            A.o  
            Empty.dwo  
            Empty.o  
            libFoo.so  
            Main.exe
src/
      A.cpp
      Empty.cpp
      Main.cpp

Main.cpp:

int f();
int main() {
   int Y = f();
   return Y;
}

Empty.cpp:

#define PrettyMacro 1

A.cpp:

struct A {
  int X;
};

int f() {
  A a;
  a.X = 12;
  return a.X;
}

build.sh:

#!/bin/sh -e
CLANG4=/path/to/clang-4.0
$CLANG4 -fPIC -gsplit-dwarf -g -O0 -c ../src/Empty.cpp -o out/Empty.o
$CLANG4 -fPIC -gsplit-dwarf -g -O0 -c ../src/A.cpp -o out/A.o
$CLANG4 -shared out/A.o out/Empty.o -o out/libFoo.so
$CLANG4 -fPIC -gsplit-dwarf -g -O0 ../src/Main.cpp -L`pwd`/out -lFoo -o out/Main.exe

running:

export LD_LIBRARY_PATH="`pwd`/build/out"
lldb -- ./build/out/Main.exe

br set --name f

  Process 3169970 stopped
  thread #1, name = 'Main.exe', stop reason = breakpoint 1.1
  frame #0: 0x00007ffff7ff4634 libFoo.so`f() at A.cpp:7
  4
  5   	int f() {
  6   	  A a;
  7   	  a.X = 12;
  8   	  return a.X;
  9   	}

(lldb) p a
warning: (x86_64) /home/alexshap/fission/build/out/Empty.dwo 0x0000000b: unable to locate module needed for external types: out/Empty.dwo
error: 'out/Empty.dwo' does not exist
Debugging will be degraded due to missing types. Rebuilding your project will regenerate the needed module files.

P.S.

llvm-dwarfdump Empty.dwo outputs the following:

.debug_info.dwo contents:
0x00000000: Compile Unit: length = 0x00000015 version = 0x0004 abbr_offset = 0x0000 addr_size = 
0x08 (next unit at 0x00000019)

0x0000000b: DW_TAG_compile_unit [1]
            DW_AT_GNU_dwo_name [DW_FORM_GNU_str_index]	( indexed (00000000) string = 
"out/Empty.dwo")
            DW_AT_producer [DW_FORM_GNU_str_index]	( indexed (00000001) string = "clang 
 version 4.0.0 (llvm: 418e5baaeb312dce78aef32a0e53911b89828a15, cfe: 
 50611996792539b2cb857de93ab34330feef5e46, compiler-rt: 
 f30125d3ab56aa0003adcb63baf43471d0c99105, lld: 
 f3e1544a0fd20c1f8875d86ee53fda439301da11) (based on LLVM 4.0.0)")
            DW_AT_language [DW_FORM_data2]	(DW_LANG_C_plus_plus)
            DW_AT_name [DW_FORM_GNU_str_index]	( indexed (00000002) string = 
 "../src/Empty.cpp")
            DW_AT_GNU_dwo_id [DW_FORM_data8]	(0x3fb4b906a44efb8f)

[alexander-shaposhnikov]

@clayborg, many thanks for the review,
i've added above a comment with more details about the issue (to the best my knowledge).

[clayborg]

Thanks for the info. Does this seem like it is a compiler bug? Seems like there should be a DW_AT_comp_dir if the DW_AT_GNU_dwo_name is relative? Is it written in the DWO spec that if the DW_AT_comp_dir is empty then it is referring to itself? I would rather not hack LLDB if we can avoid it. If this is a long released compiler generating this, then we do need a fix in LLDB since it will be running into these binaries from here on out. If this is a new compiler, lets fix the compiler. Let me know which case this is.

[alexander-shaposhnikov]

@clayborg - yeah, many thanks for the comments

Does this seem like it is a compiler bug

yeah, i think it might be the case, although right now i can't say for sure and i need to consult with the DWO spec

If this is a long released compiler generating this, then we do need a fix in LLDB since it will be running into these binaries from here on out.

yeah, it's (a long released) compiler (4.0), so this will be this way for some time.

besides fixing things for 4.0 I had another thought why this change could be reasonable - LLDB does a linear scan (inside GetSharedModule (in ModuleList::FindModules))
(in this build configuration the size of ModuleList is poportional to the total number of .dwo files which can be quite large)
and in this particular case this can be avoided (because we don't actually need to load the already loaded module).

[alexander-shaposhnikov]

ping

[clayborg]

Just add a more descriptive comment that states this exact issue more clearly as I didn't understand the issue from the comment that is there now (the comment that starts on line 1644).

lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

if (die && die.HasChildren() == false) {
                   die.GetAttributeValueAsString(DW_AT_comp_dir, nullptr);
               if (comp_dir) {
                 dwo_module_spec.GetFileSpec().SetFile(comp_dir, true);
                 dwo_module_spec.GetFileSpec().AppendPathComponent(dwo_path);
               }
             }
             dwo_module_spec.GetArchitecture() =
                 m_obj_file->GetModule()->GetArchitecture();
-            // printf ("Loading dwo = '%s'\n", dwo_path);
+
+            // When LLDB loads "external" modules it looks at the
+            // presence of DW_AT_GNU_dwo_name.
+            // However, when the already created module
+            // (corresponding to .dwo itself) is being processed,
+            // it will see the presence of DW_AT_GNU_dwo_name
+            // (which contains the name of dwo file) and
+            // will try to call ModuleList::GetSharedModule again.
+            // In some cases (i.e. for empty files) Clang 4.0
+            // generates a *.dwo file which has DW_AT_GNU_dwo_name,
+            // but no DW_AT_comp_dir. In this case the method
+            // ModuleList::GetSharedModule will fail and
+            // the warning will be printed. However, as one can notice
+            // in this case we don't actually need to try to load the already
+            // loaded module (corresponding to .dwo) so we simply skip it.
+            if (m_obj_file->GetFileSpec()
+                        .GetFileNameExtension()
+                        .GetStringRef() == "dwo" &&
+                llvm::StringRef(m_obj_file->GetFileSpec().GetPath())
+                    .endswith(dwo_module_spec.GetFileSpec().GetPath())) {
+              continue;
+            }
+
             Status error = ModuleList::GetSharedModule(
                 dwo_module_spec, module_sp, NULL, NULL, NULL);
             if (!module_sp) {
               GetObjectFile()->GetModule()->ReportWarning(
                   "0x%8.8x: unable to locate module needed for external types: "
                   "%s\nerror: %s\nDebugging will be degraded due to missing "
                   "types. Rebuilding your project will regenerate the needed "
                   "module files.",