[Lldb-commits] [PATCH] D50298: Add unit test for StringLexer
sgraenitz added a comment. > This is another one of classes we should probably get rid of (it looks like > all of this functionality is available in StringRef) Late +1. Usage in `FormattersContainer.h` should be easy to replace. Apart form that it could be an implementation detail of `AppleObjCTypeEncodingParser`. Repository: rL LLVM https://reviews.llvm.org/D50298 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function
This revision was automatically updated to reflect the committed changes. Closed by commit rL340543: Add libc++ data formatter for std::function (authored by adrian, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50864?vs=161997&id=162214#toc Repository: rL LLVM https://reviews.llvm.org/D50864 Files: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py === --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py @@ -40,13 +40,17 @@ substrs=['stopped', 'stop reason = breakpoint']) -f1 = self.get_variable('f1') -f2 = self.get_variable('f2') +self.expect("frame variable f1", +substrs=['f1 = Function = foo(int, int)']) -if self.TraceOn(): -print(f1) -if self.TraceOn(): -print(f2) +self.expect("frame variable f2", +substrs=['f2 = Lambda in File main.cpp at Line 27']) -self.assertTrue(f1.GetValueAsUnsigned(0) != 0, 'f1 has a valid value') -self.assertTrue(f2.GetValueAsUnsigned(0) != 0, 'f2 has a valid value') +self.expect("frame variable f3", +substrs=['f3 = Lambda in File main.cpp at Line 31']) + +self.expect("frame variable f4", +substrs=['f4 = Function in File main.cpp at Line 17']) + +self.expect("frame variable f5", +substrs=['f5 = Function = Bar::add_num(int) const']) Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp === --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp @@ -13,13 +13,28 @@ return x + y - 1; } -int main () +struct Bar { + int operator()() { + return 66 ; + } + int add_num(int i) const { return i + 3 ; } +} ; + +int main (int argc, char *argv[]) { int acc = 42; std::function f1 = foo; std::function f2 = [acc,f1] (int x) -> int { return x+f1(acc,x); }; -return f1(acc,acc) + f2(acc); // Set break point at this line. -} + auto f = [](int x, int y) { return x + y; }; + auto g = [](int x, int y) { return x * y; } ; + std::function f3 = argc %2 ? f : g ; + + Bar bar1 ; + std::function f4( bar1 ) ; + std::function f5 = &Bar::add_num; + + return f1(acc,acc) + f2(acc) + f3(acc+1,acc+2) + f4() + f5(bar1, 10); // Set break point at this line. +} Index: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h === --- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h +++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h @@ -36,6 +36,10 @@ const TypeSummaryOptions &options); // libc++ std::shared_ptr<> and std::weak_ptr<> +bool LibcxxFunctionSummaryProvider( +ValueObject &valobj, Stream &stream, +const TypeSummaryOptions &options); // libc++ std::function<> + SyntheticChildrenFrontEnd * LibcxxVectorBoolSyntheticFrontEndCreator(CXXSyntheticChildren *, lldb::ValueObjectSP); @@ -128,9 +132,6 @@ LibcxxInitializerListSyntheticFrontEndCreator(CXXSyntheticChildren *, lldb::ValueObjectSP); -SyntheticChildrenFrontEnd *LibcxxFunctionFrontEndCreator(CXXSyntheticChildren *, - lldb::ValueObjectSP); - SyntheticChildrenFrontEnd *LibcxxQueueFrontEndCreator(CXXSyntheticChildren *, lldb::ValueObjectSP); Index: lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp === --- lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp +++ lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp @@ -542,6 +542,11 @@ ConstString("^(std::__(ndk)?
[Lldb-commits] [lldb] r340543 - Add libc++ data formatter for std::function
Author: adrian Date: Thu Aug 23 10:02:39 2018 New Revision: 340543 URL: http://llvm.org/viewvc/llvm-project?rev=340543&view=rev Log: Add libc++ data formatter for std::function - Added LibcxxFunctionSummaryProvider - Removed LibcxxFunctionFrontEnd - Modified data formatter tests to test new summary functionality Patch by Shafik Yaghmour! Differential Revision: https://reviews.llvm.org/D50864 Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py?rev=340543&r1=340542&r2=340543&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py Thu Aug 23 10:02:39 2018 @@ -40,13 +40,17 @@ class LibCxxFunctionTestCase(TestBase): substrs=['stopped', 'stop reason = breakpoint']) -f1 = self.get_variable('f1') -f2 = self.get_variable('f2') +self.expect("frame variable f1", +substrs=['f1 = Function = foo(int, int)']) -if self.TraceOn(): -print(f1) -if self.TraceOn(): -print(f2) +self.expect("frame variable f2", +substrs=['f2 = Lambda in File main.cpp at Line 27']) -self.assertTrue(f1.GetValueAsUnsigned(0) != 0, 'f1 has a valid value') -self.assertTrue(f2.GetValueAsUnsigned(0) != 0, 'f2 has a valid value') +self.expect("frame variable f3", +substrs=['f3 = Lambda in File main.cpp at Line 31']) + +self.expect("frame variable f4", +substrs=['f4 = Function in File main.cpp at Line 17']) + +self.expect("frame variable f5", +substrs=['f5 = Function = Bar::add_num(int) const']) Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp?rev=340543&r1=340542&r2=340543&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp Thu Aug 23 10:02:39 2018 @@ -13,13 +13,28 @@ int foo(int x, int y) { return x + y - 1; } -int main () +struct Bar { + int operator()() { + return 66 ; + } + int add_num(int i) const { return i + 3 ; } +} ; + +int main (int argc, char *argv[]) { int acc = 42; std::function f1 = foo; std::function f2 = [acc,f1] (int x) -> int { return x+f1(acc,x); }; -return f1(acc,acc) + f2(acc); // Set break point at this line. -} + auto f = [](int x, int y) { return x + y; }; + auto g = [](int x, int y) { return x * y; } ; + std::function f3 = argc %2 ? f : g ; + + Bar bar1 ; + std::function f4( bar1 ) ; + std::function f5 = &Bar::add_num; + + return f1(acc,acc) + f2(acc) + f3(acc+1,acc+2) + f4() + f5(bar1, 10); // Set break point at this line. +} Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp?rev=340543&r1=340542&r2=340543&view=diff == --- lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp (original) +++ lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp Thu Aug 23 10:02:39 2018 @@ -542,6 +542,11 @@ static void LoadLibCxxFormatters(lldb::T ConstString("^(std::__(ndk)?1::)weak_ptr<.+>(( )?&)?$"), stl_synth_flags, true); + AddCXXSummary( + cpp_category_sp, lldb_private::formatters::LibcxxFunctionSummaryProvider, + "libc++ std::function summary provider", + ConstString("^std::__(ndk)?1::function<.+>$"), stl_summary
[Lldb-commits] [lldb] r340548 - lldbtest.py: Work around macOS SIP when testing ASANified builds.
Author: adrian Date: Thu Aug 23 10:19:08 2018 New Revision: 340548 URL: http://llvm.org/viewvc/llvm-project?rev=340548&view=rev Log: lldbtest.py: Work around macOS SIP when testing ASANified builds. Modified: lldb/trunk/lit/Suite/lldbtest.py Modified: lldb/trunk/lit/Suite/lldbtest.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lldbtest.py?rev=340548&r1=340547&r2=340548&view=diff == --- lldb/trunk/lit/Suite/lldbtest.py (original) +++ lldb/trunk/lit/Suite/lldbtest.py Thu Aug 23 10:19:08 2018 @@ -9,6 +9,14 @@ import lit.TestRunner import lit.util from lit.formats.base import TestFormat +def getBuildDir(cmd): +found = False +for arg in cmd: +if found: +return arg +if arg == '--build-dir': +found = True +return None class LLDBTest(TestFormat): def __init__(self, dotest_cmd): @@ -49,6 +57,18 @@ class LLDBTest(TestFormat): # python exe as the first parameter of the command. cmd = [sys.executable] + self.dotest_cmd + [testPath, '-p', testFile] +# The macOS system integrity protection (SIP) doesn't allow injecting +# libraries into system binaries, but this can be worked around by +# copying the binary into a different location. +if test.config.environment['DYLD_INSERT_LIBRARIES'] and \ +sys.executable.startswith('/System/'): +builddir = getBuildDir(cmd) +assert(builddir) +copied_python = os.path.join(builddir, 'copied-system-python') +import shutil +shutil.copy(sys.executable, os.path.join(builddir, copied_python)) +cmd[0] = copied_python + try: out, err, exitCode = lit.util.executeCommand( cmd, ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r340550 - XFAIL test for older versions of dsymutil
Author: adrian Date: Thu Aug 23 10:30:56 2018 New Revision: 340550 URL: http://llvm.org/viewvc/llvm-project?rev=340550&view=rev Log: XFAIL test for older versions of dsymutil Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py?rev=340550&r1=340549&r2=340550&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py Thu Aug 23 10:30:56 2018 @@ -9,6 +9,7 @@ class TestUnicodeSymbols(TestBase): mydir = TestBase.compute_mydir(__file__) +@expectedFailureAll(compiler="clang", compiler_version=['<', '7.0'], debug_info="dsym") def test_union_members(self): self.build() spec = lldb.SBModuleSpec() ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.
teemperor updated this revision to Diff 162227. teemperor added a comment. Herald added a subscriber: emaste. I added an flag for this to the evaluation options. I also set this flag for all utility calls I found. https://reviews.llvm.org/D50912 Files: include/lldb/Core/Debugger.h include/lldb/Target/Process.h include/lldb/Target/Target.h source/Core/Debugger.cpp source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp source/Plugins/Platform/POSIX/PlatformPOSIX.cpp source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp source/Target/Process.cpp Index: source/Target/Process.cpp === --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -1729,6 +1729,10 @@ m_mod_id.SetRunningUserExpression(on); } +void Process::SetRunningUtilityFunction(bool on) { + m_mod_id.SetRunningUtilityFunction(on); +} + addr_t Process::GetImageInfoAddress() { return LLDB_INVALID_ADDRESS; } const lldb::ABISP &Process::GetABI() { @@ -4685,7 +4689,12 @@ log->Printf("Process::%s pushing IO handler", __FUNCTION__); io_handler_sp->SetIsDone(false); -GetTarget().GetDebugger().PushIOHandler(io_handler_sp); +// If we evaluate an utility function, then we don't cancel the current +// IOHandler. Our IOHandler is non-interactive and shouldn't disturb the +// existing IOHandler that potentially provides the user interface (e.g. +// the IOHandler for Editline). +bool cancel_top_handler = m_mod_id.IsRunningUtilityFunction(); +GetTarget().GetDebugger().PushIOHandler(io_handler_sp, cancel_top_handler); return true; } return false; @@ -4875,6 +4884,11 @@ thread_plan_sp->SetIsMasterPlan(true); thread_plan_sp->SetOkayToDiscard(false); + // If we are running some utility expression for LLDB, we now have to mark + // this in the ProcesModID of this process. This RAII takes care of marking + // and reverting the mark it once we are done running the expression. + UtilityFunctionScope util_scope(options.IsForUtilityExpr() ? this : nullptr); + if (m_private_state.GetValue() != eStateStopped) { diagnostic_manager.PutString( eDiagnosticSeverityError, Index: source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp === --- source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp +++ source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp @@ -351,6 +351,7 @@ options.SetStopOthers(true); options.SetTimeout(std::chrono::milliseconds(500)); options.SetTryAllThreads(false); + options.SetIsForUtilityExpr(true); thread.CalculateExecutionContext(exe_ctx); if (!m_get_thread_item_info_impl_code) { Index: source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp === --- source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp +++ source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp @@ -354,6 +354,7 @@ options.SetStopOthers(true); options.SetTimeout(std::chrono::milliseconds(500)); options.SetTryAllThreads(false); + options.SetIsForUtilityExpr(true); thread.CalculateExecutionContext(exe_ctx); ExpressionResults func_call_ret; Index: source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp === --- source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp +++ source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp @@ -349,6 +349,7 @@ options.SetStopOthers(true); options.SetTimeout(std::chrono::milliseconds(500)); options.SetTryAllThreads(false); + options.SetIsForUtilityExpr(true); thread.CalculateExecutionContext(exe_ctx); if (get_pending_items_caller == NULL) { Index: source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp === --- source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp +++ source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp @@ -340,6 +340,7 @@ options.SetStopOthers(true); options.SetTimeout(std::chrono::milliseconds(500)); options.SetTryAllThreads(false); + options.SetIsForUtilityExpr(true); thread.CalculateExecutionContext(exe_ctx); if (!m_get_item_info_impl_code) { Index: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp === --- source/Plugins/Platform/POSIX/PlatformPOSIX.cpp +++ source/Plugins/Platform/POSIX/PlatformPOSIX.cpp @@ -1244,7 +1244,8 @@ options.SetTrapExce
[Lldb-commits] [lldb] r340557 - Fix check for dictionary entry
Author: adrian Date: Thu Aug 23 10:51:14 2018 New Revision: 340557 URL: http://llvm.org/viewvc/llvm-project?rev=340557&view=rev Log: Fix check for dictionary entry Modified: lldb/trunk/lit/Suite/lldbtest.py Modified: lldb/trunk/lit/Suite/lldbtest.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lldbtest.py?rev=340557&r1=340556&r2=340557&view=diff == --- lldb/trunk/lit/Suite/lldbtest.py (original) +++ lldb/trunk/lit/Suite/lldbtest.py Thu Aug 23 10:51:14 2018 @@ -60,7 +60,7 @@ class LLDBTest(TestFormat): # The macOS system integrity protection (SIP) doesn't allow injecting # libraries into system binaries, but this can be worked around by # copying the binary into a different location. -if test.config.environment['DYLD_INSERT_LIBRARIES'] and \ +if 'DYLD_INSERT_LIBRARIES' in test.config.environment and \ sys.executable.startswith('/System/'): builddir = getBuildDir(cmd) assert(builddir) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51175: Add support for descriptions with command completions.
teemperor created this revision. teemperor added a reviewer: LLDB. This patch adds a framework for adding descriptions to the command completions we provide. It also adds descriptions for completed top-level commands so that we can test this code. Completions are in general supposed to be displayed alongside the completion itself. The descriptions can be used to provide additional information about the completion to the user. Examples for descriptions are function signatures when completing function calls in the expression command or the binary name when providing completion for a symbol. There is still some boilerplate code from the old completion API left in LLDB (mostly because the respective APIs are reused for non-completion related purposes, so the CompletionRequest doesn't make sense to be used), so that's why I still had to change some function signatures. Also, as the old API only passes around a list of matches, and the descriptions are for these functions just another list, I had to add some code that essentially just ensures that both lists are always the same side (e.g. all the manual calls to `descriptions->AddString(X)` below a `matches->AddString(Y)` call). An example completion with descriptions looks like this: (lldb) pl Available completions: platform -- Commands to manage and create platforms. plugin -- Commands for managing LLDB plugins. Repository: rLLDB LLDB https://reviews.llvm.org/D51175 Files: include/lldb/API/SBCommandInterpreter.h include/lldb/Core/IOHandler.h include/lldb/Expression/REPL.h include/lldb/Host/Editline.h include/lldb/Interpreter/CommandInterpreter.h include/lldb/Interpreter/CommandObject.h include/lldb/Utility/CompletionRequest.h packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py packages/Python/lldbsuite/test/lldbtest.py scripts/interface/SBCommandInterpreter.i source/API/SBCommandInterpreter.cpp source/Commands/CommandObjectMultiword.cpp source/Core/IOHandler.cpp source/Expression/REPL.cpp source/Host/common/Editline.cpp source/Interpreter/CommandInterpreter.cpp source/Utility/CompletionRequest.cpp unittests/Utility/CompletionRequestTest.cpp Index: unittests/Utility/CompletionRequestTest.cpp === --- unittests/Utility/CompletionRequestTest.cpp +++ unittests/Utility/CompletionRequestTest.cpp @@ -20,9 +20,11 @@ const int match_start = 2345; const int match_max_return = 12345; StringList matches; + CompletionResult result; CompletionRequest request(command, cursor_pos, match_start, match_max_return, -matches); +result); + result.GetMatches(matches); EXPECT_STREQ(request.GetRawLine().str().c_str(), command.c_str()); EXPECT_EQ(request.GetRawCursorPos(), cursor_pos); @@ -41,63 +43,155 @@ const unsigned cursor_pos = 3; StringList matches; - CompletionRequest request(command, cursor_pos, 0, 0, matches); + CompletionResult result; + CompletionRequest request(command, cursor_pos, 0, 0, result); + result.GetMatches(matches); EXPECT_EQ(0U, request.GetNumberOfMatches()); // Add foo twice request.AddCompletion("foo"); + result.GetMatches(matches); + EXPECT_EQ(1U, request.GetNumberOfMatches()); EXPECT_EQ(1U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); request.AddCompletion("foo"); + result.GetMatches(matches); + EXPECT_EQ(1U, request.GetNumberOfMatches()); EXPECT_EQ(1U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); // Add bar twice request.AddCompletion("bar"); + result.GetMatches(matches); + EXPECT_EQ(2U, request.GetNumberOfMatches()); EXPECT_EQ(2U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); EXPECT_STREQ("bar", matches.GetStringAtIndex(1)); request.AddCompletion("bar"); + result.GetMatches(matches); + EXPECT_EQ(2U, request.GetNumberOfMatches()); EXPECT_EQ(2U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); EXPECT_STREQ("bar", matches.GetStringAtIndex(1)); // Add foo again. request.AddCompletion("foo"); + result.GetMatches(matches); + EXPECT_EQ(2U, request.GetNumberOfMatches()); EXPECT_EQ(2U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); EXPECT_STREQ("bar", matches.GetStringAtIndex(1)); // Add something with an existing prefix request.AddCompletion("foobar"); + result.GetMatches(matches); + EXPECT_EQ(3U, request.GetNumberOfMatches()); EXPECT_EQ(3U, matches.GetSize()); EXPECT_STREQ("foo", matches.GetStringAtIndex(0)); EXPECT_STREQ("bar", matches.GetStringAtIndex(1)); EXPECT_STREQ("foobar", matches.GetStringAtIndex(2)); } +TEST(CompletionRequest, DuplicateFilteringWithComments) { + std::string command = "a bad c"; + const unsigned cursor_pos = 3; + StringList matches, descr
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
lemo created this revision. lemo added reviewers: clayborg, zturner. lemo added a project: LLDB. Herald added a subscriber: abidh. 1. The dynamic loaders should not be needed for loading minidumps and they may create problems (ex. the macOS loader resets the list of loaded modules) 2. In general, the extra plugins can do extraneous work which hurts performance (ex. trying to set up implicit breakpoints, which in turn will trigger extra symbol loading) Repository: rLLDB LLDB https://reviews.llvm.org/D51176 Files: source/Plugins/Process/minidump/ProcessMinidump.cpp source/Plugins/Process/minidump/ProcessMinidump.h Index: source/Plugins/Process/minidump/ProcessMinidump.h === --- source/Plugins/Process/minidump/ProcessMinidump.h +++ source/Plugins/Process/minidump/ProcessMinidump.h @@ -55,7 +55,7 @@ Status DoLoadCore() override; - DynamicLoader *GetDynamicLoader() override; + DynamicLoader *GetDynamicLoader() override { return nullptr; } ConstString GetPluginName() override; @@ -102,6 +102,8 @@ void ReadModuleList(); + JITLoaderList &GetJITLoaders() override; + private: FileSpec m_core_file; llvm::ArrayRef m_thread_list; Index: source/Plugins/Process/minidump/ProcessMinidump.cpp === --- source/Plugins/Process/minidump/ProcessMinidump.cpp +++ source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -16,7 +16,7 @@ #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/PluginManager.h" #include "lldb/Core/Section.h" -#include "lldb/Target/DynamicLoader.h" +#include "lldb/Target/JITLoaderList.h" #include "lldb/Target/MemoryRegionInfo.h" #include "lldb/Target/SectionLoadList.h" #include "lldb/Target/Target.h" @@ -201,12 +201,6 @@ return error; } -DynamicLoader *ProcessMinidump::GetDynamicLoader() { - if (m_dyld_ap.get() == nullptr) -m_dyld_ap.reset(DynamicLoader::FindPlugin(this, nullptr)); - return m_dyld_ap.get(); -} - ConstString ProcessMinidump::GetPluginName() { return GetPluginNameStatic(); } uint32_t ProcessMinidump::GetPluginVersion() { return 1; } @@ -401,3 +395,10 @@ } return true; } + +JITLoaderList &ProcessMinidump::GetJITLoaders() { + if (!m_jit_loaders_ap) { +m_jit_loaders_ap = llvm::make_unique(); + } + return *m_jit_loaders_ap; +} Index: source/Plugins/Process/minidump/ProcessMinidump.h === --- source/Plugins/Process/minidump/ProcessMinidump.h +++ source/Plugins/Process/minidump/ProcessMinidump.h @@ -55,7 +55,7 @@ Status DoLoadCore() override; - DynamicLoader *GetDynamicLoader() override; + DynamicLoader *GetDynamicLoader() override { return nullptr; } ConstString GetPluginName() override; @@ -102,6 +102,8 @@ void ReadModuleList(); + JITLoaderList &GetJITLoaders() override; + private: FileSpec m_core_file; llvm::ArrayRef m_thread_list; Index: source/Plugins/Process/minidump/ProcessMinidump.cpp === --- source/Plugins/Process/minidump/ProcessMinidump.cpp +++ source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -16,7 +16,7 @@ #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/PluginManager.h" #include "lldb/Core/Section.h" -#include "lldb/Target/DynamicLoader.h" +#include "lldb/Target/JITLoaderList.h" #include "lldb/Target/MemoryRegionInfo.h" #include "lldb/Target/SectionLoadList.h" #include "lldb/Target/Target.h" @@ -201,12 +201,6 @@ return error; } -DynamicLoader *ProcessMinidump::GetDynamicLoader() { - if (m_dyld_ap.get() == nullptr) -m_dyld_ap.reset(DynamicLoader::FindPlugin(this, nullptr)); - return m_dyld_ap.get(); -} - ConstString ProcessMinidump::GetPluginName() { return GetPluginNameStatic(); } uint32_t ProcessMinidump::GetPluginVersion() { return 1; } @@ -401,3 +395,10 @@ } return true; } + +JITLoaderList &ProcessMinidump::GetJITLoaders() { + if (!m_jit_loaders_ap) { +m_jit_loaders_ap = llvm::make_unique(); + } + return *m_jit_loaders_ap; +} ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.
clayborg added inline comments. Comment at: include/lldb/Target/Process.h:479 +else + m_running_utility_function--; + } Might want this to be: ``` else if (m_running_utility_function > 0) --m_running_utility_function; ``` Just to make sure we don't ever underflow Comment at: source/Target/Process.cpp:4696-4697 +// the IOHandler for Editline). +bool cancel_top_handler = m_mod_id.IsRunningUtilityFunction(); +GetTarget().GetDebugger().PushIOHandler(io_handler_sp, cancel_top_handler); return true; Do we still need this extra bool to PushIOHandler? Can't we do the code snippet I suggested above? https://reviews.llvm.org/D50912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r340560 - [cmake] Add option to skip building lldb-server
Author: xiaobai Date: Thu Aug 23 11:05:45 2018 New Revision: 340560 URL: http://llvm.org/viewvc/llvm-project?rev=340560&view=rev Log: [cmake] Add option to skip building lldb-server Summary: There is currently a way to skip the debugserver build. See how the CMake variables SKIP_DEBUGSERVER and LLDB_CODESIGN_IDENTITY are used if you're interested in that. This allows us to skip building lldb-server as well. There is another debug server called ds2 that can be used with LLDB. If you choose to use ds2, this flag is very useful because it can cut down the build time of LLDB. Differential Revision: https://reviews.llvm.org/D49282 Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake lldb/trunk/tools/CMakeLists.txt lldb/trunk/unittests/tools/CMakeLists.txt Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=340560&r1=340559&r2=340560&view=diff == --- lldb/trunk/cmake/modules/LLDBConfig.cmake (original) +++ lldb/trunk/cmake/modules/LLDBConfig.cmake Thu Aug 23 11:05:45 2018 @@ -357,6 +357,8 @@ endif() list(APPEND system_libs ${CMAKE_DL_LIBS}) +SET(SKIP_LLDB_SERVER_BUILD OFF CACHE BOOL "Skip building lldb-server") + # Figure out if lldb could use lldb-server. If so, then we'll # ensure we build lldb-server when an lldb target is being built. if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD") Modified: lldb/trunk/tools/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/CMakeLists.txt?rev=340560&r1=340559&r2=340560&view=diff == --- lldb/trunk/tools/CMakeLists.txt (original) +++ lldb/trunk/tools/CMakeLists.txt Thu Aug 23 11:05:45 2018 @@ -6,7 +6,7 @@ add_subdirectory(argdumper) add_subdirectory(driver) add_subdirectory(lldb-mi) add_subdirectory(lldb-vscode) -if (LLDB_CAN_USE_LLDB_SERVER) +if (LLDB_CAN_USE_LLDB_SERVER AND NOT SKIP_LLDB_SERVER_BUILD) add_subdirectory(lldb-server) endif() add_subdirectory(intel-features) Modified: lldb/trunk/unittests/tools/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/CMakeLists.txt?rev=340560&r1=340559&r2=340560&view=diff == --- lldb/trunk/unittests/tools/CMakeLists.txt (original) +++ lldb/trunk/unittests/tools/CMakeLists.txt Thu Aug 23 11:05:45 2018 @@ -1,5 +1,5 @@ if(CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|Linux|NetBSD") - if (CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_DEBUGSERVER) + if ((CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_DEBUGSERVER) OR (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_LLDB_SERVER_BUILD)) # These tests are meant to test lldb-server/debugserver in isolation, and # don't provide any value if run against a server copied from somewhere. else() ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49282: [cmake] Add option to skip building lldb-server
This revision was automatically updated to reflect the committed changes. Closed by commit rL340560: [cmake] Add option to skip building lldb-server (authored by xiaobai, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D49282?vs=155459&id=162236#toc Repository: rL LLVM https://reviews.llvm.org/D49282 Files: lldb/trunk/cmake/modules/LLDBConfig.cmake lldb/trunk/tools/CMakeLists.txt lldb/trunk/unittests/tools/CMakeLists.txt Index: lldb/trunk/unittests/tools/CMakeLists.txt === --- lldb/trunk/unittests/tools/CMakeLists.txt +++ lldb/trunk/unittests/tools/CMakeLists.txt @@ -1,5 +1,5 @@ if(CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|Linux|NetBSD") - if (CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_DEBUGSERVER) + if ((CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_DEBUGSERVER) OR (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_LLDB_SERVER_BUILD)) # These tests are meant to test lldb-server/debugserver in isolation, and # don't provide any value if run against a server copied from somewhere. else() Index: lldb/trunk/tools/CMakeLists.txt === --- lldb/trunk/tools/CMakeLists.txt +++ lldb/trunk/tools/CMakeLists.txt @@ -6,7 +6,7 @@ add_subdirectory(driver) add_subdirectory(lldb-mi) add_subdirectory(lldb-vscode) -if (LLDB_CAN_USE_LLDB_SERVER) +if (LLDB_CAN_USE_LLDB_SERVER AND NOT SKIP_LLDB_SERVER_BUILD) add_subdirectory(lldb-server) endif() add_subdirectory(intel-features) Index: lldb/trunk/cmake/modules/LLDBConfig.cmake === --- lldb/trunk/cmake/modules/LLDBConfig.cmake +++ lldb/trunk/cmake/modules/LLDBConfig.cmake @@ -357,6 +357,8 @@ list(APPEND system_libs ${CMAKE_DL_LIBS}) +SET(SKIP_LLDB_SERVER_BUILD OFF CACHE BOOL "Skip building lldb-server") + # Figure out if lldb could use lldb-server. If so, then we'll # ensure we build lldb-server when an lldb target is being built. if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD") Index: lldb/trunk/unittests/tools/CMakeLists.txt === --- lldb/trunk/unittests/tools/CMakeLists.txt +++ lldb/trunk/unittests/tools/CMakeLists.txt @@ -1,5 +1,5 @@ if(CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|Linux|NetBSD") - if (CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_DEBUGSERVER) + if ((CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_DEBUGSERVER) OR (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin" AND SKIP_LLDB_SERVER_BUILD)) # These tests are meant to test lldb-server/debugserver in isolation, and # don't provide any value if run against a server copied from somewhere. else() Index: lldb/trunk/tools/CMakeLists.txt === --- lldb/trunk/tools/CMakeLists.txt +++ lldb/trunk/tools/CMakeLists.txt @@ -6,7 +6,7 @@ add_subdirectory(driver) add_subdirectory(lldb-mi) add_subdirectory(lldb-vscode) -if (LLDB_CAN_USE_LLDB_SERVER) +if (LLDB_CAN_USE_LLDB_SERVER AND NOT SKIP_LLDB_SERVER_BUILD) add_subdirectory(lldb-server) endif() add_subdirectory(intel-features) Index: lldb/trunk/cmake/modules/LLDBConfig.cmake === --- lldb/trunk/cmake/modules/LLDBConfig.cmake +++ lldb/trunk/cmake/modules/LLDBConfig.cmake @@ -357,6 +357,8 @@ list(APPEND system_libs ${CMAKE_DL_LIBS}) +SET(SKIP_LLDB_SERVER_BUILD OFF CACHE BOOL "Skip building lldb-server") + # Figure out if lldb could use lldb-server. If so, then we'll # ensure we build lldb-server when an lldb target is being built. if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD") ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Dynamic loaders are needed for loading breakpad minidumps that are for MacOSX and iOS processes. They should also be needed for loading any minidumps that have stack traces. Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
clayborg added a comment. No dynamic loader plug-ins should be affecting the module list during the plug-in loading/selection, if that is happening, that is a bug and it should be fixed. Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
lemo added a comment. > Dynamic loaders are needed for loading breakpad minidumps that are for MacOSX > and iOS processes. They should also be needed for loading any minidumps that > have stack traces. Thanks. I just validated the change against a macOS minidump and everything works fine in my local test. What parts of the dynamic loader is relevant to minidump loading? (ie. anything specific I should pay attention to?) > No dynamic loader plug-ins should be affecting the module list during the > plug-in loading/selection, if that is happening, that is a bug and it should > be fixed. I agree. Although that's outside the scope of this change if I'm right in that we can avoid the dynamic loader plugins completely. Here's an example where the DynamicLoaderDarwin is misbehaving (note the DynamicLoaderDarwin::PrivateInitialize() call to Target::ClearAllLoadedSections()) lldb_private::SectionLoadList::Clear(lldb_private::SectionLoadList * this) (lldb/source/Target/SectionLoadList.cpp:47) lldb_private::SectionLoadList::~SectionLoadList(lldb_private::SectionLoadList * this) (lldb/include/lldb/Target/SectionLoadList.h:39) std::_Sp_counted_ptr::_M_dispose(std::_Sp_counted_ptr * this) (libstdcxx/include/bits/shared_ptr_base.h:371) std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release(std::_Sp_counted_base<__gnu_cxx::_S_atomic> * this) (libstdcxx/include/bits/shared_ptr_base.h:149) std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count(std::__shared_count<__gnu_cxx::_S_atomic> * this) (libstdcxx/include/bits/shared_ptr_base.h:664) std::__shared_ptr::~__shared_ptr(std::__shared_ptr * this) (libstdcxx/include/bits/shared_ptr_base.h:912) std::shared_ptr::~shared_ptr(std::shared_ptr * this) (libstdcxx/include/bits/shared_ptr_base.h:342) std::pair >::~pair(std::pair > * this) (libstdcxx/include/bits/stl_pair.h:96) __gnu_cxx::new_allocator > > >::destroy > >(__gnu_cxx::new_allocator > > > * this, std::pair > * __p) (libstdcxx/include/ext/new_allocator.h:165) std::allocator_traits > > > >::destroy > >(std::allocator_traits > > > >::allocator_type & __a, std::pair > * __p) (libstdcxx/include/bits/alloc_traits.h:539) std::_Rb_tree >, std::_Select1st > >, std::map, std::less, std::allocator > > >::_ConstCompare >, std::allocator > > >::_M_destroy_node(std::_Rb_tree >, std::_Select1st > >, std::map, std::less, std::allocator > > >::_ConstCompare >, std::allocator > > > * this, std::_Rb_tree >, std::_Select1st > >, std::map, std::less, std::allocator > > >::_ConstCompare >, std::allocator > > >::_Link_type __p) (libstdcxx/include/bits/stl_tree.h:435) std::_Rb_tree >, std::_Select1st > >, std::map, std::less, std::allocator > > >::_ConstCompare >, std::allocator > > >::_M_erase(std::_Rb_tree >, std::_Select1st > >, std::map, std::less, std::allocator > > >::_ConstCompare >, std::allocator > > > * this, std::_Rb_tree >, std::_Select1st > >, std::map, std::less, std::allocator > > >::_ConstCompare >, std::allocator > > >::_Link_type __x) (libstdcxx/include/bits/stl_tree.h:1283) std::_Rb_tree >, std::_Select1st > >, std::map, std::less, std::allocator > > >::_ConstCompare >, std::allocator > > >::clear(std::_Rb_tree >, std::_Select1st > >, std::map, std::less, std::allocator > > >::_ConstCompare >, std::allocator > > > * this) (libstdcxx/include/bits/stl_tree.h:944) std::map, std::less, std::allocator > > >::clear(std::map, std::less, std::allocator > > > * this) (libstdcxx/include/bits/stl_map.h:862) lldb_private::SectionLoadHistory::Clear(lldb_private::SectionLoadHistory * this) (lldb/source/Target/SectionLoadHistory.cpp:29) lldb_private::Target::ClearAllLoadedSections(lldb_private::Target * this) (lldb/source/Target/Target.cpp:2952) lldb_private::DynamicLoaderDarwin::PrivateInitialize(lldb_private::DynamicLoaderDarwin * this, lldb_private::Process * process) (lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp:768) lldb_private::DynamicLoaderDarwin::DidAttach(lldb_private::DynamicLoaderDarwin * this) (lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp:73) lldb_private::Process::LoadCore(lldb_private::Process * this) (lldb/source/Target/Process.cpp:2847) lldb::SBTarget::LoadCore(lldb::SBTarget * this, const char * core_file, lldb::SBError & error) (lldb/source/API/SBTarget.cpp:218) Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
jingham added a comment. That's not changing the module list, that's changing the loaded section information. It is the dynamic loader's job to figure out what got loaded where, plus your stack trace show this happening after we've selected a plugin, not in the process of negotiating for the plugin. Clearing the section load list before setting to work seems like an appropriate thing for a selected plugin to do. Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.
teemperor updated this revision to Diff 162238. teemperor added a comment. - Add an assert against underflow checking. - Fixed a (serious) typo that broke the test. https://reviews.llvm.org/D50912 Files: include/lldb/Core/Debugger.h include/lldb/Target/Process.h include/lldb/Target/Target.h source/Core/Debugger.cpp source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp source/Plugins/Platform/POSIX/PlatformPOSIX.cpp source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp source/Target/Process.cpp Index: source/Target/Process.cpp === --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -1729,6 +1729,10 @@ m_mod_id.SetRunningUserExpression(on); } +void Process::SetRunningUtilityFunction(bool on) { + m_mod_id.SetRunningUtilityFunction(on); +} + addr_t Process::GetImageInfoAddress() { return LLDB_INVALID_ADDRESS; } const lldb::ABISP &Process::GetABI() { @@ -4685,7 +4689,12 @@ log->Printf("Process::%s pushing IO handler", __FUNCTION__); io_handler_sp->SetIsDone(false); -GetTarget().GetDebugger().PushIOHandler(io_handler_sp); +// If we evaluate an utility function, then we don't cancel the current +// IOHandler. Our IOHandler is non-interactive and shouldn't disturb the +// existing IOHandler that potentially provides the user interface (e.g. +// the IOHandler for Editline). +bool cancel_top_handler = !m_mod_id.IsRunningUtilityFunction(); +GetTarget().GetDebugger().PushIOHandler(io_handler_sp, cancel_top_handler); return true; } return false; @@ -4875,6 +4884,11 @@ thread_plan_sp->SetIsMasterPlan(true); thread_plan_sp->SetOkayToDiscard(false); + // If we are running some utility expression for LLDB, we now have to mark + // this in the ProcesModID of this process. This RAII takes care of marking + // and reverting the mark it once we are done running the expression. + UtilityFunctionScope util_scope(options.IsForUtilityExpr() ? this : nullptr); + if (m_private_state.GetValue() != eStateStopped) { diagnostic_manager.PutString( eDiagnosticSeverityError, Index: source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp === --- source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp +++ source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp @@ -351,6 +351,7 @@ options.SetStopOthers(true); options.SetTimeout(std::chrono::milliseconds(500)); options.SetTryAllThreads(false); + options.SetIsForUtilityExpr(true); thread.CalculateExecutionContext(exe_ctx); if (!m_get_thread_item_info_impl_code) { Index: source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp === --- source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp +++ source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp @@ -354,6 +354,7 @@ options.SetStopOthers(true); options.SetTimeout(std::chrono::milliseconds(500)); options.SetTryAllThreads(false); + options.SetIsForUtilityExpr(true); thread.CalculateExecutionContext(exe_ctx); ExpressionResults func_call_ret; Index: source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp === --- source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp +++ source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp @@ -349,6 +349,7 @@ options.SetStopOthers(true); options.SetTimeout(std::chrono::milliseconds(500)); options.SetTryAllThreads(false); + options.SetIsForUtilityExpr(true); thread.CalculateExecutionContext(exe_ctx); if (get_pending_items_caller == NULL) { Index: source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp === --- source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp +++ source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp @@ -340,6 +340,7 @@ options.SetStopOthers(true); options.SetTimeout(std::chrono::milliseconds(500)); options.SetTryAllThreads(false); + options.SetIsForUtilityExpr(true); thread.CalculateExecutionContext(exe_ctx); if (!m_get_item_info_impl_code) { Index: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp === --- source/Plugins/Platform/POSIX/PlatformPOSIX.cpp +++ source/Plugins/Platform/POSIX/PlatformPOSIX.cpp @@ -1244,7 +1244,8 @@ options.SetTrapExceptions(false); // dlopen can't throw exceptions,
[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.
teemperor added inline comments. Comment at: source/Target/Process.cpp:4696-4697 +// the IOHandler for Editline). +bool cancel_top_handler = m_mod_id.IsRunningUtilityFunction(); +GetTarget().GetDebugger().PushIOHandler(io_handler_sp, cancel_top_handler); return true; clayborg wrote: > Do we still need this extra bool to PushIOHandler? Can't we do the code > snippet I suggested above? Do we know for sure that omitting this IOHandler push doesn't break something? From the comments above it seems that this is actually changing LLDB's behavior. And we can't easily revert this commit once I stack the expression completion + follow-ups on top of it. I'm fine with doing this change as a direct follow-up commit that won't have any dependencies attached. https://reviews.llvm.org/D50912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
clayborg added a comment. Dynamic loaders will fill out the section load list in the target that saids "__TEXT" from "/tmp/a.out" was loaded at address 0x120200. So yes they are needed. If the process minidump is manually setting the section load list itself, then there really is no need for a dynamic loader and this fix might work. Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.
clayborg added a comment. Sounds good. I am ok with this as long as Jim Ingham is. https://reviews.llvm.org/D50912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
lemo added a comment. > That's not changing the module list, that's changing the loaded section > information. It is the dynamic loader's job to figure out what got loaded > where, plus your stack trace show this happening after we've selected a > plugin, not in the process of negotiating for the plugin. Clearing the > section load list before setting to work seems like an appropriate thing for > a selected plugin to do. Correct, sorry - I meant the loaded sections list. This reset is problematic for minidumps since we build the loaded sections list while loading the minidump. Is the dynamic loader relevant to loading the minidumps in any way? I'm open to any other ides on how to avoid this conflict between the minidump loading and the dynamic loader, although I'd strongly prefer to minimize the code path to the strictly minimum required. For example this dynamic loader issue is particularly unfortunate since it only happens for macOS minidumps. So one immediate consequence at least is that it complicates the test matrix (BTW, we should probably have at least one macOS & iOS minidumps for LLDB tests, what do you think?) Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
lemo added a comment. In https://reviews.llvm.org/D51176#1211307, @clayborg wrote: > Dynamic loaders will fill out the section load list in the target that saids > "__TEXT" from "/tmp/a.out" was loaded at address 0x120200. So yes they > are needed. If the process minidump is manually setting the section load list > itself, then there really is no need for a dynamic loader and this fix might > work. That was my original thought: for minidumps we don't really have "load addresses", the memory map is recorded in the minidump and that's what we use (see PlaceholderModule::CreateImageSection) Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
clayborg added a comment. So this might actually work. Take a look around and see what else the dynamic loader is used for and make sure that they won't be needed for anything else. If not, this fix should work, but we should document it. Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
lemo added a comment. In https://reviews.llvm.org/D51176#1211358, @clayborg wrote: > So this might actually work. Take a look around and see what else the dynamic > loader is used for and make sure that they won't be needed for anything else. > If not, this fix should work, but we should document it. I took another look and I don't see anything where the dynamic loader is required in the context of loading minidumps. Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. I am fine with this change then. Probably best to get the OK from Zach as well. Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
jingham added a comment. The other option would be to move the code that populates the section load list into the mini dump dynamic loader. That has the benefit of keeping this consistent with the other process plugins, but OTOH is just moving code around... Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
clayborg added a comment. In https://reviews.llvm.org/D51176#1211433, @jingham wrote: > The other option would be to move the code that populates the section load > list into the mini dump dynamic loader. That has the benefit of keeping this > consistent with the other process plugins, but OTOH is just moving code > around... Yes the dynamic loader plug-ins aren't hard to write and the code already exists in the ProcessMinidump. Then you would request the plug-in by name instead of passing a nullptr as the name in ProcessMinidump::GetDynamicLoader(). Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50481: Fix broken builtin functions in the expression command
teemperor updated this revision to Diff 162255. teemperor added a comment. This revision is now accepted and ready to land. - Removed unrelated comments in the test. - No longer using self.expect. - Small refactoring in the parsing code. https://reviews.llvm.org/D50481 Files: packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h === --- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h +++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h @@ -149,8 +149,6 @@ m_file_manager; ///< The Clang file manager object used by the compiler std::unique_ptr m_compiler; ///< The Clang compiler used to parse expressions into IR - std::unique_ptr - m_builtin_context; ///< Context for Clang built-ins std::unique_ptr m_selector_table; ///< Selector table for Objective-C methods std::unique_ptr Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp === --- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -508,12 +508,21 @@ // 8. Most of this we get from the CompilerInstance, but we also want to give // the context an ExternalASTSource. m_selector_table.reset(new SelectorTable()); - m_builtin_context.reset(new Builtin::Context()); + + // We enable all builtin functions beside the builtins from libc/libm (e.g. + // 'fopen'). Those libc functions are already correctly handled by LLDB, and + // additionally enabling them as expandable builtins is breaking Clang. + m_compiler->getLangOpts().NoBuiltin = true; + + auto &PP = m_compiler->getPreprocessor(); + auto &builtin_context = PP.getBuiltinInfo(); + builtin_context.initializeBuiltins(PP.getIdentifierTable(), + m_compiler->getLangOpts()); std::unique_ptr ast_context( new ASTContext(m_compiler->getLangOpts(), m_compiler->getSourceManager(), m_compiler->getPreprocessor().getIdentifierTable(), - *m_selector_table.get(), *m_builtin_context.get())); + *m_selector_table.get(), builtin_context)); ast_context->InitBuiltinTypes(m_compiler->getTarget()); Index: packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py === --- /dev/null +++ packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py @@ -0,0 +1,53 @@ +""" +Tests calling builtin functions using expression evaluation. +""" + +from __future__ import print_function + + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class ExprCommandCallBuiltinFunction(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +# Builtins are expanded by Clang, so debug info shouldn't matter. +NO_DEBUG_INFO_TESTCASE = True + +def setUp(self): +TestBase.setUp(self) +# Find the line number to break for main.c. +self.line = line_number( +'main.cpp', +'// Please test these expressions while stopped at this line:') + +def test(self): +self.build() + +# Set breakpoint in main and run exe +self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET) +lldbutil.run_break_set_by_file_and_line( +self, "main.cpp", self.line, num_expected_locations=-1, loc_exact=True) + +self.runCmd("run", RUN_SUCCEEDED) + +interp = self.dbg.GetCommandInterpreter() +result = lldb.SBCommandReturnObject() + +# Test different builtin functions. + +interp.HandleCommand("expr __builtin_isinf(0.0f)", result) +self.assertEqual(result.GetOutput(), "(int) $0 = 0\n") + +interp.HandleCommand("expr __builtin_isnormal(0.0f)", result) +self.assertEqual(result.GetOutput(), "(int) $1 = 0\n") + +interp.HandleCommand("expr __builtin_constant_p(1)", result) +self.assertEqual(result.GetOutput(), "(int) $2 = 1\n") + +interp.HandleCommand("expr __builtin_abs(-14)", result) +self.assertEqual(result.GetOutput(), "(int) $3 = 14\n") Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h === --- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h +++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h @@ -149,8 +149,6 @@ m_file_manager; ///< The Clang file manager object used by the compiler std::unique
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
lemo added subscribers: bgianfo, clayborg, zturner. lemo added a comment. It's an interesting idea, thanks! I don't object moving code around if there's a strong case for it, but I'd like to keep the fix small and simple for now, but it's worth considering if the current minidump loading path will need more flexibility. Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
It's an interesting idea, thanks! I don't object moving code around if there's a strong case for it, but I'd like to keep the fix small and simple for now, but it's worth considering if the current minidump loading path will need more flexibility. On Thu, Aug 23, 2018 at 1:27 PM, Greg Clayton via Phabricator < revi...@reviews.llvm.org> wrote: > clayborg added a comment. > > In https://reviews.llvm.org/D51176#1211433, @jingham wrote: > > > The other option would be to move the code that populates the section > load list into the mini dump dynamic loader. That has the benefit of > keeping this consistent with the other process plugins, but OTOH is just > moving code around... > > > Yes the dynamic loader plug-ins aren't hard to write and the code already > exists in the ProcessMinidump. Then you would request the plug-in by name > instead of passing a nullptr as the name in ProcessMinidump:: > GetDynamicLoader(). > > > Repository: > rLLDB LLDB > > https://reviews.llvm.org/D51176 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r340571 - Fix broken builtin functions in the expression command
Author: teemperor Date: Thu Aug 23 13:40:45 2018 New Revision: 340571 URL: http://llvm.org/viewvc/llvm-project?rev=340571&view=rev Log: Fix broken builtin functions in the expression command Summary: Calling any non-libc builtin function in the expression command currently just causes Clang to state that the function is not known. The reason for this is that we actually never initialize the list of builtin functions in the Builtin::Context. This patch just calls the initializer for the builtins in the preprocessor. Also adds some tests for the new builtins. It also gets rid of the extra list of builtins in the ClangExpressionParser, as we can just reuse the existing list in the Preprocessor for the ASTContext. Having just one list of builtins around is also closer to the standard Clang behavior. Reviewers: #lldb, vsk Reviewed By: vsk Subscribers: sgraenitz, clayborg, vsk, lldb-commits Differential Revision: https://reviews.llvm.org/D50481 Added: lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h Added: lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py?rev=340571&view=auto == --- lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py (added) +++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py Thu Aug 23 13:40:45 2018 @@ -0,0 +1,53 @@ +""" +Tests calling builtin functions using expression evaluation. +""" + +from __future__ import print_function + + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class ExprCommandCallBuiltinFunction(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +# Builtins are expanded by Clang, so debug info shouldn't matter. +NO_DEBUG_INFO_TESTCASE = True + +def setUp(self): +TestBase.setUp(self) +# Find the line number to break for main.c. +self.line = line_number( +'main.cpp', +'// Please test these expressions while stopped at this line:') + +def test(self): +self.build() + +# Set breakpoint in main and run exe +self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET) +lldbutil.run_break_set_by_file_and_line( +self, "main.cpp", self.line, num_expected_locations=-1, loc_exact=True) + +self.runCmd("run", RUN_SUCCEEDED) + +interp = self.dbg.GetCommandInterpreter() +result = lldb.SBCommandReturnObject() + +# Test different builtin functions. + +interp.HandleCommand("expr __builtin_isinf(0.0f)", result) +self.assertEqual(result.GetOutput(), "(int) $0 = 0\n") + +interp.HandleCommand("expr __builtin_isnormal(0.0f)", result) +self.assertEqual(result.GetOutput(), "(int) $1 = 0\n") + +interp.HandleCommand("expr __builtin_constant_p(1)", result) +self.assertEqual(result.GetOutput(), "(int) $2 = 1\n") + +interp.HandleCommand("expr __builtin_abs(-14)", result) +self.assertEqual(result.GetOutput(), "(int) $3 = 14\n") Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=340571&r1=340570&r2=340571&view=diff == --- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (original) +++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp Thu Aug 23 13:40:45 2018 @@ -508,12 +508,21 @@ ClangExpressionParser::ClangExpressionPa // 8. Most of this we get from the CompilerInstance, but we also want to give // the context an ExternalASTSource. m_selector_table.reset(new SelectorTable()); - m_builtin_context.reset(new Builtin::Context()); + + // We enable all builtin functions beside the builtins from libc/libm (e.g. + // 'fopen'). Those libc functions are already correctly handled by LLDB, and + // additionally enabling them as expandable builtins is breaking Clang. + m_compiler->getLangOpts().NoBuiltin = true; + + auto &PP = m_compiler->getPreprocessor(); + auto &builtin_context = PP.getBuiltinInfo(); + builtin_context.initializeBuiltins(PP.getIdentifierTable(), + m_compiler->getLangOpts()); std::unique_ptr ast_context( new ASTContext(m_compiler->getLa
[Lldb-commits] [PATCH] D50481: Fix broken builtin functions in the expression command
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB340571: Fix broken builtin functions in the expression command (authored by teemperor, committed by ). Repository: rLLDB LLDB https://reviews.llvm.org/D50481 Files: packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h Index: packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py === --- packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py +++ packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py @@ -0,0 +1,53 @@ +""" +Tests calling builtin functions using expression evaluation. +""" + +from __future__ import print_function + + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class ExprCommandCallBuiltinFunction(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +# Builtins are expanded by Clang, so debug info shouldn't matter. +NO_DEBUG_INFO_TESTCASE = True + +def setUp(self): +TestBase.setUp(self) +# Find the line number to break for main.c. +self.line = line_number( +'main.cpp', +'// Please test these expressions while stopped at this line:') + +def test(self): +self.build() + +# Set breakpoint in main and run exe +self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET) +lldbutil.run_break_set_by_file_and_line( +self, "main.cpp", self.line, num_expected_locations=-1, loc_exact=True) + +self.runCmd("run", RUN_SUCCEEDED) + +interp = self.dbg.GetCommandInterpreter() +result = lldb.SBCommandReturnObject() + +# Test different builtin functions. + +interp.HandleCommand("expr __builtin_isinf(0.0f)", result) +self.assertEqual(result.GetOutput(), "(int) $0 = 0\n") + +interp.HandleCommand("expr __builtin_isnormal(0.0f)", result) +self.assertEqual(result.GetOutput(), "(int) $1 = 0\n") + +interp.HandleCommand("expr __builtin_constant_p(1)", result) +self.assertEqual(result.GetOutput(), "(int) $2 = 1\n") + +interp.HandleCommand("expr __builtin_abs(-14)", result) +self.assertEqual(result.GetOutput(), "(int) $3 = 14\n") Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h === --- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h +++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h @@ -149,8 +149,6 @@ m_file_manager; ///< The Clang file manager object used by the compiler std::unique_ptr m_compiler; ///< The Clang compiler used to parse expressions into IR - std::unique_ptr - m_builtin_context; ///< Context for Clang built-ins std::unique_ptr m_selector_table; ///< Selector table for Objective-C methods std::unique_ptr Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp === --- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -508,12 +508,21 @@ // 8. Most of this we get from the CompilerInstance, but we also want to give // the context an ExternalASTSource. m_selector_table.reset(new SelectorTable()); - m_builtin_context.reset(new Builtin::Context()); + + // We enable all builtin functions beside the builtins from libc/libm (e.g. + // 'fopen'). Those libc functions are already correctly handled by LLDB, and + // additionally enabling them as expandable builtins is breaking Clang. + m_compiler->getLangOpts().NoBuiltin = true; + + auto &PP = m_compiler->getPreprocessor(); + auto &builtin_context = PP.getBuiltinInfo(); + builtin_context.initializeBuiltins(PP.getIdentifierTable(), + m_compiler->getLangOpts()); std::unique_ptr ast_context( new ASTContext(m_compiler->getLangOpts(), m_compiler->getSourceManager(), m_compiler->getPreprocessor().getIdentifierTable(), - *m_selector_table.get(), *m_builtin_context.get())); + *m_selector_table.get(), builtin_context)); ast_context->InitBuiltinTypes(m_compiler->getTarget()); Index: packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py === --- packages/Python/lldbsuite/test/expression_command/call-function/TestCallBuiltinFunction.py +++ packages/Python/lldbsuite/test/expression_command
[Lldb-commits] [lldb] r340573 - XFAIL test for older versions of clang
Author: adrian Date: Thu Aug 23 14:00:37 2018 New Revision: 340573 URL: http://llvm.org/viewvc/llvm-project?rev=340573&view=rev Log: XFAIL test for older versions of clang Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py?rev=340573&r1=340572&r2=340573&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py Thu Aug 23 14:00:37 2018 @@ -9,7 +9,7 @@ class TestUnicodeSymbols(TestBase): mydir = TestBase.compute_mydir(__file__) -@expectedFailureAll(compiler="clang", compiler_version=['<', '7.0'], debug_info="dsym") +@expectedFailureAll(compiler="clang", compiler_version=['<', '7.0']) def test_union_members(self): self.build() spec = lldb.SBModuleSpec() ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
zturner accepted this revision. zturner added inline comments. Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:399-404 +JITLoaderList &ProcessMinidump::GetJITLoaders() { + if (!m_jit_loaders_ap) { +m_jit_loaders_ap = llvm::make_unique(); + } + return *m_jit_loaders_ap; +} Can you put a comment in here explaining that the base class implementation does additional work that is unnecessary for a minidump plugin, so this is basically a performance optimization? Repository: rLLDB LLDB https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r340574 - Change xfail to skipIf. The exact condition is really difficult to get
Author: adrian Date: Thu Aug 23 14:08:30 2018 New Revision: 340574 URL: http://llvm.org/viewvc/llvm-project?rev=340574&view=rev Log: Change xfail to skipIf. The exact condition is really difficult to get right and doesn't add much signal. Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py?rev=340574&r1=340573&r2=340574&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py Thu Aug 23 14:08:30 2018 @@ -9,7 +9,7 @@ class TestUnicodeSymbols(TestBase): mydir = TestBase.compute_mydir(__file__) -@expectedFailureAll(compiler="clang", compiler_version=['<', '7.0']) +@skipIf(compiler="clang", compiler_version=['<', '7.0']) def test_union_members(self): self.build() spec = lldb.SBModuleSpec() ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
lemo updated this revision to Diff 162270. lemo added a comment. Added the comment requested by zturner https://reviews.llvm.org/D51176 Files: source/Plugins/Process/minidump/ProcessMinidump.cpp source/Plugins/Process/minidump/ProcessMinidump.h Index: source/Plugins/Process/minidump/ProcessMinidump.h === --- source/Plugins/Process/minidump/ProcessMinidump.h +++ source/Plugins/Process/minidump/ProcessMinidump.h @@ -55,7 +55,7 @@ Status DoLoadCore() override; - DynamicLoader *GetDynamicLoader() override; + DynamicLoader *GetDynamicLoader() override { return nullptr; } ConstString GetPluginName() override; @@ -102,6 +102,8 @@ void ReadModuleList(); + JITLoaderList &GetJITLoaders() override; + private: FileSpec m_core_file; llvm::ArrayRef m_thread_list; Index: source/Plugins/Process/minidump/ProcessMinidump.cpp === --- source/Plugins/Process/minidump/ProcessMinidump.cpp +++ source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -16,7 +16,7 @@ #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/PluginManager.h" #include "lldb/Core/Section.h" -#include "lldb/Target/DynamicLoader.h" +#include "lldb/Target/JITLoaderList.h" #include "lldb/Target/MemoryRegionInfo.h" #include "lldb/Target/SectionLoadList.h" #include "lldb/Target/Target.h" @@ -201,12 +201,6 @@ return error; } -DynamicLoader *ProcessMinidump::GetDynamicLoader() { - if (m_dyld_ap.get() == nullptr) -m_dyld_ap.reset(DynamicLoader::FindPlugin(this, nullptr)); - return m_dyld_ap.get(); -} - ConstString ProcessMinidump::GetPluginName() { return GetPluginNameStatic(); } uint32_t ProcessMinidump::GetPluginVersion() { return 1; } @@ -401,3 +395,14 @@ } return true; } + +// For minidumps there's no runtime generated code so we don't need JITLoader(s) +// Avoiding them will also speed up minidump loading since JITLoaders normally +// try to set up symbolic breakpoints, which in turn may force loading more +// debug information than needed. +JITLoaderList &ProcessMinidump::GetJITLoaders() { + if (!m_jit_loaders_ap) { +m_jit_loaders_ap = llvm::make_unique(); + } + return *m_jit_loaders_ap; +} Index: source/Plugins/Process/minidump/ProcessMinidump.h === --- source/Plugins/Process/minidump/ProcessMinidump.h +++ source/Plugins/Process/minidump/ProcessMinidump.h @@ -55,7 +55,7 @@ Status DoLoadCore() override; - DynamicLoader *GetDynamicLoader() override; + DynamicLoader *GetDynamicLoader() override { return nullptr; } ConstString GetPluginName() override; @@ -102,6 +102,8 @@ void ReadModuleList(); + JITLoaderList &GetJITLoaders() override; + private: FileSpec m_core_file; llvm::ArrayRef m_thread_list; Index: source/Plugins/Process/minidump/ProcessMinidump.cpp === --- source/Plugins/Process/minidump/ProcessMinidump.cpp +++ source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -16,7 +16,7 @@ #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/PluginManager.h" #include "lldb/Core/Section.h" -#include "lldb/Target/DynamicLoader.h" +#include "lldb/Target/JITLoaderList.h" #include "lldb/Target/MemoryRegionInfo.h" #include "lldb/Target/SectionLoadList.h" #include "lldb/Target/Target.h" @@ -201,12 +201,6 @@ return error; } -DynamicLoader *ProcessMinidump::GetDynamicLoader() { - if (m_dyld_ap.get() == nullptr) -m_dyld_ap.reset(DynamicLoader::FindPlugin(this, nullptr)); - return m_dyld_ap.get(); -} - ConstString ProcessMinidump::GetPluginName() { return GetPluginNameStatic(); } uint32_t ProcessMinidump::GetPluginVersion() { return 1; } @@ -401,3 +395,14 @@ } return true; } + +// For minidumps there's no runtime generated code so we don't need JITLoader(s) +// Avoiding them will also speed up minidump loading since JITLoaders normally +// try to set up symbolic breakpoints, which in turn may force loading more +// debug information than needed. +JITLoaderList &ProcessMinidump::GetJITLoaders() { + if (!m_jit_loaders_ap) { +m_jit_loaders_ap = llvm::make_unique(); + } + return *m_jit_loaders_ap; +} ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
lemo marked an inline comment as done. lemo added a comment. In https://reviews.llvm.org/D51176#1211433, @jingham wrote: > The other option would be to move the code that populates the section load > list into the mini dump dynamic loader. That has the benefit of keeping this > consistent with the other process plugins, but OTOH is just moving code > around... It's an interesting idea, thanks. I'd like to keep the fix small and simple for now, but it's worth considering if the current minidump loading path will need more flexibility. https://reviews.llvm.org/D51176 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB340578: Restrict the set of plugins used for ProcessMinidump (authored by lemo, committed by ). Repository: rLLDB LLDB https://reviews.llvm.org/D51176 Files: source/Plugins/Process/minidump/ProcessMinidump.cpp source/Plugins/Process/minidump/ProcessMinidump.h Index: source/Plugins/Process/minidump/ProcessMinidump.h === --- source/Plugins/Process/minidump/ProcessMinidump.h +++ source/Plugins/Process/minidump/ProcessMinidump.h @@ -55,7 +55,7 @@ Status DoLoadCore() override; - DynamicLoader *GetDynamicLoader() override; + DynamicLoader *GetDynamicLoader() override { return nullptr; } ConstString GetPluginName() override; @@ -102,6 +102,8 @@ void ReadModuleList(); + JITLoaderList &GetJITLoaders() override; + private: FileSpec m_core_file; llvm::ArrayRef m_thread_list; Index: source/Plugins/Process/minidump/ProcessMinidump.cpp === --- source/Plugins/Process/minidump/ProcessMinidump.cpp +++ source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -16,7 +16,7 @@ #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/PluginManager.h" #include "lldb/Core/Section.h" -#include "lldb/Target/DynamicLoader.h" +#include "lldb/Target/JITLoaderList.h" #include "lldb/Target/MemoryRegionInfo.h" #include "lldb/Target/SectionLoadList.h" #include "lldb/Target/Target.h" @@ -201,12 +201,6 @@ return error; } -DynamicLoader *ProcessMinidump::GetDynamicLoader() { - if (m_dyld_ap.get() == nullptr) -m_dyld_ap.reset(DynamicLoader::FindPlugin(this, nullptr)); - return m_dyld_ap.get(); -} - ConstString ProcessMinidump::GetPluginName() { return GetPluginNameStatic(); } uint32_t ProcessMinidump::GetPluginVersion() { return 1; } @@ -401,3 +395,14 @@ } return true; } + +// For minidumps there's no runtime generated code so we don't need JITLoader(s) +// Avoiding them will also speed up minidump loading since JITLoaders normally +// try to set up symbolic breakpoints, which in turn may force loading more +// debug information than needed. +JITLoaderList &ProcessMinidump::GetJITLoaders() { + if (!m_jit_loaders_ap) { +m_jit_loaders_ap = llvm::make_unique(); + } + return *m_jit_loaders_ap; +} Index: source/Plugins/Process/minidump/ProcessMinidump.h === --- source/Plugins/Process/minidump/ProcessMinidump.h +++ source/Plugins/Process/minidump/ProcessMinidump.h @@ -55,7 +55,7 @@ Status DoLoadCore() override; - DynamicLoader *GetDynamicLoader() override; + DynamicLoader *GetDynamicLoader() override { return nullptr; } ConstString GetPluginName() override; @@ -102,6 +102,8 @@ void ReadModuleList(); + JITLoaderList &GetJITLoaders() override; + private: FileSpec m_core_file; llvm::ArrayRef m_thread_list; Index: source/Plugins/Process/minidump/ProcessMinidump.cpp === --- source/Plugins/Process/minidump/ProcessMinidump.cpp +++ source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -16,7 +16,7 @@ #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/PluginManager.h" #include "lldb/Core/Section.h" -#include "lldb/Target/DynamicLoader.h" +#include "lldb/Target/JITLoaderList.h" #include "lldb/Target/MemoryRegionInfo.h" #include "lldb/Target/SectionLoadList.h" #include "lldb/Target/Target.h" @@ -201,12 +201,6 @@ return error; } -DynamicLoader *ProcessMinidump::GetDynamicLoader() { - if (m_dyld_ap.get() == nullptr) -m_dyld_ap.reset(DynamicLoader::FindPlugin(this, nullptr)); - return m_dyld_ap.get(); -} - ConstString ProcessMinidump::GetPluginName() { return GetPluginNameStatic(); } uint32_t ProcessMinidump::GetPluginVersion() { return 1; } @@ -401,3 +395,14 @@ } return true; } + +// For minidumps there's no runtime generated code so we don't need JITLoader(s) +// Avoiding them will also speed up minidump loading since JITLoaders normally +// try to set up symbolic breakpoints, which in turn may force loading more +// debug information than needed. +JITLoaderList &ProcessMinidump::GetJITLoaders() { + if (!m_jit_loaders_ap) { +m_jit_loaders_ap = llvm::make_unique(); + } + return *m_jit_loaders_ap; +} ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r340578 - Restrict the set of plugins used for ProcessMinidump
Author: lemo Date: Thu Aug 23 14:34:33 2018 New Revision: 340578 URL: http://llvm.org/viewvc/llvm-project?rev=340578&view=rev Log: Restrict the set of plugins used for ProcessMinidump 1. The dynamic loaders should not be needed for loading minidumps and they may create problems (ex. the macOS loader resets the list of loaded sections, which for minidumps are already set up during minidump loading) 2. In general, the extra plugins can do extraneous work which hurts performance (ex. trying to set up implicit symbolic breakpoints, which in turn will trigger extra debug information loading) Differential Revision: https://reviews.llvm.org/D51176 Modified: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h Modified: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp?rev=340578&r1=340577&r2=340578&view=diff == --- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp (original) +++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp Thu Aug 23 14:34:33 2018 @@ -16,7 +16,7 @@ #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/PluginManager.h" #include "lldb/Core/Section.h" -#include "lldb/Target/DynamicLoader.h" +#include "lldb/Target/JITLoaderList.h" #include "lldb/Target/MemoryRegionInfo.h" #include "lldb/Target/SectionLoadList.h" #include "lldb/Target/Target.h" @@ -201,12 +201,6 @@ Status ProcessMinidump::DoLoadCore() { return error; } -DynamicLoader *ProcessMinidump::GetDynamicLoader() { - if (m_dyld_ap.get() == nullptr) -m_dyld_ap.reset(DynamicLoader::FindPlugin(this, nullptr)); - return m_dyld_ap.get(); -} - ConstString ProcessMinidump::GetPluginName() { return GetPluginNameStatic(); } uint32_t ProcessMinidump::GetPluginVersion() { return 1; } @@ -401,3 +395,14 @@ bool ProcessMinidump::GetProcessInfo(Pro } return true; } + +// For minidumps there's no runtime generated code so we don't need JITLoader(s) +// Avoiding them will also speed up minidump loading since JITLoaders normally +// try to set up symbolic breakpoints, which in turn may force loading more +// debug information than needed. +JITLoaderList &ProcessMinidump::GetJITLoaders() { + if (!m_jit_loaders_ap) { +m_jit_loaders_ap = llvm::make_unique(); + } + return *m_jit_loaders_ap; +} Modified: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h?rev=340578&r1=340577&r2=340578&view=diff == --- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h (original) +++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h Thu Aug 23 14:34:33 2018 @@ -55,7 +55,7 @@ public: Status DoLoadCore() override; - DynamicLoader *GetDynamicLoader() override; + DynamicLoader *GetDynamicLoader() override { return nullptr; } ConstString GetPluginName() override; @@ -102,6 +102,8 @@ protected: void ReadModuleList(); + JITLoaderList &GetJITLoaders() override; + private: FileSpec m_core_file; llvm::ArrayRef m_thread_list; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51185: Reuse the SelectorTable from Clang's Preprocessor
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM. This is NFC, it seems. There's a FIXME in Preprocessor.h about the lifetime of SelectorTable eventually not being tied to Preprocessor, but this is correct for now. Repository: rLLDB LLDB https://reviews.llvm.org/D51185 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51185: Reuse the SelectorTable from Clang's Preprocessor
teemperor added a comment. Yeah, but the FIXME has been around for like a decade, so I think that bug has survived long enough to be promoted to a feature :). Repository: rLLDB LLDB https://reviews.llvm.org/D51185 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r340585 - Reuse the SelectorTable from Clang's Preprocessor
Author: teemperor Date: Thu Aug 23 15:40:54 2018 New Revision: 340585 URL: http://llvm.org/viewvc/llvm-project?rev=340585&view=rev Log: Reuse the SelectorTable from Clang's Preprocessor Summary: At the moment we create our own SelectorTable even though the Preprocessor always creates one for us that we can (and should) reuse. Reviewers: vsk Reviewed By: vsk Subscribers: lldb-commits Differential Revision: https://reviews.llvm.org/D51185 Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=340585&r1=340584&r2=340585&view=diff == --- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (original) +++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp Thu Aug 23 15:40:54 2018 @@ -507,8 +507,6 @@ ClangExpressionParser::ClangExpressionPa // 8. Most of this we get from the CompilerInstance, but we also want to give // the context an ExternalASTSource. - m_selector_table.reset(new SelectorTable()); - // We enable all builtin functions beside the builtins from libc/libm (e.g. // 'fopen'). Those libc functions are already correctly handled by LLDB, and // additionally enabling them as expandable builtins is breaking Clang. @@ -522,7 +520,7 @@ ClangExpressionParser::ClangExpressionPa std::unique_ptr ast_context( new ASTContext(m_compiler->getLangOpts(), m_compiler->getSourceManager(), m_compiler->getPreprocessor().getIdentifierTable(), - *m_selector_table.get(), builtin_context)); + PP.getSelectorTable(), builtin_context)); ast_context->InitBuiltinTypes(m_compiler->getTarget()); Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h?rev=340585&r1=340584&r2=340585&view=diff == --- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h (original) +++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h Thu Aug 23 15:40:54 2018 @@ -149,8 +149,6 @@ private: m_file_manager; ///< The Clang file manager object used by the compiler std::unique_ptr m_compiler; ///< The Clang compiler used to parse expressions into IR - std::unique_ptr - m_selector_table; ///< Selector table for Objective-C methods std::unique_ptr m_code_generator; ///< The Clang object that generates IR ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51185: Reuse the SelectorTable from Clang's Preprocessor
This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB340585: Reuse the SelectorTable from Clang's Preprocessor (authored by teemperor, committed by ). Repository: rLLDB LLDB https://reviews.llvm.org/D51185 Files: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h === --- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h +++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h @@ -149,8 +149,6 @@ m_file_manager; ///< The Clang file manager object used by the compiler std::unique_ptr m_compiler; ///< The Clang compiler used to parse expressions into IR - std::unique_ptr - m_selector_table; ///< Selector table for Objective-C methods std::unique_ptr m_code_generator; ///< The Clang object that generates IR Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp === --- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -507,8 +507,6 @@ // 8. Most of this we get from the CompilerInstance, but we also want to give // the context an ExternalASTSource. - m_selector_table.reset(new SelectorTable()); - // We enable all builtin functions beside the builtins from libc/libm (e.g. // 'fopen'). Those libc functions are already correctly handled by LLDB, and // additionally enabling them as expandable builtins is breaking Clang. @@ -522,7 +520,7 @@ std::unique_ptr ast_context( new ASTContext(m_compiler->getLangOpts(), m_compiler->getSourceManager(), m_compiler->getPreprocessor().getIdentifierTable(), - *m_selector_table.get(), builtin_context)); + PP.getSelectorTable(), builtin_context)); ast_context->InitBuiltinTypes(m_compiler->getTarget()); Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h === --- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h +++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h @@ -149,8 +149,6 @@ m_file_manager; ///< The Clang file manager object used by the compiler std::unique_ptr m_compiler; ///< The Clang compiler used to parse expressions into IR - std::unique_ptr - m_selector_table; ///< Selector table for Objective-C methods std::unique_ptr m_code_generator; ///< The Clang object that generates IR Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp === --- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -507,8 +507,6 @@ // 8. Most of this we get from the CompilerInstance, but we also want to give // the context an ExternalASTSource. - m_selector_table.reset(new SelectorTable()); - // We enable all builtin functions beside the builtins from libc/libm (e.g. // 'fopen'). Those libc functions are already correctly handled by LLDB, and // additionally enabling them as expandable builtins is breaking Clang. @@ -522,7 +520,7 @@ std::unique_ptr ast_context( new ASTContext(m_compiler->getLangOpts(), m_compiler->getSourceManager(), m_compiler->getPreprocessor().getIdentifierTable(), - *m_selector_table.get(), builtin_context)); + PP.getSelectorTable(), builtin_context)); ast_context->InitBuiltinTypes(m_compiler->getTarget()); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk added a comment. Ping. https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r340589 - Add more pre-run asserts for the DirCompletionAbsolute test
Author: teemperor Date: Thu Aug 23 16:21:52 2018 New Revision: 340589 URL: http://llvm.org/viewvc/llvm-project?rev=340589&view=rev Log: Add more pre-run asserts for the DirCompletionAbsolute test Summary: The DirCompletionAbsolute is still randomly failing on the nodes even after D50722, so this patch adds more asserts that verify certain properties on which the actual completion implementation relies on. The first assert checks that the directory we complete on actually exists. If the directory doesn't exist on the next CI failure, this assert should catch it and we know that the 0 matches come from a missing base directory. The second assert is just checking that we are below the PATH_MAX limit that the completion checks against. This check could randomly fail if the temporary directories we generate are sometimes longer than PATH_MAX, and the assert can tell us that this is the reason we failed (instead of the mysterious '0 matches'). (As a sidenote: We shouldn't be checking against PATH_MAX anyway in the code (as this is just wrong). Also the disk completion API really needs a better error mechanism than returning 0 on both error or no-results.) Reviewers: aprantl, friss Reviewed By: aprantl Subscribers: abidh Differential Revision: https://reviews.llvm.org/D5 Modified: lldb/trunk/unittests/Interpreter/TestCompletion.cpp Modified: lldb/trunk/unittests/Interpreter/TestCompletion.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Interpreter/TestCompletion.cpp?rev=340589&r1=340588&r2=340589&view=diff == --- lldb/trunk/unittests/Interpreter/TestCompletion.cpp (original) +++ lldb/trunk/unittests/Interpreter/TestCompletion.cpp Thu Aug 23 16:21:52 2018 @@ -163,6 +163,9 @@ TEST_F(CompletionTest, DirCompletionAbso // When a directory is specified that doesn't end in a slash, it searches // for that directory, not items under it. + // Sanity check that the path we complete on exists and isn't too long. + ASSERT_TRUE(llvm::sys::fs::exists(BaseDir)); + ASSERT_LE(BaseDir.size(), static_cast(PATH_MAX)); size_t Count = CommandCompletions::DiskDirectories(BaseDir, Results, Resolver); ASSERT_EQ(1u, Count); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits