dawn created this revision.
dawn added a reviewer: jingham.
dawn added a subscriber: lldb-commits.
dawn set the repository for this revision to rL LLVM.
This patch fixes setting breakpoints on symbol for variants of C and Pascal
where the language is "unknown" within the filtering process added in r252356.
It also renames GetLanguageForSymbolByName to GuessLanguageForSymbolByName and
adds comments explaining the pitfalls of the flawed assumption that the
language can be determined solely from the name and target.
Comment: Our users want to be able to set breakpoints in C, C++ or Pascal, but
since the parsing of Pascal breakpoint identifiers is incompatible with ObjC,
they're forced to set the target language option to Pascal. After r252356
however, the Pascal identifiers are interpreted as C++ and the C symbols are
unknown, so all symbols are filtered out because they don't match the target
language setting of Pascal, and they can no longer set symbolic breakpoints. I
don't see a way to fix this without breaking the intent of r252356. Locally we
have had to disable the filtering code. Ideas for how to resolve this?
Repository:
rL LLVM
http://reviews.llvm.org/D15175
Files:
include/lldb/Target/LanguageRuntime.h
source/Breakpoint/BreakpointResolverName.cpp
source/Target/LanguageRuntime.cpp
Index: source/Target/LanguageRuntime.cpp
===================================================================
--- source/Target/LanguageRuntime.cpp
+++ source/Target/LanguageRuntime.cpp
@@ -347,16 +347,21 @@
}
lldb::LanguageType
-LanguageRuntime::GetLanguageForSymbolByName (Target &target, const char
*symbol_name)
+LanguageRuntime::GuessLanguageForSymbolByName (Target &target, const char
*symbol_name)
{
- // This is not the right way to do this. Different targets could have
different ways of mangling names
- // from a given language. So we should ask the various LanguageRuntime
plugin instances for this target
- // to recognize the name. But right now the plugin instances depend on
the process, not the target.
- // That is unfortunate, because I want to use this for filtering
breakpoints by language, and so I need to know
- // the "language for symbol-name" prior to running. So we'd have to make
a "LanguageRuntimeTarget" and
- // "LanguageRuntimeProcess", and direct the questions that don't need a
running process to the former, and that
- // do to the latter.
+ // We "guess" the language because we can't determine a symbol's language
from it's name.
+ // For example, a Pascal symbol can be mangled using the C++ Itanium
scheme, and defined
+ // in a compilation unit within the same module as other C++ units.
//
+ // In addition, different targets could have different ways of mangling
names from a given
+ // language, likewise compilation units within those targets. It would
help to be able to
+ // ask the various LanguageRuntime plugin instances for this target to
recognize the name,
+ // but right now the plugin instances depend on the process, not the
target. That is
+ // unfortunate, because to use this for filtering breakpoints by language,
we need to know
+ // the "language for symbol-name" prior to running. So we'd have to make a
+ // "LanguageRuntimeTarget" and "LanguageRuntimeProcess", and direct the
questions that don't
+ // need a running process to the former, and that do to the latter.
+ //
// That's more work than I want to do for this feature.
if (CPlusPlusLanguage::IsCPPMangledName (symbol_name))
return eLanguageTypeC_plus_plus;
Index: source/Breakpoint/BreakpointResolverName.cpp
===================================================================
--- source/Breakpoint/BreakpointResolverName.cpp
+++ source/Breakpoint/BreakpointResolverName.cpp
@@ -279,8 +279,9 @@
const char *name =
sc.GetFunctionName(Mangled::ePreferMangled).AsCString();
if (name)
{
- LanguageType sym_language =
LanguageRuntime::GetLanguageForSymbolByName(target, name);
- if (m_language == eLanguageTypeC)
+ LanguageType sym_language =
LanguageRuntime::GuessLanguageForSymbolByName(target, name);
+ if (Language::LanguageIsC(m_language) ||
+ Language::LanguageIsPascal(m_language))
{
// We don't currently have a way to say "This symbol
name is C" so for now, C means
// not ObjC and not C++, etc...
@@ -293,6 +294,12 @@
}
else if (sym_language != m_language)
{
+ // Note: This code prevents us from being able to find
symbols
+ // like 'printf' if the target language's option is
set. It
+ // would be better to limit this filtering to only
when the
+ // breakpoint's language option is set (and not the
target's),
+ // but we can't know if m_language was set from the
target or
+ // breakpoint option.
remove_it = true;
}
}
Index: include/lldb/Target/LanguageRuntime.h
===================================================================
--- include/lldb/Target/LanguageRuntime.h
+++ include/lldb/Target/LanguageRuntime.h
@@ -118,7 +118,7 @@
}
static lldb::LanguageType
- GetLanguageForSymbolByName (Target &target, const char *symbol_name);
+ GuessLanguageForSymbolByName (Target &target, const char *symbol_name);
Target&
GetTargetRef()
Index: source/Target/LanguageRuntime.cpp
===================================================================
--- source/Target/LanguageRuntime.cpp
+++ source/Target/LanguageRuntime.cpp
@@ -347,16 +347,21 @@
}
lldb::LanguageType
-LanguageRuntime::GetLanguageForSymbolByName (Target &target, const char *symbol_name)
+LanguageRuntime::GuessLanguageForSymbolByName (Target &target, const char *symbol_name)
{
- // This is not the right way to do this. Different targets could have different ways of mangling names
- // from a given language. So we should ask the various LanguageRuntime plugin instances for this target
- // to recognize the name. But right now the plugin instances depend on the process, not the target.
- // That is unfortunate, because I want to use this for filtering breakpoints by language, and so I need to know
- // the "language for symbol-name" prior to running. So we'd have to make a "LanguageRuntimeTarget" and
- // "LanguageRuntimeProcess", and direct the questions that don't need a running process to the former, and that
- // do to the latter.
+ // We "guess" the language because we can't determine a symbol's language from it's name.
+ // For example, a Pascal symbol can be mangled using the C++ Itanium scheme, and defined
+ // in a compilation unit within the same module as other C++ units.
//
+ // In addition, different targets could have different ways of mangling names from a given
+ // language, likewise compilation units within those targets. It would help to be able to
+ // ask the various LanguageRuntime plugin instances for this target to recognize the name,
+ // but right now the plugin instances depend on the process, not the target. That is
+ // unfortunate, because to use this for filtering breakpoints by language, we need to know
+ // the "language for symbol-name" prior to running. So we'd have to make a
+ // "LanguageRuntimeTarget" and "LanguageRuntimeProcess", and direct the questions that don't
+ // need a running process to the former, and that do to the latter.
+ //
// That's more work than I want to do for this feature.
if (CPlusPlusLanguage::IsCPPMangledName (symbol_name))
return eLanguageTypeC_plus_plus;
Index: source/Breakpoint/BreakpointResolverName.cpp
===================================================================
--- source/Breakpoint/BreakpointResolverName.cpp
+++ source/Breakpoint/BreakpointResolverName.cpp
@@ -279,8 +279,9 @@
const char *name = sc.GetFunctionName(Mangled::ePreferMangled).AsCString();
if (name)
{
- LanguageType sym_language = LanguageRuntime::GetLanguageForSymbolByName(target, name);
- if (m_language == eLanguageTypeC)
+ LanguageType sym_language = LanguageRuntime::GuessLanguageForSymbolByName(target, name);
+ if (Language::LanguageIsC(m_language) ||
+ Language::LanguageIsPascal(m_language))
{
// We don't currently have a way to say "This symbol name is C" so for now, C means
// not ObjC and not C++, etc...
@@ -293,6 +294,12 @@
}
else if (sym_language != m_language)
{
+ // Note: This code prevents us from being able to find symbols
+ // like 'printf' if the target language's option is set. It
+ // would be better to limit this filtering to only when the
+ // breakpoint's language option is set (and not the target's),
+ // but we can't know if m_language was set from the target or
+ // breakpoint option.
remove_it = true;
}
}
Index: include/lldb/Target/LanguageRuntime.h
===================================================================
--- include/lldb/Target/LanguageRuntime.h
+++ include/lldb/Target/LanguageRuntime.h
@@ -118,7 +118,7 @@
}
static lldb::LanguageType
- GetLanguageForSymbolByName (Target &target, const char *symbol_name);
+ GuessLanguageForSymbolByName (Target &target, const char *symbol_name);
Target&
GetTargetRef()
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
