xiaobai created this revision.
With this patch I want to do the following:
When searching for global data symbols, only consider non-externally visible
symbols if we are in the module where they are visible.
I was investigating bug 35043
(https://bugs.llvm.org/show_bug.cgi?id=35043)
and found out I was able to reproduce this on a Linux x86-64 machine I have
access to. I was doing some digging, and found out that even when we're not in
the module with visibility, we would still consider internal symbols from other
modules when clang was asking us to find external declarations by name.
So in TestLambdas, we want to define a lambda with parameters (int a, int b).
Clang wants to ensure we don't redeclare our parameters. Through a long chain of
function calls, the call chain ends up
at SymbolContext::FindBestGlobalDataSymbol(), which (on my machine) will find
that there is are symbols named "a" and "b" in another module, but they are not
externally visible. lldb will complain that there are multiple defined internal
symbols, but these shouldn't matter for our lambda.
I don't have as much context as some of y'all about whether or not this kind of
change will present significant problems elsewhere, or if we should tackle this
problem another way. Feedback is very much appreciated.
https://reviews.llvm.org/D39307
Files:
packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
source/Symbol/SymbolContext.cpp
Index: source/Symbol/SymbolContext.cpp
===================================================================
--- source/Symbol/SymbolContext.cpp
+++ source/Symbol/SymbolContext.cpp
@@ -802,156 +802,152 @@
const Symbol *
SymbolContext::FindBestGlobalDataSymbol(const ConstString &name, Status &error) {
error.Clear();
-
+
if (!target_sp) {
return nullptr;
}
-
+
Target &target = *target_sp;
Module *module = module_sp.get();
-
- auto ProcessMatches = [this, &name, &target, module]
- (SymbolContextList &sc_list, Status &error) -> const Symbol* {
- llvm::SmallVector<const Symbol *, 1> external_symbols;
- llvm::SmallVector<const Symbol *, 1> internal_symbols;
+
+ auto ProcessMatches = [this, &name, &target,
+ module](SymbolContextList &sc_list, Status &error,
+ bool external_only) -> const Symbol * {
+ llvm::SmallVector<const Symbol *, 1> 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;
- }
+ 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 (external_only) {
if (symbol->IsExternal()) {
- external_symbols.push_back(symbol);
- } else {
- internal_symbols.push_back(symbol);
+ 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 =
+ } else {
+ 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);
}
- // 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 FindBestGlobalDataSymbol(
- symbol->GetReExportedSymbolName(), error);
}
- } 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;
+ // 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 FindBestGlobalDataSymbol(symbol->GetReExportedSymbolName(),
+ error);
+ }
+ } 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) {
+
+ if (symbols.size() > 1) {
StreamString ss;
- ss.Printf("Multiple external symbols found for '%s'\n", name.AsCString());
- for (const Symbol *symbol : external_symbols) {
+ if (external_only)
+ ss.Printf("Multiple external symbols found for '%s'\n", name.AsCString());
+ else
+ ss.Printf("Multiple internal symbols found for '%s'\n", name.AsCString());
+ for (const Symbol *symbol : 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 if (symbols.size()) {
+ return symbols[0];
} else {
return nullptr;
}
};
-
+
if (module) {
SymbolContextList sc_list;
module->FindSymbolsWithNameAndType(name, eSymbolTypeAny, sc_list);
- const Symbol *const module_symbol = ProcessMatches(sc_list, error);
-
+ const Symbol *const module_symbol =
+ ProcessMatches(sc_list, error, /* external_only = */ false);
+
if (!error.Success()) {
return nullptr;
} else if (module_symbol) {
return module_symbol;
}
}
-
+
{
SymbolContextList sc_list;
target.GetImages().FindSymbolsWithNameAndType(name, eSymbolTypeAny,
sc_list);
- const Symbol *const target_symbol = ProcessMatches(sc_list, error);
-
+ const Symbol *const target_symbol =
+ ProcessMatches(sc_list, error, /* external_only = */ true);
+
if (!error.Success()) {
return nullptr;
} else if (target_symbol) {
return target_symbol;
}
}
-
+
return nullptr; // no error; we just didn't find anything
}
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
@@ -87,4 +87,4 @@
"An error should be printed when symbols can't be ordered",
error=True,
substrs=[
- "Multiple internal symbols"])
+ "use of undeclared identifier"])
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
