Michael137 created this revision. Michael137 added a reviewer: labath. Herald added a reviewer: shafik. Herald added a project: All. Michael137 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
This check was put in place to prevent static functions from translation units outside the one that the current expression is evaluated from taking precedence over functions in the global namespace. However, this is really a different bug. LLDB lumps functions from all CUs into a single AST and ends up picking the file-static even when C++ context rules wouldn't allow that to happen. This patch removes the check so we apply the AsmLabel to all FunctionDecls we create from DWARF if we have a linkage name available. This makes the code-path easier to reason about and allows calling static functions in contexts where we previously would've chosen the wrong function. We also flip the XFAILs in the API test to reflect what effect this change has. **Testing** - Fixed API tests and added XFAIL Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D132231 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py =================================================================== --- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py +++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py @@ -36,6 +36,26 @@ substrs=['stopped', 'stop reason = breakpoint']) + @skipIfWindows # This is flakey on Windows: llvm.org/pr38373 + @expectedFailure("CU-local objects incorrectly scoped") + def test_scope_lookup_with_run_command(self): + """Test scope lookup of functions in lldb.""" + self.build() + + lldbutil.run_to_source_breakpoint( + self, + self.line_break_global_scope, + lldb.SBFileSpec("ns.cpp")) + + # Evaluate func() - should call ::func() + # FIXME: LLDB does not correctly scope CU-local objects. + # LLDB currently lumps functions from all files into + # a single AST and depending on the order with which + # functions are considered, LLDB can incorrectly call + # the static local ns.cpp::func() instead of the expected + # ::func() + self.expect_expr("func()", expect_type="int", expect_value="1") + @skipIfWindows # This is flakey on Windows: llvm.org/pr38373 def test_scope_lookup_with_run_command(self): """Test scope lookup of functions in lldb.""" @@ -81,8 +101,7 @@ # Run to BP_global_scope at global scope self.runToBkpt("run") - # Evaluate func() - should call ::func() - self.expect("expr -- func()", startstr="(int) $0 = 1") + # Evaluate A::B::func() - should call A::B::func() self.expect("expr -- A::B::func()", startstr="(int) $1 = 4") # Evaluate func(10) - should call ::func(int) @@ -174,7 +193,6 @@ # before functions. self.expect("expr -- foo()", startstr="(int) $2 = 42") - @expectedFailure("lldb file scope lookup bugs") @skipIfWindows # This is flakey on Windows: llvm.org/pr38373 def test_file_scope_lookup_with_run_command(self): """Test file scope lookup in lldb.""" @@ -191,9 +209,7 @@ # Run to BP_file_scope at file scope self.runToBkpt("run") # Evaluate func() - should call static ns2.cpp:func() - # FIXME: This test fails because lldb doesn't know about file scopes so - # finds the global ::func(). - self.expect("expr -- func()", startstr="(int) $0 = 2") + self.expect_expr("func()", result_type="int", result_value="2") @skipIfWindows # This is flakey on Windows: llvm.org/pr38373 def test_scope_lookup_before_using_with_run_command(self): Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -1258,7 +1258,7 @@ // example is generating calls to ABI-tagged template functions. // This is done separately for member functions in // AddMethodToCXXRecordType. - if (attrs.mangled_name && attrs.storage == clang::SC_Extern) + if (attrs.mangled_name) function_decl->addAttr(clang::AsmLabelAttr::CreateImplicit( m_ast.getASTContext(), attrs.mangled_name, /*literal=*/false));
Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py =================================================================== --- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py +++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py @@ -36,6 +36,26 @@ substrs=['stopped', 'stop reason = breakpoint']) + @skipIfWindows # This is flakey on Windows: llvm.org/pr38373 + @expectedFailure("CU-local objects incorrectly scoped") + def test_scope_lookup_with_run_command(self): + """Test scope lookup of functions in lldb.""" + self.build() + + lldbutil.run_to_source_breakpoint( + self, + self.line_break_global_scope, + lldb.SBFileSpec("ns.cpp")) + + # Evaluate func() - should call ::func() + # FIXME: LLDB does not correctly scope CU-local objects. + # LLDB currently lumps functions from all files into + # a single AST and depending on the order with which + # functions are considered, LLDB can incorrectly call + # the static local ns.cpp::func() instead of the expected + # ::func() + self.expect_expr("func()", expect_type="int", expect_value="1") + @skipIfWindows # This is flakey on Windows: llvm.org/pr38373 def test_scope_lookup_with_run_command(self): """Test scope lookup of functions in lldb.""" @@ -81,8 +101,7 @@ # Run to BP_global_scope at global scope self.runToBkpt("run") - # Evaluate func() - should call ::func() - self.expect("expr -- func()", startstr="(int) $0 = 1") + # Evaluate A::B::func() - should call A::B::func() self.expect("expr -- A::B::func()", startstr="(int) $1 = 4") # Evaluate func(10) - should call ::func(int) @@ -174,7 +193,6 @@ # before functions. self.expect("expr -- foo()", startstr="(int) $2 = 42") - @expectedFailure("lldb file scope lookup bugs") @skipIfWindows # This is flakey on Windows: llvm.org/pr38373 def test_file_scope_lookup_with_run_command(self): """Test file scope lookup in lldb.""" @@ -191,9 +209,7 @@ # Run to BP_file_scope at file scope self.runToBkpt("run") # Evaluate func() - should call static ns2.cpp:func() - # FIXME: This test fails because lldb doesn't know about file scopes so - # finds the global ::func(). - self.expect("expr -- func()", startstr="(int) $0 = 2") + self.expect_expr("func()", result_type="int", result_value="2") @skipIfWindows # This is flakey on Windows: llvm.org/pr38373 def test_scope_lookup_before_using_with_run_command(self): Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -1258,7 +1258,7 @@ // example is generating calls to ABI-tagged template functions. // This is done separately for member functions in // AddMethodToCXXRecordType. - if (attrs.mangled_name && attrs.storage == clang::SC_Extern) + if (attrs.mangled_name) function_decl->addAttr(clang::AsmLabelAttr::CreateImplicit( m_ast.getASTContext(), attrs.mangled_name, /*literal=*/false));
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits