[Lldb-commits] [PATCH] D43688: Partial fix for TestConflictingSymbol.py on Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a subscriber: spyffe.
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D43688#1017831, @amccarth wrote:

> OK, as far as I can tell, Windows doesn't have anything equivalent to the .symtab:  a DLL has exported symbols and it might have debug info (usually in a PDB).  There's no "third place" to look.
>
> This test seems intent on making sure this symbol is neither exported nor placed in the debug info.  I can see why that would be an interesting test case on Linux, since there you've got .symtab.
>
> But that doesn't seem to be the intent of _this_ test.  It seems that the intent here is to make sure the debugger can distinguish between two identically named symbols that happen to reside in different shared libraries/DLLs.  I don't understand why the test is trying to keep these symbols out of the debug info.
>
> > I have no idea how these things work on windows. If we have no way of finding this variable without debug info, then possibly the test just does not makes sense on windows.
>
> (And I have no ideas how things work on Linux.)
>
> What I think the test is supposed to check (distinguishing between symbols in different libraries) _is_ interesting on Windows.  Finding symbols that are not in the exported symbol table nor in the debug info would not be interesting on Windows.  Unfortunately, this test is trying to do both of those.
>
> I'm tempted to remove the extra work the test does to keep the symbols out of the debug info, so that we can test what the test actually purports to test.  But I don't know enough about Linux.  Would having the conflicting symbol in the debug info of both libraries be invalid?


I think Sean was the one who added this test, so we would have to ask him if this made a difference. What I know is that looking up things in the debug info and looking up via symbol table uses quite distinct code paths, and in fact I was not able to reproduce the bug in the second test in this test case (test_shadowed) without actually removing debug info (that is why I added the second test to this file).

So, not having a conflicting symbol in the debug info would *not* be invalid, and it sounds like a reasonable thing to test. However, I think we should do that via a separate test instead of modifying this test case.

> While it's unlikely in this case, I believe the Windows rules are convoluted enough that, in the general case, you might not get the exact DLL you request even when using a full path.  And, anyway, pre-loading the module doesn't seem to be the point of _this_ test.

Fair enough, that's true. lgtm then.


https://reviews.llvm.org/D43688