labath added inline comments.

================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4103
+    // this point.
+    // TODO:  Is this part worthwhile?  `foo.exe` will never match `foo.pdb`
+    if (matching_modules.IsEmpty())
----------------
This is not unreasonable in the non-pdb world. You can have a stripped version 
of a file somewhere prepared for deployment to some device (or downloaded from 
the device), and then you'll also have an unstripped version of that file (with 
the same name) in some build folder.


================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4149
+    
+    lldbassert(matching_modules.GetSize() == 1);
+    ModuleSP module_sp(matching_modules.GetModuleAtIndex(0));
----------------
This should be a regular assert according to 
<https://lldb.llvm.org/resources/contributing.html#error-handling-and-use-of-assertions-in-lldb>.


================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4175-4178
+    if (object_file->GetFileSpec() != symbol_fspec) {
+      result.AppendWarning("there is a discrepancy between the module file "
+                           "spec and the symbol file spec\n");
+    }
----------------
This part is not good. Everywhere else except pdbs this means that we were in 
fact unable to load the symbol file. What happens is that if we cannot load the 
object file representing the symbol file (at [[ 
https://github.com/llvm/llvm-project/blob/master/lldb/source/Symbol/SymbolVendor.cpp#L48
 | SymbolVendor.cpp:48 ]]), we fall back to creating a SymbolFile using the 
object file of the original module (line 52).

The effect of that is that the symbol file creation will always succeed, the 
previous checks are more-or-less useless, and the only way to check if we are 
really using the symbols from the file the user specified is to compare the 
file specs.

Now, PDB symbol files are abusing this fallback, and reading the symbols from 
the pdb files even though they were in fact asked to read them from the 
executable file. This is why this may sound like a "discrepancy" coming from 
the pdb world, but everywhere else this just means that the symbol file was not 
actually loaded.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73594/new/

https://reviews.llvm.org/D73594



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to