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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

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

> In https://reviews.llvm.org/D43688#1017638, @labath wrote:
>
> > On non-windows systems we're expected to pick up the name from the symbol table. Is there such a thing on windows as well?
>
>
> Given that constant is declared with visibility("hidden"), why would it end up in a symbol table?  Windows DLLs have tables for items explicitly exported, but that's contrary to asking the symbol to be hidden.


visibility((hidden)) means it won't end up in the *exported* symbol table (.dynsym on linux). However, the compiler will still put it into the local symbol table (.symtab, which is not used by the linker or at runtime, but the debugger can use it to get more insight). 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.

>> I think this could also be related to your  breakpoint location resolving problem. The test explicitly calls `self.registerSharedLibrariesWithTarget` which explicitly adds the shared library to the list of target modules, so (assuming registerSharedLibrariesWithTarget works on windows) this test should not depend on automatic library searching in lldb. I'm guessing you have some problems looking things up when debug info is absent.
> 
> No, registering the libraries doesn't work on Windows (as discussed in the other patch were we introduced the -2 magic value).  The Windows search rules for DLLs are quite complex -- it would be very difficult to replicate them in lldb.

Yes, but here I am not talking about parsing the imports table of the EXE and trying to replicate the OS search logic. Here, the test explicitly calls SBTarget.AddModule and specifies the full path of the module to add. I think this should work regardless of the platform.

>> With all that in mind, I don't think this patch would hurt this test, but I don't believe it will help you getting the test passing either, so I would keep the num_expected_locations as-is (the LLDB_TEST_API thingy is obviously fine).
> 
> But that would leave the test failing for a reason other than the underlying problem:  that we can't resolve the symbol, which probably indicates an incomplete implementation somewhere.  I think a test that failure that points to the root problem is more useful than a general this-doesn't-even-link-on-Windows error.

IIUC, the LLDB_TEST_API is the thing which fixes the link error. I have no objections to that.

The part that is not clear to me is why the test is failing to resolve the breakpoint even though the module was explicitly added through SBTarget.AddImage. Can you check at what the module list looks like after the AddImage call?


https://reviews.llvm.org/D43688