spyffe updated this revision to Diff 98672.
spyffe added a comment.
Improved symbol lookup per Greg and Jim's suggestion, with flipped steps 2 and
3.
Also added testing for the error case.
https://reviews.llvm.org/D33083
Files:
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
===================================================================
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/main.c
@@ -0,0 +1,11 @@
+#include "One/One.h"
+#include "Two/Two.h"
+
+#include <stdio.h>
+
+int main() {
+ one();
+ two();
+ printf("main\n"); // break here
+ return(0);
+}
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
===================================================================
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/TwoConstant.c
@@ -0,0 +1 @@
+__private_extern__ int conflicting_symbol = 22222;
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
===================================================================
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
@@ -0,0 +1,4 @@
+#ifndef TWO_H
+#define TWO_H
+void two();
+#endif
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
===================================================================
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.c
@@ -0,0 +1,6 @@
+#include "Two.h"
+#include <stdio.h>
+
+void two() {
+ printf("Two\n"); // break here
+}
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
===================================================================
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
@@ -0,0 +1,87 @@
+"""Test that conflicting symbols in different shared libraries work correctly"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestRealDefinition(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ @skipUnlessDarwin
+ def test_frame_var_after_stop_at_implementation(self):
+ if self.getArchitecture() == 'i386':
+ self.skipTest("requires modern objc runtime")
+ self.build()
+ self.common_setup()
+
+ One_line = line_number('One/One.c', '// break here')
+ Two_line = line_number('Two/Two.c', '// break here')
+ main_line = line_number('main.c', '// break here')
+ lldbutil.run_break_set_by_file_and_line(
+ self, 'One.c', One_line, num_expected_locations=1, loc_exact=True)
+ lldbutil.run_break_set_by_file_and_line(
+ self, 'Two.c', Two_line, num_expected_locations=1, loc_exact=True)
+ lldbutil.run_break_set_by_file_and_line(
+ self, 'main.c', main_line, num_expected_locations=1, loc_exact=True)
+
+ self.runCmd("run", RUN_SUCCEEDED)
+
+ # The stop reason of the thread should be breakpoint.
+ self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+ substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+ self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
+ substrs=[' resolved, hit count = 1'])
+
+ # This should display correctly.
+ self.expect(
+ "expr (unsigned long long)conflicting_symbol",
+ "Symbol from One should be found",
+ substrs=[
+ "11111"])
+
+ self.runCmd("continue", RUN_SUCCEEDED)
+
+ # The stop reason of the thread should be breakpoint.
+ self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+ substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+ self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
+ substrs=[' resolved, hit count = 1'])
+
+ self.expect(
+ "expr (unsigned long long)conflicting_symbol",
+ "Symbol from Two should be found",
+ substrs=[
+ "22222"])
+
+ self.runCmd("continue", RUN_SUCCEEDED)
+
+ # The stop reason of the thread should be breakpoint.
+ self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+ substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+ self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
+ substrs=[' resolved, hit count = 1'])
+
+ self.expect(
+ "expr (unsigned long long)conflicting_symbol",
+ "An error should be printed when symbols can't be ordered",
+ error=True,
+ substrs=[
+ "Multiple internal symbols"])
+
+ def common_setup(self):
+ exe = os.path.join(os.getcwd(), "a.out")
+ self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
===================================================================
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/OneConstant.c
@@ -0,0 +1 @@
+__private_extern__ int conflicting_symbol = 11111;
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
===================================================================
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
@@ -0,0 +1,4 @@
+#ifndef ONE_H
+#define ONE_H
+void one();
+#endif
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
===================================================================
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.c
@@ -0,0 +1,6 @@
+#include "One.h"
+#include <stdio.h>
+
+void one() {
+ printf("One\n"); // break here
+}
Index: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
===================================================================
--- packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
+++ packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile
@@ -0,0 +1,27 @@
+LEVEL = ../../../make
+
+DEBUG_CFLAGS = -g -O0
+NDEBUG_CFLAGS = -O0
+LDFLAGS = $(CFLAGS) -lobjc -framework Foundation
+
+all: a.out libOne.dylib libTwo.dylib
+
+libOne.dylib: One/One.c One/OneConstant.c
+ $(CC) $(DEBUG_CFLAGS) -I. -c -o One.o One/One.c
+ $(CC) $(NDEBUG_CFLAGS) -I. -c -o OneConstant.o One/OneConstant.c
+ $(CC) $(LDFLAGS) -shared -o libOne.dylib One.o OneConstant.o
+ dsymutil libOne.dylib
+
+libTwo.dylib: Two/Two.c Two/TwoConstant.c
+ $(CC) $(DEBUG_CFLAGS) -I. -c -o Two.o Two/Two.c
+ $(CC) $(NDEBUG_CFLAGS) -I. -c -o TwoConstant.o Two/TwoConstant.c
+ $(CC) $(LDFLAGS) -shared -o libTwo.dylib Two.o TwoConstant.o
+ dsymutil libTwo.dylib
+
+a.out: main.c libOne.dylib libTwo.dylib
+ $(CC) $(DEBUG_CFLAGS) $(LDFLAGS) -I. -L. -lOne -lTwo -o a.out main.c
+
+.PHONY: clean
+
+clean:
+ rm -rf *.dylib *.o *.dSYM a.out
Index: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
===================================================================
--- source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
@@ -450,20 +450,21 @@
//------------------------------------------------------------------
/// Given a target, find a data symbol that has the given name.
///
- /// @param[in] target
- /// The target to use as the basis for the search.
+ /// @param[in] lookup_context
+ /// The symbol context to use to order the search.
///
/// @param[in] name
/// The name as a plain C string.
///
- /// @param[in] module
- /// The module to limit the search to. This can be NULL
+ /// @param[in] error
+ /// An error to populate with information if there is a lookup failure
///
/// @return
/// The LLDB Symbol found, or NULL if none was found.
//------------------------------------------------------------------
- const Symbol *FindGlobalDataSymbol(Target &target, const ConstString &name,
- Module *module = NULL);
+ const Symbol *FindGlobalDataSymbol(SymbolContext &lookup_context,
+ const ConstString &name,
+ Error &error);
//------------------------------------------------------------------
/// Given a target, find a variable that matches the given name and
Index: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===================================================================
--- source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -592,100 +592,160 @@
}
const Symbol *ClangExpressionDeclMap::FindGlobalDataSymbol(
- Target &target, const ConstString &name, lldb_private::Module *module) {
- SymbolContextList sc_list;
-
- if (module)
- module->FindSymbolsWithNameAndType(name, eSymbolTypeAny, sc_list);
- else
- target.GetImages().FindSymbolsWithNameAndType(name, eSymbolTypeAny,
- sc_list);
-
- const uint32_t matches = sc_list.GetSize();
- for (uint32_t i = 0; i < matches; ++i) {
- SymbolContext sym_ctx;
- sc_list.GetContextAtIndex(i, sym_ctx);
- if (sym_ctx.symbol) {
- const Symbol *symbol = sym_ctx.symbol;
- const Address sym_address = symbol->GetAddress();
-
- if (sym_address.IsValid()) {
- switch (symbol->GetType()) {
- case eSymbolTypeData:
- case eSymbolTypeRuntime:
- case eSymbolTypeAbsolute:
- case eSymbolTypeObjCClass:
- case eSymbolTypeObjCMetaClass:
- case eSymbolTypeObjCIVar:
- if (symbol->GetDemangledNameIsSynthesized()) {
- // If the demangled name was synthesized, then don't use it
- // for expressions. Only let the symbol match if the mangled
- // named matches for these symbols.
- if (symbol->GetMangled().GetMangledName() != name)
- break;
- }
- return symbol;
-
- case eSymbolTypeReExported: {
- ConstString reexport_name = symbol->GetReExportedSymbolName();
- if (reexport_name) {
- ModuleSP reexport_module_sp;
- ModuleSpec reexport_module_spec;
- reexport_module_spec.GetPlatformFileSpec() =
- symbol->GetReExportedSymbolSharedLibrary();
- if (reexport_module_spec.GetPlatformFileSpec()) {
- reexport_module_sp =
+ SymbolContext &lookup_context, const ConstString &name, Error &error) {
+ error.Clear();
+
+ if (!lookup_context.target_sp) {
+ return nullptr;
+ }
+
+ Target &target = *lookup_context.target_sp;
+ Module *module = lookup_context.module_sp.get();
+
+ auto ProcessMatches = [this, &name, &target, module]
+ (SymbolContextList &sc_list, Error &error) -> const Symbol* {
+ llvm::SmallVector<const Symbol *, 1> external_symbols;
+ llvm::SmallVector<const Symbol *, 1> internal_symbols;
+ const uint32_t matches = sc_list.GetSize();
+ for (uint32_t i = 0; i < matches; ++i) {
+ SymbolContext sym_ctx;
+ sc_list.GetContextAtIndex(i, sym_ctx);
+ if (sym_ctx.symbol) {
+ const Symbol *symbol = sym_ctx.symbol;
+ const Address sym_address = symbol->GetAddress();
+
+ if (sym_address.IsValid()) {
+ switch (symbol->GetType()) {
+ case eSymbolTypeData:
+ case eSymbolTypeRuntime:
+ case eSymbolTypeAbsolute:
+ case eSymbolTypeObjCClass:
+ case eSymbolTypeObjCMetaClass:
+ case eSymbolTypeObjCIVar:
+ if (symbol->GetDemangledNameIsSynthesized()) {
+ // If the demangled name was synthesized, then don't use it
+ // for expressions. Only let the symbol match if the mangled
+ // named matches for these symbols.
+ if (symbol->GetMangled().GetMangledName() != name)
+ break;
+ }
+ if (symbol->IsExternal()) {
+ external_symbols.push_back(symbol);
+ } else {
+ internal_symbols.push_back(symbol);
+ }
+ break;
+ case eSymbolTypeReExported: {
+ ConstString reexport_name = symbol->GetReExportedSymbolName();
+ if (reexport_name) {
+ ModuleSP reexport_module_sp;
+ ModuleSpec reexport_module_spec;
+ reexport_module_spec.GetPlatformFileSpec() =
+ symbol->GetReExportedSymbolSharedLibrary();
+ if (reexport_module_spec.GetPlatformFileSpec()) {
+ reexport_module_sp =
+ target.GetImages().FindFirstModule(reexport_module_spec);
+ if (!reexport_module_sp) {
+ reexport_module_spec.GetPlatformFileSpec()
+ .GetDirectory()
+ .Clear();
+ reexport_module_sp =
target.GetImages().FindFirstModule(reexport_module_spec);
- if (!reexport_module_sp) {
- reexport_module_spec.GetPlatformFileSpec()
- .GetDirectory()
- .Clear();
- reexport_module_sp =
- target.GetImages().FindFirstModule(reexport_module_spec);
+ }
}
+ // Don't allow us to try and resolve a re-exported symbol if it is
+ // the same as the current symbol
+ if (name == symbol->GetReExportedSymbolName() &&
+ module == reexport_module_sp.get())
+ return nullptr;
+
+ return FindGlobalDataSymbol(sym_ctx,
+ symbol->GetReExportedSymbolName(),
+ error);
}
- // Don't allow us to try and resolve a re-exported symbol if it is
- // the same
- // as the current symbol
- if (name == symbol->GetReExportedSymbolName() &&
- module == reexport_module_sp.get())
- return NULL;
-
- return FindGlobalDataSymbol(target,
- symbol->GetReExportedSymbolName(),
- reexport_module_sp.get());
+ } break;
+
+ case eSymbolTypeCode: // We already lookup functions elsewhere
+ case eSymbolTypeVariable:
+ case eSymbolTypeLocal:
+ case eSymbolTypeParam:
+ case eSymbolTypeTrampoline:
+ case eSymbolTypeInvalid:
+ case eSymbolTypeException:
+ case eSymbolTypeSourceFile:
+ case eSymbolTypeHeaderFile:
+ case eSymbolTypeObjectFile:
+ case eSymbolTypeCommonBlock:
+ case eSymbolTypeBlock:
+ case eSymbolTypeVariableType:
+ case eSymbolTypeLineEntry:
+ case eSymbolTypeLineHeader:
+ case eSymbolTypeScopeBegin:
+ case eSymbolTypeScopeEnd:
+ case eSymbolTypeAdditional:
+ case eSymbolTypeCompiler:
+ case eSymbolTypeInstrumentation:
+ case eSymbolTypeUndefined:
+ case eSymbolTypeResolver:
+ break;
}
- } break;
-
- case eSymbolTypeCode: // We already lookup functions elsewhere
- case eSymbolTypeVariable:
- case eSymbolTypeLocal:
- case eSymbolTypeParam:
- case eSymbolTypeTrampoline:
- case eSymbolTypeInvalid:
- case eSymbolTypeException:
- case eSymbolTypeSourceFile:
- case eSymbolTypeHeaderFile:
- case eSymbolTypeObjectFile:
- case eSymbolTypeCommonBlock:
- case eSymbolTypeBlock:
- case eSymbolTypeVariableType:
- case eSymbolTypeLineEntry:
- case eSymbolTypeLineHeader:
- case eSymbolTypeScopeBegin:
- case eSymbolTypeScopeEnd:
- case eSymbolTypeAdditional:
- case eSymbolTypeCompiler:
- case eSymbolTypeInstrumentation:
- case eSymbolTypeUndefined:
- case eSymbolTypeResolver:
- break;
}
}
}
+
+ if (external_symbols.size() > 1) {
+ StreamString ss;
+ ss.Printf("Multiple external symbols found for '%s'\n", name.AsCString());
+ for (const Symbol *symbol : external_symbols) {
+ symbol->GetDescription(&ss, eDescriptionLevelFull, &target);
+ }
+ ss.PutChar('\n');
+ error.SetErrorString(ss.GetData());
+ return nullptr;
+ } else if (external_symbols.size()) {
+ return external_symbols[0];
+ } else if (internal_symbols.size() > 1) {
+ StreamString ss;
+ ss.Printf("Multiple internal symbols found for '%s'\n", name.AsCString());
+ for (const Symbol *symbol : internal_symbols) {
+ symbol->GetDescription(&ss, eDescriptionLevelVerbose, &target);
+ ss.PutChar('\n');
+ }
+ error.SetErrorString(ss.GetData());
+ return nullptr;
+ } else if (internal_symbols.size()) {
+ return internal_symbols[0];
+ } else {
+ return nullptr; // don't set error, because we might find something later
+ }
+ };
+
+ if (module) {
+ SymbolContextList sc_list;
+ module->FindSymbolsWithNameAndType(name, eSymbolTypeAny, sc_list);
+ const Symbol *const module_symbol = ProcessMatches(sc_list, error);
+
+ if (!error.Success()) {
+ return nullptr;
+ } else if (module_symbol) {
+ return module_symbol;
+ }
}
-
- return NULL;
+
+ {
+ SymbolContextList sc_list;
+ target.GetImages().FindSymbolsWithNameAndType(name, eSymbolTypeAny,
+ sc_list);
+ const Symbol *const target_symbol = ProcessMatches(sc_list, error);
+
+ if (!error.Success()) {
+ return nullptr;
+ } else if (target_symbol) {
+ return target_symbol;
+ }
+ }
+
+ return nullptr; // no error; we just didn't find anything
}
lldb::VariableSP ClangExpressionDeclMap::FindGlobalVariable(
@@ -1526,9 +1586,18 @@
// We couldn't find a non-symbol variable for this. Now we'll hunt for
// a generic
// data symbol, and -- if it is found -- treat it as a variable.
-
- const Symbol *data_symbol = FindGlobalDataSymbol(*target, name);
-
+ Error error;
+
+ const Symbol *data_symbol = FindGlobalDataSymbol(m_parser_vars->m_sym_ctx,
+ name, error);
+
+ if (!error.Success()) {
+ const unsigned diag_id =
+ m_ast_context->getDiagnostics().getCustomDiagID(
+ clang::DiagnosticsEngine::Level::Error, "%0");
+ m_ast_context->getDiagnostics().Report(diag_id) << error.AsCString();
+ }
+
if (data_symbol) {
std::string warning("got name from symbols: ");
warning.append(name.AsCString());
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
