RFC: Removing the DIA PDB plugin from LLDB

[DavidSpickett]

Thanks and thanks for explaining the causes.

I bodged the setting to default to the native plugin and got mostly the same results on Windows on Arm:

********************
Failed Tests (12):
  lldb-shell :: BuildScript/toolchain-msvc.test
  lldb-shell :: Settings/TestFrameFormatFunctionReturnObjC.test
  lldb-shell :: SymbolFile/DWARF/dwo-static-data-member-access.test
  lldb-shell :: SymbolFile/PDB/expressions.test
  lldb-shell :: SymbolFile/PDB/func-symbols.test
  lldb-shell :: SymbolFile/PDB/function-nested-block.test
  lldb-shell :: SymbolFile/PDB/native-setting.cpp
  lldb-shell :: SymbolFile/PDB/pointers.test
  lldb-shell :: SymbolFile/PDB/type-quals.test
  lldb-shell :: SymbolFile/PDB/typedefs.test
  lldb-shell :: SymbolFile/PDB/udt-layout.test
  lldb-shell :: SymbolFile/PDB/variables.test

********************
Unexpectedly Passed Tests (1):
  lldb-shell :: Commands/command-disassemble-mixed.c

toolchain-msvc.test is something to do with how I set up my developer terminal session, ignore that.

Then there are 11 failures like you had, minus the x86_64 specific windows-unaligned-x86_64.test. Which makes sense.

command-disassemble-mixed.c I don’t know why it’s not passing but hey, probably a good thing

@Nerixyz

breakpoint set -n <name> doesn’t work with the native plugin(?).

Certainly something we should fix before flipping the default.

This seems like a good blocker for switching. If there’s obvious user commands that don’t work yet. Beyond that, we can decide if the remaining issues are actually blockers or have decent workarounds (one of which will be switching back to the old plugin, but ideally we don’t end up forcing everyone to do that).

I will remind myself to revisit this in a month or and we can make a decision or wait longer if neccesary.

@Nerixyz

SymbolFile/DWARF/dwo-static-data-member-access.test (known)

@Nerixyz

Though for the second test, it’s unexpected that this runs with PDB on Windows in the first place.

I will try to figure that out.

@Nerixyz

SymbolFile/PDB/udt-layout.test (known)
This hits an assertion failure, because it tries to print a static whose type doesn’t have a layout through target variable. It’s a type from the CRT that is forward declared without a complete definition.

Does this mean there is a missing feature in the native plugin to handle this situation, or that the test itself is either incorrect, or asking for trouble by requesting this type.

For instance, would it still serve it’s purpose if it asked for something else that’s easier to deal with?

[labath]

@DavidSpickett

Does this mean there is a missing feature in the native plugin to handle this situation, or that the test itself is either incorrect, or asking for trouble by requesting this type.

For instance, would it still serve it’s purpose if it asked for something else that’s easier to deal with?

That definitely works in DWARF (as in, you don’t crash and get a reasonable error message), so I’d say that’s a PDB bug.

$ cat d.cc
struct Foo { Foo(); };

Foo foo;
$ clang++ -c d.cc  -g
$ lldb d.o 
(lldb) target create "d.o"
Current executable set to '/tmp/d.o' (x86_64).
(lldb) target variable foo
(Foo) foo = <incomplete type "Foo">

[charles-zablit]

Here is an updated list of the failing tests on x86_64 as of 3b14414cbdb8bd37ff861862a99a47104b87765b:

********************
Failed Tests (3):
  lldb-shell :: SymbolFile/PDB/func-symbols.test
  lldb-shell :: SymbolFile/PDB/pointers.test
  lldb-shell :: SymbolFile/PDB/typedefs.test

[Nerixyz]

There shouldn’t be any failing tests on x86_64 anymore when LLDB is compiled without DIA.

[DavidSpickett]

Great news!

At the moment I do not have access to our Windows on Arm machines to confirm, but it seems likely that besides 1 or 2 tests, the same will be true there.

We can change the default and if Arm is a total mess revert it, otherwise, xfail individual tests and we’ll get to them when we can.

PR welcome to switch the default. Please include a release note, which also explains how to switch back to DIA, and that support for it will be removed in a future release.

[DavidSpickett]

As of [LLDB] Use native PDB reader by default by Nerixyz · Pull Request #165363 · llvm/llvm-project · GitHub, the native plugin is the default.

Quoting the release note:

• The default PDB reader on Windows was changed from DIA to native, which uses LLVM’s PDB and CodeView support. You can switch back to the DIA reader with settings set plugin.symbol-file.pdb.reader dia. Note that support for the DIA reader will be removed in a future version of LLDB.

This will be included in the future LLDB 22 release.

If anyone finds bugs due to the new default, please raise an issue.

Thanks again to @Nerixyz, @charles-zablit and others who worked on this.