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

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

In https://reviews.llvm.org/D43688#1017638, @labath wrote:

> This test deliberately compiles without `-g`, so the variable should not be in the debug info.


Ah, that makes more sense than what I was thinking:  that it was trying to make sure the unused symbol wasn't being optimized away.

> 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.

> Maybe you need to lookup the symbol as `_conflicting_symbol` ?

No, that doesn't make a difference.

> 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.

> 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.


https://reviews.llvm.org/D43688