Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
labath added a comment. In http://reviews.llvm.org/D18886#397843, @amccarth wrote: > It's weird in that, if you run the test independently, it passes. But if you > run it with the multiprocess test runner (ninja check-lldb), then it fails on > this line: > > self.fail("Setting a breakpoint generated an unexpected event: %s" % > lldb.SBDebugger.StateAsCString(lldb.SBProcess.GetStateFromEvent(event))) > > The state in this case is stopped. I suspect this test has uncovered a problem in the ProcessWindows implementation. The same problem we were solving here for ProcessGdbRemote. Namely, the question here is what happens if we try to set a breakpoint while the process is running. ProcessGdbRemote (and I expect ProcessWindows as well) needs to stop the inferior to be able to set the breakpoint. However, since this stop wasn't requested by the user, it should not generate any externally visible stops. The process should be silently resumed after you are done with it (I have no idea how is this supposed to be done with the non-gdb-remote processes). I suspect the test is flaky for you because of different timings when running under heavy system load, but after the issue is fixed the test should pass 100% of time. Unfortunately, I think this task will be up to you. :) http://reviews.llvm.org/D18886 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
labath added a comment. A random thought: will `getchar()` block the inferior on windows (because of missing stdio forwarding, et al.). If it wont then this could be the cause of the flakyness. If that's the case, then we can replace that call with something that will surely halt progress, like an infinite loop with sleep or something. http://reviews.llvm.org/D18886 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266050 - Revert "Restore the lazy initialization of ScriptInterpreterPython, which was lost as part of the SystemLifetimeManager work"
Author: labath Date: Tue Apr 12 04:06:08 2016 New Revision: 266050 URL: http://llvm.org/viewvc/llvm-project?rev=266050&view=rev Log: Revert "Restore the lazy initialization of ScriptInterpreterPython, which was lost as part of the SystemLifetimeManager work" This change breaks python unit tests. This reverts commit 266033. Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=266050&r1=266049&r2=266050&view=diff == --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp (original) +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Tue Apr 12 04:06:08 2016 @@ -274,8 +274,8 @@ ScriptInterpreterPython::ScriptInterpret m_lock_count(0), m_command_thread_state(nullptr) { -InitializePrivate(); - +assert(g_initialized && "ScriptInterpreterPython created but InitializePrivate has not been called!"); + m_dictionary_name.append("_dict"); StreamString run_string; run_string.Printf ("%s = dict()", m_dictionary_name.c_str()); @@ -330,6 +330,8 @@ ScriptInterpreterPython::Initialize() std::call_once(g_once_flag, []() { +InitializePrivate(); + PluginManager::RegisterPlugin(GetPluginNameStatic(), GetPluginDescriptionStatic(), lldb::eScriptLanguagePython, @@ -3095,9 +3097,7 @@ ScriptInterpreterPython::InitializeInter void ScriptInterpreterPython::InitializePrivate () { -if (g_initialized) -return; - +assert(!g_initialized && "ScriptInterpreterPython::InitializePrivate() called more than once!"); g_initialized = true; Timer scoped_timer (__PRETTY_FUNCTION__, __PRETTY_FUNCTION__); Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h?rev=266050&r1=266049&r2=266050&view=diff == --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h (original) +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h Tue Apr 12 04:06:08 2016 @@ -372,6 +372,9 @@ public: void ResetOutputFileHandle(FILE *new_fh) override; static void +InitializePrivate (); + +static void InitializeInterpreter (SWIGInitCallback python_swig_init_callback, SWIGBreakpointCallbackFunction swig_breakpoint_callback, SWIGWatchpointCallbackFunction swig_watchpoint_callback, @@ -504,9 +507,6 @@ public: }; protected: -static void -InitializePrivate (); - class SynchronicityHandler { private: ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r266033 - Restore the lazy initialization of ScriptInterpreterPython, which was lost as part of the SystemLifetimeManager work
This breaks the python unit tests. Doesn't seem hard to fix, it seems we just need some way to "really" initialize python before we run the tests. However, I have reverted this change until that happens to keep to bots green. pl On 12 April 2016 at 02:41, Enrico Granata via lldb-commits < lldb-commits@lists.llvm.org> wrote: > > On Apr 11, 2016, at 6:28 PM, Zachary Turner wrote: > > I have a feeling this breaks something, but I'm not sure what. I remember > specifically needing this for some reason. > > > If you manage to repro the breakage, let’s work on getting it fixed > I’d like to not have to initialize Python eagerly - most users will > probably not touch anything Python during debugging, and I have seen a few > spin dumps connected to the eager initialization during debugging startup > > Did you verify that the gtest suite > > > No - but I believe the bots run it, so I will probably get yelled at soon > if I broke something > > as well as the dotest suite > > > Yes > > and the interactive interpreter all still pass / work? > > > Yes > > On Mon, Apr 11, 2016 at 6:14 PM Enrico Granata via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > >> Author: enrico >> Date: Mon Apr 11 20:08:35 2016 >> New Revision: 266033 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=266033&view=rev >> Log: >> Restore the lazy initialization of ScriptInterpreterPython, which was >> lost as part of the SystemLifetimeManager work >> >> >> Modified: >> >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >> >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >> >> Modified: >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=266033&r1=266032&r2=266033&view=diff >> >> == >> --- >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >> (original) >> +++ >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >> Mon Apr 11 20:08:35 2016 >> @@ -274,8 +274,8 @@ ScriptInterpreterPython::ScriptInterpret >> m_lock_count(0), >> m_command_thread_state(nullptr) >> { >> -assert(g_initialized && "ScriptInterpreterPython created but >> InitializePrivate has not been called!"); >> - >> +InitializePrivate(); >> + >> m_dictionary_name.append("_dict"); >> StreamString run_string; >> run_string.Printf ("%s = dict()", m_dictionary_name.c_str()); >> @@ -330,8 +330,6 @@ ScriptInterpreterPython::Initialize() >> >> std::call_once(g_once_flag, []() >> { >> -InitializePrivate(); >> - >> PluginManager::RegisterPlugin(GetPluginNameStatic(), >>GetPluginDescriptionStatic(), >>lldb::eScriptLanguagePython, >> @@ -3097,7 +3095,9 @@ ScriptInterpreterPython::InitializeInter >> void >> ScriptInterpreterPython::InitializePrivate () >> { >> -assert(!g_initialized && >> "ScriptInterpreterPython::InitializePrivate() called more than once!"); >> +if (g_initialized) >> +return; >> + >> g_initialized = true; >> >> Timer scoped_timer (__PRETTY_FUNCTION__, __PRETTY_FUNCTION__); >> >> Modified: >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h?rev=266033&r1=266032&r2=266033&view=diff >> >> == >> --- >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >> (original) >> +++ >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >> Mon Apr 11 20:08:35 2016 >> @@ -372,9 +372,6 @@ public: >> void ResetOutputFileHandle(FILE *new_fh) override; >> >> static void >> -InitializePrivate (); >> - >> -static void >> InitializeInterpreter (SWIGInitCallback python_swig_init_callback, >> SWIGBreakpointCallbackFunction >> swig_breakpoint_callback, >> SWIGWatchpointCallbackFunction >> swig_watchpoint_callback, >> @@ -507,6 +504,9 @@ public: >> }; >> >> protected: >> +static void >> +InitializePrivate (); >> + >> class SynchronicityHandler >> { >> private: >> >> >> ___ >> lldb-commits mailing list >> lldb-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >> > > > Thanks, > *- Enrico* > 📩 egranata@.com ☎️ 27683 > > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > > _
Re: [Lldb-commits] [PATCH] D18977: Add new ABI callback to return CFA offset
uweigand added a comment. In http://reviews.llvm.org/D18977#397626, @clayborg wrote: > I am not sure why this offset of 160 isn't represented in the unwind info. I > guess the OS unwinder that uses the EH frame info just knows that it must > adjust the CFA? That seems like a hack. It seems like the unwind info should > be fixed and the OS level unwinder should be fixed so that the info can be > used correctly without every debugger or tool that has to parse this know > about such a hack? Do you have control over the OS and are you able to fix > this? That seems like the better fix. The offset of 160 is represented in the unwind info when computing the CFA itself. This change is about unwinding the SP register when no explicit unwind info for SP is present. In this case, the LLDB unwinder assumes it should set SP to the CFA, which is where the problem comes in. Now, on s390, most frames actually have explicit unwind instructions for SP, so this special case is not even triggered. Only for leaf functions without a stack frame can we ever see the scenario that there are no SP unwind instructions. In those cases, the correct result on s390 is to simply leave SP unchanged (since the function did not in fact allocate a frame). However, due to the special case in the LLDB unwinder, SP is not left unchanged, but set to the CFA (i.e. SP + 160 in this case). This is wrong. There are two possible ways to fix this: - Disable the special case of setting SP to CFA on s390. Instead, when there is no unwind info for SP, just leave it unchanged (just like any other call-saved register). This solution is implemented in the platform unwinder (libgcc) and also in GDB. - Fix the special case of setting SP to CFA by actually taking the CFA bias into account. This is what this patch implements for LLDB. I've implemented the second method for LLDB since it actually seemed a bit more general to me (describes more precisely what actually happens on the platform), and also it seemed to fit nicely into the ABI scheme. If you prefer, I can implement the first method instead. However, since we cannot unconditionally disable the special case of setting SP to CFA (other platforms depend on that -- which is arguably the real hack to begin with), we would still need some ABI method to tell the unwinder whether or not the special case is needed. There's no real way to "fix the unwind info", since this is not a new platform and binaries containing this unwind info have been in the field for more than 15 years. In any case, it is not clear that there is anything to "fix" -- a binary containing no unwind info for SP to indicate that SP does not change isn't really "wrong" IMO. http://reviews.llvm.org/D18977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18985: Fix test cases for big-endian systems
uweigand added a comment. In http://reviews.llvm.org/D18985#397674, @clayborg wrote: > So many tests above are going to accept either a little endian or big endian > value. This will make most of these tests useless since if a little endian > machine fails with a big endian number we won't catch it and vice versa. So > we need to expect the correct value for little endian and a different but > correct one for big endian tests and only accept the correct one. I fully agree this would be preferable. Unfortunately I didn't see a straightforward way to detect in those test cases whether the target platform is big- or little-endian. In the Python API tests this is simple, but these tests here just operate on the console interface. Is there a good way to detect target byte order in such tests? > Zero seems like a bad alternate value as it could cover up a real failure. > Can we improve this test so that we are testing for actual values? Or can we > change the test by endianness and test for 16777216 for little endian and > something else that is not zero for big endian? We don't want zero to be > valid for little endian tests. The test case is a bit weird in how it (apparently deliberately) overlays mis-aligned synthetic fields across an integer array. This is always going to behave quite differently depending on byte order. I'll see if I can change the test so that values will be nonzero both on big- and on little-endian platforms. http://reviews.llvm.org/D18985 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18985: Fix test cases for big-endian systems
labath added a subscriber: labath. labath added a comment. In http://reviews.llvm.org/D18985#398169, @uweigand wrote: > In http://reviews.llvm.org/D18985#397674, @clayborg wrote: > > > So many tests above are going to accept either a little endian or big > > endian value. This will make most of these tests useless since if a little > > endian machine fails with a big endian number we won't catch it and vice > > versa. So we need to expect the correct value for little endian and a > > different but correct one for big endian tests and only accept the correct > > one. > > > I fully agree this would be preferable. Unfortunately I didn't see a > straightforward way to detect in those test cases whether the target platform > is big- or little-endian. In the Python API tests this is simple, but these > tests here just operate on the console interface. Is there a good way to > detect target byte order in such tests? Just because the tests test the console interface, it doesn't mean you can't use the python interface to do auxiliary tasks, like computing the expectations. It sounds like you should be able to to a `self.target.GetByteOrder()` to get the info you need. (use `self.dbg.GetSelectedTarget()` if `self.target` happens to be empty, i don't remember if it's always set). http://reviews.llvm.org/D18985 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266054 - Skip a test in TestNamespaceLookup on linux to avoid a crash
Author: labath Date: Tue Apr 12 05:06:37 2016 New Revision: 266054 URL: http://llvm.org/viewvc/llvm-project?rev=266054&view=rev Log: Skip a test in TestNamespaceLookup on linux to avoid a crash Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespaceLookup.py Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespaceLookup.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespaceLookup.py?rev=266054&r1=266053&r2=266054&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespaceLookup.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/TestNamespaceLookup.py Tue Apr 12 05:06:37 2016 @@ -159,6 +159,7 @@ class NamespaceLookupTestCase(TestBase): # NOTE: this test may fail on older systems that don't emit import # entries in DWARF - may need to add checks for compiler versions here. +@skipIf(compiler="gcc", oslist=["linux"], debug_info=["dwo"]) # Skip to avoid crash @expectedFailureAll(oslist=["windows", "linux", "freebsd"], bugnumber="llvm.org/pr25819") def test_scope_after_using_directive_lookup_with_run_command(self): """Test scope lookup after using directive in lldb.""" ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18978: Support Linux on SystemZ as platform
labath added a comment. Could you also add a core file test to `TestLinuxCore`? It should be a matter of running `make_core.sh`, saving the files and creating a new test function in the file (I've tried to make it simple, if you see room for improvement, then let me know). I think this is especially important as probably noone here has access to this hardware, so this will enable the rest of the developers to run at least a basic sanity check on their changes. Comment at: packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchlocation/TestWatchLocation.py:36 @@ -35,2 +35,3 @@ @expectedFailureAll(triple = re.compile('^mips')) # Most of the MIPS boards provide only one H/W watchpoints, and S/W watchpoints are not supported yet +@expectedFailureAll(archs=['s390x']) # SystemZ also currently supports only one H/W watchpoint @skipIfDarwin We generally expectedFailure for things which we consider an lldb bug, and skip for cases when the test simply does not apply. Platform not having enough watchpoints sounds like the latter case. I see you were simply copying the mips case (which does not follow this either), and it doesn't really matter to me, but I just wanted to make you aware of that. http://reviews.llvm.org/D18978 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18982: Handle bit fields on big-endian systems correctly
labath added a subscriber: labath. labath added a comment. It would be worthwhile to add a unit test for the DataExtractor fix (we don't have many of those, but we're trying to build them up). http://reviews.llvm.org/D18982 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18981: Fix usage of APInt.getRawData for big-endian systems
labath added a comment. It would be great if you could add a some unit tests for your modifications to the Scalar class. You don't have to be too exhaustive, but a couple of tests would go a long way. http://reviews.llvm.org/D18981 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266058 - Bump up timeout in TestGdbRemoteProcessInfo
Author: labath Date: Tue Apr 12 06:59:41 2016 New Revision: 266058 URL: http://llvm.org/viewvc/llvm-project?rev=266058&view=rev Log: Bump up timeout in TestGdbRemoteProcessInfo the process info packet is slow, and sometimes it does not arrive on time when run on the android emulator. Modified: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py Modified: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py?rev=266058&r1=266057&r2=266058&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteProcessInfo.py Tue Apr 12 06:59:41 2016 @@ -52,7 +52,7 @@ class TestGdbRemoteProcessInfo(gdbremote self.add_process_info_collection_packets() # Run the stream -context = self.expect_gdbremote_sequence() +context = self.expect_gdbremote_sequence(timeout_seconds = 8) self.assertIsNotNone(context) # Gather process info response ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18984: Fix ARM instruction emulation tests on big-endian systems
omjavaid added a comment. Seems legit but One cosmetic comment inline. Also have you tested this patch by running LLDB testsuite on arm in both little and big endian modes? Comment at: source/Plugins/Instruction/ARM/EmulationStateARM.cpp:157 @@ -149,12 +156,3 @@ { -if (size > 8) -return false; - -if (size <= 4) -m_memory[p_address] = value; -else if (size == 8) -{ -m_memory[p_address] = (value << 32) >> 32; -m_memory[p_address + 4] = value << 32; -} +m_memory[p_address] = value; return true; m_memory is a map with map type uint32? I think we will end up loosing data here if its larger than 4 bytes. if StoreToPseudoAddress isnt used elsewhere better change value from uint64 to uint32 ? Also size seems to be a redundant argument now. http://reviews.llvm.org/D18984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18985: Fix test cases for big-endian systems
uweigand added a comment. In http://reviews.llvm.org/D18985#398171, @labath wrote: > Just because the tests test the console interface, it doesn't mean you can't > use the python interface to do auxiliary tasks, like computing the > expectations. It sounds like you should be able to to a > `self.target.GetByteOrder()` to get the info you need. (use > `self.dbg.GetSelectedTarget()` if `self.target` happens to be empty, i don't > remember if it's always set). Indeed, that seems to work fine. Thanks for the tip! I'll update the tests accordingly. http://reviews.llvm.org/D18985 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266072 - Fixup TestFdLeak
Author: labath Date: Tue Apr 12 08:55:54 2016 New Revision: 266072 URL: http://llvm.org/viewvc/llvm-project?rev=266072&view=rev Log: Fixup TestFdLeak this test was unintentionally XFAILed due to a change in the behavior of the expectedFailure decorator. Fix that. Also, mark the test as debug-info independent while I'm in there. Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py?rev=266072&r1=266071&r2=266072&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py Tue Apr 12 08:55:54 2016 @@ -16,12 +16,15 @@ from lldbsuite.test.decorators import * def python_leaky_fd_version(test): import sys # Python random module leaks file descriptors on some versions. -return (sys.version_info >= (2, 7, 8) and sys.version_info < (2, 7, 10), -"Python random module leaks file descriptors in this python version") +if sys.version_info >= (2, 7, 8) and sys.version_info < (2, 7, 10): +return "Python random module leaks file descriptors in this python version" +return None class AvoidsFdLeakTestCase(TestBase): +NO_DEBUG_INFO_TESTCASE = True + mydir = TestBase.compute_mydir(__file__) @expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376") @@ -56,7 +59,6 @@ class AvoidsFdLeakTestCase(TestBase): @expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376") @expectedFailureAll(oslist=['freebsd'], bugnumber="llvm.org/pr25624 still failing with Python 2.7.10") -@expectedFlakeyLinux @skipIfWindows # The check for descriptor leakage needs to be implemented differently here. @skipIfTargetAndroid() # Android have some other file descriptors open by the shell def test_fd_leak_multitarget (self): ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18973: Find .plt section in object files generated by recent ld
tberghammer accepted this revision. tberghammer added a comment. Looks good as I am not aware of any system where the plt sections has different section name http://reviews.llvm.org/D18973 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18980: Make Scalar::GetBytes and RegisterValue::GetBytes const
tberghammer accepted this revision. tberghammer added a comment. Looks good http://reviews.llvm.org/D18980 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18984: Fix ARM instruction emulation tests on big-endian systems
tberghammer added a comment. Generally looks good with 2 minor comment inline. I also run the test suite on Android ARM (little endian) and everything looked fine Comment at: source/Plugins/Instruction/ARM/EmulationStateARM.cpp:127 @@ -119,3 +126,3 @@ else -value = m_vfp_regs.d_regs[reg_num - dwarf_d16]; +value = m_vfp_regs.d_regs[idx]; } I think you need "idx - 16" here Comment at: source/Plugins/Instruction/ARM/EmulationStateARM.cpp:157 @@ -149,12 +156,3 @@ { -if (size > 8) -return false; - -if (size <= 4) -m_memory[p_address] = value; -else if (size == 8) -{ -m_memory[p_address] = (value << 32) >> 32; -m_memory[p_address + 4] = value << 32; -} +m_memory[p_address] = value; return true; omjavaid wrote: > m_memory is a map with map type uint32? I think we will end up loosing data > here if its larger than 4 bytes. if StoreToPseudoAddress isnt used elsewhere > better change value from uint64 to uint32 ? Also size seems to be a redundant > argument now. > > All caller of the function ensures size<=4 so I think we should change value to a uint32_t and remove size as it is not used anymore. http://reviews.llvm.org/D18984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19004: Use the section sizes to determine symbols sizes in the Symtab instead of just using the following symbol's address
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Only add sections that don't have children in AddSectionsToRangeMap() and this will be good to go. Comment at: source/Symbol/Symtab.cpp:952-975 @@ +951,26 @@ +// recusively adding any children sections. +static void +AddSectionsToRangeMap (SectionList *sectlist, RangeVector §ion_ranges) +{ +const int num_sections = sectlist->GetNumSections (0); +for (int i = 0; i < num_sections; i++) +{ +SectionSP sect_sp = sectlist->GetSectionAtIndex (i); +if (sect_sp) +{ +addr_t base_addr = sect_sp->GetFileAddress(); +size_t size = sect_sp->GetByteSize(); +RangeVector::Entry entry; +entry.SetRangeBase (base_addr); +entry.SetByteSize (size); +section_ranges.Append (entry); + +SectionList &child_sectlist = sect_sp->GetChildren(); + if (child_sectlist.GetNumSections (0) > 0) + { +AddSectionsToRangeMap (&child_sectlist, section_ranges); +} +} +} +} + You want to only add sections that don't have children. On MacOSX we have: __TEXT [0x1000-0x2000) __text [0x1000-0x1100) __textcoal_nt [0x1100-0x1200) If you have all of these in the mix you will have a non deterministic sort since __TEXT and __text will compare to the same thing. Object files are also tricky because they have only one LC_SEGMENT with a bunch of sections and the ObjectFileMachO will actually make segments from the segname and sectname and they will have overlapping address ranges. Checking out a .o file from a recent build: ``` % dwarfdump -R main.o Sections Section Name Segment Name addr size offset alignreloff nreloc flagsreserv1 reserv2 reserv3 size size % == __text __TEXT 0165 0590 0004 0b60 0015 8400 357 nan% __debug_info __DWARF 0165 00da 06f5 0c08 0004 0200 218 nan% __debug_abbrev __DWARF 023f 007d 07cf 0200 125 nan% __debug_line __DWARF 02bc 0075 084c 0c28 0001 0200 117 nan% __debug_str __DWARF 0331 008f 08c1 0200 143 nan% __debug_loc __DWARF 03c0 0950 0200 0 nan% __debug_ranges __DWARF 03c0 0950 0200 0 nan% __cstring__TEXT 03c0 009c 0950 0002 156 nan% __apple_names__DWARF 045c 003c 09ec 0200 60 nan% __apple_objc __DWARF 0498 0024 0a28 0200 36 nan% __apple_namespac __DWARF 04bc 0024 0a4c 0200 36 nan% __apple_types__DWARF 04e0 0066 0a70 0200 102 nan% __apple_exttypes __DWARF 0546 0024 0ad6 0200 36 nan% __compact_unwind __LD 0570 0020 0b00 0003 0c30 0001 0200 32 nan% __eh_frame __TEXT 0590 0040 0b20 0003 680b 64 nan% ``` Now if we look at what ObjectFileMachO does: ``` % xcrun lldb main.o (lldb) target create "main.o" Current executable set to 'main.o' (x86_64). (lldb) image dump sections Dumping sections for 1 modules. Sections for '/Volumes/work/gclayton/Documents/src/args/main.o' (x86_64): SectID Type File Address File Off. File Size Flags Section Name -- --- -- -- -- ---
Re: [Lldb-commits] [PATCH] D18977: Add new ABI callback to return CFA offset
clayborg added a comment. In http://reviews.llvm.org/D18977#398157, @uweigand wrote: > In http://reviews.llvm.org/D18977#397626, @clayborg wrote: > > > I am not sure why this offset of 160 isn't represented in the unwind info. > > I guess the OS unwinder that uses the EH frame info just knows that it must > > adjust the CFA? That seems like a hack. It seems like the unwind info > > should be fixed and the OS level unwinder should be fixed so that the info > > can be used correctly without every debugger or tool that has to parse this > > know about such a hack? Do you have control over the OS and are you able to > > fix this? That seems like the better fix. > > > The offset of 160 is represented in the unwind info when computing the CFA > itself. This change is about unwinding the SP register when no explicit > unwind info for SP is present. In this case, the LLDB unwinder assumes it > should set SP to the CFA, which is where the problem comes in. This is usually represented by the CIE (Common Information Entry) which is pointed to from the FDE (Frame Description Entry). The CIE has the initial state that all FDEs can share. Seems like there should be an entry for the SP that says the rule to unwind it is "CFA+160"? > Now, on s390, most frames actually have explicit unwind instructions for SP, > so this special case is not even triggered. Only for leaf functions without > a stack frame can we ever see the scenario that there are no SP unwind > instructions. In those cases, the correct result on s390 is to simply leave > SP unchanged (since the function did not in fact allocate a frame). > > However, due to the special case in the LLDB unwinder, SP is not left > unchanged, but set to the CFA (i.e. SP + 160 in this case). This is wrong. > There are two possible ways to fix this: > > - Disable the special case of setting SP to CFA on s390. Instead, when > there is no unwind info for SP, just leave it unchanged (just like any other > call-saved register). This solution is implemented in the platform unwinder > (libgcc) and also in GDB. > - Fix the special case of setting SP to CFA by actually taking the CFA bias > into account. This is what this patch implements for LLDB. There seems to be magic happening here. It seems like the CIE or FDE should describe how to unwind the SP when things are tricky. We do have the notion that registers that have no rule can have a default rule applied to them. Currently this is only used for callee saved registers and for any such registers they rule defaults to "the registers is in the register itself". For volatile registers, their default rule is "the register is not available". This is the part where I get fuzzy with respect to how the OS unwinder works: does it have this same notion of default rules for registers? If so, we should be implementing the same thing for LLDB's EH frame parser. So a third solution is to allow the ABI plug-in to specify the default rules for registers when they aren't specified. I think the ABI plug-in is where we get the fact that a register is volatile or not... > I've implemented the second method for LLDB since it actually seemed a bit > more general to me (describes more precisely what actually happens on the > platform), and also it seemed to fit nicely into the ABI scheme. > > If you prefer, I can implement the first method instead. However, since we > cannot unconditionally disable the special case of setting SP to CFA (other > platforms depend on that -- which is arguably the real hack to begin with), > we would still need some ABI method to tell the unwinder whether or not the > special case is needed. > > There's no real way to "fix the unwind info", since this is not a new > platform and binaries containing this unwind info have been in the field for > more than 15 years. In any case, it is not clear that there is anything to > "fix" -- a binary containing no unwind info for SP to indicate that SP does > not change isn't really "wrong" IMO. So I think implementing the default rule for unspecified registers more completely will help us solve this problem correctly. Let me know what you think? http://reviews.llvm.org/D18977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266092 - 'int' is reported as an exception on OS X not as a signal. I don't think
Author: jingham Date: Tue Apr 12 12:04:12 2016 New Revision: 266092 URL: http://llvm.org/viewvc/llvm-project?rev=266092&view=rev Log: 'int' is reported as an exception on OS X not as a signal. I don't think this test ever succeeded on OS X. Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/debugbreak/TestDebugBreak.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/debugbreak/TestDebugBreak.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/debugbreak/TestDebugBreak.py?rev=266092&r1=266091&r2=266092&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/debugbreak/TestDebugBreak.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/debugbreak/TestDebugBreak.py Tue Apr 12 12:04:12 2016 @@ -27,7 +27,7 @@ class DebugBreakTestCase(TestBase): # We've hit the first stop, so grab the frame. self.assertEqual(process.GetState(), lldb.eStateStopped) -stop_reason = lldb.eStopReasonException if (lldbplatformutil.getPlatform()=="windows") else lldb.eStopReasonSignal +stop_reason = lldb.eStopReasonException if (lldbplatformutil.getPlatform()=="windows" or lldbplatformutil.getPlatform()=="macosx") else lldb.eStopReasonSignal thread = lldbutil.get_stopped_thread(process, stop_reason) self.assertIsNotNone(thread, "Unable to find thread stopped at the __debugbreak()") frame = thread.GetFrameAtIndex(0) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18973: Find .plt section in object files generated by recent ld
clayborg added a comment. We can create a new SectionType enumeration in lldb-enumerations.h. Then in ObjectFileELF::CreateSections() we would classify the ".plt" section as eSectionTypeProcedureLinkageTable: static ConstString g_sect_name_plt (".plt"); if (name == g_sect_name_plt) sect_type = eSectionTypeProcedureLinkageTable; Then you can find sections by type no matter what the file format is. This isn't needed for this fix, but if sections contain common information in different file formats, this is handy to avoid having to know to look for sections by name. For instance, DWARF debug info is contained in ".debug_info" and ".debug_abbrev" in ELF, but in MachO the sections are "__debug_info" and "__debug_abbrev". So we made section types for them in the SectionType enumeration and then the DWARF parser just asks the lldb_private::ObjectFile for the sections by type (eSectionTypeDWARFDebugInfo and eSectionTypeDWARFDebugAbbrev). http://reviews.llvm.org/D18973 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266093 - Breakpoint conditions were making result variables, which they should not do.
Author: jingham Date: Tue Apr 12 12:17:35 2016 New Revision: 266093 URL: http://llvm.org/viewvc/llvm-project?rev=266093&view=rev Log: Breakpoint conditions were making result variables, which they should not do. The result variables aren't useful, and if you have a breakpoint on a common function you can generate a lot of these. So I changed the code that checks the condition to set ResultVariableIsInternal in the EvaluateExpressionOptions that we pass to the execution. Unfortunately, the check for this variable was done in the wrong place (the static UserExpression::Evaluate) which is not how breakpoint conditions execute expressions (UserExpression::Execute). So I moved the check to UserExpression::Execute (which Evaluate also calls) and made the overridden method DoExecute. Modified: lldb/trunk/include/lldb/Expression/LLVMUserExpression.h lldb/trunk/include/lldb/Expression/UserExpression.h lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py lldb/trunk/source/Breakpoint/BreakpointLocation.cpp lldb/trunk/source/Expression/LLVMUserExpression.cpp lldb/trunk/source/Expression/UserExpression.cpp lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.cpp lldb/trunk/source/Plugins/ExpressionParser/Go/GoUserExpression.h Modified: lldb/trunk/include/lldb/Expression/LLVMUserExpression.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/LLVMUserExpression.h?rev=266093&r1=266092&r2=266093&view=diff == --- lldb/trunk/include/lldb/Expression/LLVMUserExpression.h (original) +++ lldb/trunk/include/lldb/Expression/LLVMUserExpression.h Tue Apr 12 12:17:35 2016 @@ -40,11 +40,6 @@ public: const EvaluateExpressionOptions &options); ~LLVMUserExpression() override; -lldb::ExpressionResults -Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx, -const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me, -lldb::ExpressionVariableSP &result) override; - bool FinalizeJITExecution(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx, lldb::ExpressionVariableSP &result, @@ -70,6 +65,11 @@ public: lldb::ModuleSP GetJITModule() override; protected: +lldb::ExpressionResults +DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx, + const EvaluateExpressionOptions &options, lldb::UserExpressionSP &shared_ptr_to_me, + lldb::ExpressionVariableSP &result) override; + virtual void ScanContext(ExecutionContext &exe_ctx, lldb_private::Error &err) = 0; Modified: lldb/trunk/include/lldb/Expression/UserExpression.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/UserExpression.h?rev=266093&r1=266092&r2=266093&view=diff == --- lldb/trunk/include/lldb/Expression/UserExpression.h (original) +++ lldb/trunk/include/lldb/Expression/UserExpression.h Tue Apr 12 12:17:35 2016 @@ -107,7 +107,8 @@ public: MatchesContext (ExecutionContext &exe_ctx); //-- -/// Execute the parsed expression +/// Execute the parsed expression by callinng the derived class's +/// DoExecute method. /// /// @param[in] diagnostic_manager /// A diagnostic manager to report errors to. @@ -133,9 +134,9 @@ public: /// @return /// A Process::Execution results value. //-- -virtual lldb::ExpressionResults +lldb::ExpressionResults Execute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options, -lldb::UserExpressionSP &shared_ptr_to_me, lldb::ExpressionVariableSP &result) = 0; +lldb::UserExpressionSP &shared_ptr_to_me, lldb::ExpressionVariableSP &result); //-- /// Apply the side effects of the function to program state. @@ -312,6 +313,10 @@ public: } protected: +virtual lldb::ExpressionResults +DoExecute(DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options, +lldb::UserExpressionSP &shared_ptr_to_me, lldb::ExpressionVariableSP &result) = 0; + static lldb::addr_t GetObjectPointer (lldb::StackFrameSP frame_sp, ConstString &object_name, Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/brea
Re: [Lldb-commits] [PATCH] D18977: Add new ABI callback to return CFA offset
uweigand added a comment. In http://reviews.llvm.org/D18977#398557, @clayborg wrote: > In http://reviews.llvm.org/D18977#398157, @uweigand wrote: > > > In http://reviews.llvm.org/D18977#397626, @clayborg wrote: > > > > > I am not sure why this offset of 160 isn't represented in the unwind > > > info. I guess the OS unwinder that uses the EH frame info just knows that > > > it must adjust the CFA? That seems like a hack. It seems like the unwind > > > info should be fixed and the OS level unwinder should be fixed so that > > > the info can be used correctly without every debugger or tool that has to > > > parse this know about such a hack? Do you have control over the OS and > > > are you able to fix this? That seems like the better fix. > > > > > > The offset of 160 is represented in the unwind info when computing the CFA > > itself. This change is about unwinding the SP register when no explicit > > unwind info for SP is present. In this case, the LLDB unwinder assumes it > > should set SP to the CFA, which is where the problem comes in. > > > This is usually represented by the CIE (Common Information Entry) which is > pointed to from the FDE (Frame Description Entry). The CIE has the initial > state that all FDEs can share. Seems like there should be an entry for the SP > that says the rule to unwind it is "CFA+160"? Well, there could be, just like there could be a default "same register" rule for call-saved registers so we wouldn't have to apply ABI knowledge. However, none of that has been done historically on any DWARF platform I know of, mostly because this would duplicate information known via the ABI into every object file and thus waste space ... In any case, the current state is as it is, we have to handle object files that currently exist. > > Now, on s390, most frames actually have explicit unwind instructions for > > SP, so this special case is not even triggered. Only for leaf functions > > without a stack frame can we ever see the scenario that there are no SP > > unwind instructions. In those cases, the correct result on s390 is to > > simply leave SP unchanged (since the function did not in fact allocate a > > frame). > > > > > > However, due to the special case in the LLDB unwinder, SP is not left > > unchanged, but set to the CFA (i.e. SP + 160 in this case). This is wrong. > > There are two possible ways to fix this: > > > > > > - Disable the special case of setting SP to CFA on s390. Instead, when > > there is no unwind info for SP, just leave it unchanged (just like any > > other call-saved register). This solution is implemented in the platform > > unwinder (libgcc) and also in GDB. > > > - Fix the special case of setting SP to CFA by actually taking the CFA bias > > into account. This is what this patch implements for LLDB. > > > There seems to be magic happening here. It seems like the CIE or FDE should > describe how to unwind the SP when things are tricky. We do have the notion > that registers that have no rule can have a default rule applied to them. > Currently this is only used for callee saved registers and for any such > registers they rule defaults to "the registers is in the register itself". > For volatile registers, their default rule is "the register is not > available". This is the part where I get fuzzy with respect to how the OS > unwinder works: does it have this same notion of default rules for registers? > If so, we should be implementing the same thing for LLDB's EH frame parser. Actually, there are three different default rules: the ones for call-saved and call-clobbered registers you mention, and the special rule for the SP. The platform unwinder does not care about volatile registers (because when you dispatch an exception, the catch point by definition must not care about the contents of those), but it does implement the "same register" rule for call-saved registers, and it does implement the special rule for SP on the platforms that need it. > So a third solution is to allow the ABI plug-in to specify the default rules > for registers when they aren't specified. I think the ABI plug-in is where we > get the fact that a register is volatile or not... Yes, that would be a solution, in fact this is the exact solution I implemented for GDB back in the days ... Right now, if RegisterContextLLDB::SavedLocationForRegister finds no unwind info for a register, it implements (itself) the three default rules, in this order: if it is SP, use the CFA rule; otherwise, if the ABI says the register is volatile, use the "not available rule"; otherwise, say "register not found", which the caller interprets as "same as in the next frame". One option to fix the s390x problem would be to instead of hard-coding the SP rule and the volatile check here, always call an ABI callback to define a "fallback register rule" by returning a UnwindPlan::Row::RegisterLocation. The ABI cou
Re: [Lldb-commits] [lldb] r266033 - Restore the lazy initialization of ScriptInterpreterPython, which was lost as part of the SystemLifetimeManager work
Is there any failure logs I can look at? I am gonna guess the issue is simply that the unit test is assuming Python to be initialized. Which is a bad assumption after my change. Sent from my iPhone > On Apr 12, 2016, at 2:16 AM, Pavel Labath wrote: > > This breaks the python unit tests. Doesn't seem hard to fix, it seems we just > need some way to "really" initialize python before we run the tests. However, > I have reverted this change until that happens to keep to bots green. > > pl > >> On 12 April 2016 at 02:41, Enrico Granata via lldb-commits >> wrote: >> >>> On Apr 11, 2016, at 6:28 PM, Zachary Turner wrote: >>> >>> I have a feeling this breaks something, but I'm not sure what. I remember >>> specifically needing this for some reason. >> >> If you manage to repro the breakage, let’s work on getting it fixed >> I’d like to not have to initialize Python eagerly - most users will probably >> not touch anything Python during debugging, and I have seen a few spin dumps >> connected to the eager initialization during debugging startup >> >>> Did you verify that the gtest suite >> >> No - but I believe the bots run it, so I will probably get yelled at soon if >> I broke something >> >>> as well as the dotest suite >> >> Yes >> >>> and the interactive interpreter all still pass / work? >> >> Yes >> On Mon, Apr 11, 2016 at 6:14 PM Enrico Granata via lldb-commits wrote: Author: enrico Date: Mon Apr 11 20:08:35 2016 New Revision: 266033 URL: http://llvm.org/viewvc/llvm-project?rev=266033&view=rev Log: Restore the lazy initialization of ScriptInterpreterPython, which was lost as part of the SystemLifetimeManager work Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=266033&r1=266032&r2=266033&view=diff == --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp (original) +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Mon Apr 11 20:08:35 2016 @@ -274,8 +274,8 @@ ScriptInterpreterPython::ScriptInterpret m_lock_count(0), m_command_thread_state(nullptr) { -assert(g_initialized && "ScriptInterpreterPython created but InitializePrivate has not been called!"); - +InitializePrivate(); + m_dictionary_name.append("_dict"); StreamString run_string; run_string.Printf ("%s = dict()", m_dictionary_name.c_str()); @@ -330,8 +330,6 @@ ScriptInterpreterPython::Initialize() std::call_once(g_once_flag, []() { -InitializePrivate(); - PluginManager::RegisterPlugin(GetPluginNameStatic(), GetPluginDescriptionStatic(), lldb::eScriptLanguagePython, @@ -3097,7 +3095,9 @@ ScriptInterpreterPython::InitializeInter void ScriptInterpreterPython::InitializePrivate () { -assert(!g_initialized && "ScriptInterpreterPython::InitializePrivate() called more than once!"); +if (g_initialized) +return; + g_initialized = true; Timer scoped_timer (__PRETTY_FUNCTION__, __PRETTY_FUNCTION__); Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h?rev=266033&r1=266032&r2=266033&view=diff == --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h (original) +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h Mon Apr 11 20:08:35 2016 @@ -372,9 +372,6 @@ public: void ResetOutputFileHandle(FILE *new_fh) override; static void -InitializePrivate (); - -static void InitializeInterpreter (SWIGInitCallback python_swig_init_callback, SWIGBreakpointCallbackFunction swig_breakpoint_callback, SWIGWatchpointCallbackFunction swig_watchpoint_callback, @@ -507,6 +504,9 @@ public: }; protected: +static void +InitializePrivate (); + class SynchronicityHandler
Re: [Lldb-commits] [lldb] r266033 - Restore the lazy initialization of ScriptInterpreterPython, which was lost as part of the SystemLifetimeManager work
I think todd did some work to get the gtests working in Xcode. Is that not working? On Tue, Apr 12, 2016 at 11:12 AM Enrico Granata wrote: > Is there any failure logs I can look at? > I am gonna guess the issue is simply that the unit test is assuming Python > to be initialized. Which is a bad assumption after my change. > > Sent from my iPhone > > On Apr 12, 2016, at 2:16 AM, Pavel Labath wrote: > > This breaks the python unit tests. Doesn't seem hard to fix, it seems we > just need some way to "really" initialize python before we run the tests. > However, I have reverted this change until that happens to keep to bots > green. > > pl > > On 12 April 2016 at 02:41, Enrico Granata via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > >> >> On Apr 11, 2016, at 6:28 PM, Zachary Turner wrote: >> >> I have a feeling this breaks something, but I'm not sure what. I remember >> specifically needing this for some reason. >> >> >> If you manage to repro the breakage, let’s work on getting it fixed >> I’d like to not have to initialize Python eagerly - most users will >> probably not touch anything Python during debugging, and I have seen a few >> spin dumps connected to the eager initialization during debugging startup >> >> Did you verify that the gtest suite >> >> >> No - but I believe the bots run it, so I will probably get yelled at soon >> if I broke something >> >> as well as the dotest suite >> >> >> Yes >> >> and the interactive interpreter all still pass / work? >> >> >> Yes >> >> On Mon, Apr 11, 2016 at 6:14 PM Enrico Granata via lldb-commits < >> lldb-commits@lists.llvm.org> wrote: >> >>> Author: enrico >>> Date: Mon Apr 11 20:08:35 2016 >>> New Revision: 266033 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=266033&view=rev >>> Log: >>> Restore the lazy initialization of ScriptInterpreterPython, which was >>> lost as part of the SystemLifetimeManager work >>> >>> >>> Modified: >>> >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >>> >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >>> >>> Modified: >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=266033&r1=266032&r2=266033&view=diff >>> >>> == >>> --- >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >>> (original) >>> +++ >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >>> Mon Apr 11 20:08:35 2016 >>> @@ -274,8 +274,8 @@ ScriptInterpreterPython::ScriptInterpret >>> m_lock_count(0), >>> m_command_thread_state(nullptr) >>> { >>> -assert(g_initialized && "ScriptInterpreterPython created but >>> InitializePrivate has not been called!"); >>> - >>> +InitializePrivate(); >>> + >>> m_dictionary_name.append("_dict"); >>> StreamString run_string; >>> run_string.Printf ("%s = dict()", m_dictionary_name.c_str()); >>> @@ -330,8 +330,6 @@ ScriptInterpreterPython::Initialize() >>> >>> std::call_once(g_once_flag, []() >>> { >>> -InitializePrivate(); >>> - >>> PluginManager::RegisterPlugin(GetPluginNameStatic(), >>>GetPluginDescriptionStatic(), >>>lldb::eScriptLanguagePython, >>> @@ -3097,7 +3095,9 @@ ScriptInterpreterPython::InitializeInter >>> void >>> ScriptInterpreterPython::InitializePrivate () >>> { >>> -assert(!g_initialized && >>> "ScriptInterpreterPython::InitializePrivate() called more than once!"); >>> +if (g_initialized) >>> +return; >>> + >>> g_initialized = true; >>> >>> Timer scoped_timer (__PRETTY_FUNCTION__, __PRETTY_FUNCTION__); >>> >>> Modified: >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h?rev=266033&r1=266032&r2=266033&view=diff >>> >>> == >>> --- >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >>> (original) >>> +++ >>> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >>> Mon Apr 11 20:08:35 2016 >>> @@ -372,9 +372,6 @@ public: >>> void ResetOutputFileHandle(FILE *new_fh) override; >>> >>> static void >>> -InitializePrivate (); >>> - >>> -static void >>> InitializeInterpreter (SWIGInitCallback python_swig_init_callback, >>> SWIGBreakpointCallbackFunction >>> swig_breakpoint_callback, >>> SWIGWatchpointCallbackFunction >>> swig_watchpoint_callback, >>> @@ -507,6 +504,9 @@ public: >>> }; >>> >>> protected: >>> +static voi
Re: [Lldb-commits] [PATCH] D18977: Add new ABI callback to return CFA offset
clayborg added a comment. Yes, that plan sounds great. http://reviews.llvm.org/D18977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r266033 - Restore the lazy initialization of ScriptInterpreterPython, which was lost as part of the SystemLifetimeManager work
Yeah. I ended up tracking it down by hand. I am running one final run of tests and should have a commit going in very soon. Sent from my iPhone > On Apr 12, 2016, at 11:20 AM, Zachary Turner wrote: > > I think todd did some work to get the gtests working in Xcode. Is that not > working? >> On Tue, Apr 12, 2016 at 11:12 AM Enrico Granata wrote: >> Is there any failure logs I can look at? >> I am gonna guess the issue is simply that the unit test is assuming Python >> to be initialized. Which is a bad assumption after my change. >> >> Sent from my iPhone >> >>> On Apr 12, 2016, at 2:16 AM, Pavel Labath wrote: >>> >>> This breaks the python unit tests. Doesn't seem hard to fix, it seems we >>> just need some way to "really" initialize python before we run the tests. >>> However, I have reverted this change until that happens to keep to bots >>> green. >>> >>> pl >>> On 12 April 2016 at 02:41, Enrico Granata via lldb-commits wrote: > On Apr 11, 2016, at 6:28 PM, Zachary Turner wrote: > > I have a feeling this breaks something, but I'm not sure what. I remember > specifically needing this for some reason. If you manage to repro the breakage, let’s work on getting it fixed I’d like to not have to initialize Python eagerly - most users will probably not touch anything Python during debugging, and I have seen a few spin dumps connected to the eager initialization during debugging startup > Did you verify that the gtest suite No - but I believe the bots run it, so I will probably get yelled at soon if I broke something > as well as the dotest suite Yes > and the interactive interpreter all still pass / work? Yes >> On Mon, Apr 11, 2016 at 6:14 PM Enrico Granata via lldb-commits >> wrote: >> Author: enrico >> Date: Mon Apr 11 20:08:35 2016 >> New Revision: 266033 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=266033&view=rev >> Log: >> Restore the lazy initialization of ScriptInterpreterPython, which was >> lost as part of the SystemLifetimeManager work >> >> >> Modified: >> >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >> >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >> >> Modified: >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=266033&r1=266032&r2=266033&view=diff >> == >> --- >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >> (original) >> +++ >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp >> Mon Apr 11 20:08:35 2016 >> @@ -274,8 +274,8 @@ ScriptInterpreterPython::ScriptInterpret >> m_lock_count(0), >> m_command_thread_state(nullptr) >> { >> -assert(g_initialized && "ScriptInterpreterPython created but >> InitializePrivate has not been called!"); >> - >> +InitializePrivate(); >> + >> m_dictionary_name.append("_dict"); >> StreamString run_string; >> run_string.Printf ("%s = dict()", m_dictionary_name.c_str()); >> @@ -330,8 +330,6 @@ ScriptInterpreterPython::Initialize() >> >> std::call_once(g_once_flag, []() >> { >> -InitializePrivate(); >> - >> PluginManager::RegisterPlugin(GetPluginNameStatic(), >>GetPluginDescriptionStatic(), >>lldb::eScriptLanguagePython, >> @@ -3097,7 +3095,9 @@ ScriptInterpreterPython::InitializeInter >> void >> ScriptInterpreterPython::InitializePrivate () >> { >> -assert(!g_initialized && >> "ScriptInterpreterPython::InitializePrivate() called more than once!"); >> +if (g_initialized) >> +return; >> + >> g_initialized = true; >> >> Timer scoped_timer (__PRETTY_FUNCTION__, __PRETTY_FUNCTION__); >> >> Modified: >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >> URL: >> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h?rev=266033&r1=266032&r2=266033&view=diff >> == >> --- >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >> (original) >> +++ >> lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h >> Mon Apr 11 20:08:35 2016 >> @@ -372,9 +372,6 @
[Lldb-commits] [lldb] r266103 - Initialize the Python script interpreter lazily (i.e. not at debugger startup)
Author: enrico Date: Tue Apr 12 13:23:18 2016 New Revision: 266103 URL: http://llvm.org/viewvc/llvm-project?rev=266103&view=rev Log: Initialize the Python script interpreter lazily (i.e. not at debugger startup) This time it should also pass the gtests Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp lldb/trunk/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=266103&r1=266102&r2=266103&view=diff == --- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp (original) +++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Tue Apr 12 13:23:18 2016 @@ -274,7 +274,7 @@ ScriptInterpreterPython::ScriptInterpret m_lock_count(0), m_command_thread_state(nullptr) { -assert(g_initialized && "ScriptInterpreterPython created but InitializePrivate has not been called!"); +InitializePrivate(); m_dictionary_name.append("_dict"); StreamString run_string; @@ -330,8 +330,6 @@ ScriptInterpreterPython::Initialize() std::call_once(g_once_flag, []() { -InitializePrivate(); - PluginManager::RegisterPlugin(GetPluginNameStatic(), GetPluginDescriptionStatic(), lldb::eScriptLanguagePython, @@ -3097,7 +3095,9 @@ ScriptInterpreterPython::InitializeInter void ScriptInterpreterPython::InitializePrivate () { -assert(!g_initialized && "ScriptInterpreterPython::InitializePrivate() called more than once!"); +if (g_initialized) +return; + g_initialized = true; Timer scoped_timer (__PRETTY_FUNCTION__, __PRETTY_FUNCTION__); Modified: lldb/trunk/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp?rev=266103&r1=266102&r2=266103&view=diff == --- lldb/trunk/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp (original) +++ lldb/trunk/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp Tue Apr 12 13:23:18 2016 @@ -24,6 +24,7 @@ PythonTestSuite::SetUp() // ScriptInterpreterPython::Initialize() depends on HostInfo being // initializedso it can compute the python directory etc. ScriptInterpreterPython::Initialize(); +ScriptInterpreterPython::InitializePrivate(); // Although we don't care about concurrency for the purposes of running // this test suite, Python requires the GIL to be locked even for ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18978: Support Linux on SystemZ as platform
uweigand added a comment. In http://reviews.llvm.org/D18978#398180, @labath wrote: > Could you also add a core file test to `TestLinuxCore`? It should be a matter > of running `make_core.sh`, saving the files and creating a new test function > in the file (I've tried to make it simple, if you see room for improvement, > then let me know). > > I think this is especially important as probably noone here has access to > this hardware, so this will enable the rest of the developers to run at least > a basic sanity check on their changes. Yes, that seems to work fine. I'll add those files with the next update. > We generally expectedFailure for things which we consider an lldb bug, and > skip for cases when the test simply does not apply. Platform not having > enough watchpoints sounds like the latter case. I see you were simply copying > the mips case (which does not follow this either), and it doesn't really > matter to me, but I just wanted to make you aware of that. So I guess my thought was that in theory, there may be ways to support multiple watchpoints. In particular, while the hardware only supports a single watchpoint, this may span an address range of arbitrary length. So it may be possible to implement two or more watchpoints by registering a watchpoint range with the hardware that spans all the areas that need to be watched. However, when the hardware then reports a watchpoint hit, we'd have to find out which of the original watchpoints is affected, and ignore cases where we get false positives due to hits elsewhere in the range. This is in fact implemented in GDB, so it should be possible. But an LLDB implementation of this idea seems a bit more complex, and certainly not something I wanted to include in the initial submission ... http://reviews.llvm.org/D18978 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266118 - Revert to using libdispatch to reap threads on MacOSX. Code was accidentally checked in that is now reverted.
Author: gclayton Date: Tue Apr 12 15:26:41 2016 New Revision: 266118 URL: http://llvm.org/viewvc/llvm-project?rev=266118&view=rev Log: Revert to using libdispatch to reap threads on MacOSX. Code was accidentally checked in that is now reverted. Modified: lldb/trunk/source/Host/macosx/Host.mm Modified: lldb/trunk/source/Host/macosx/Host.mm URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/Host.mm?rev=266118&r1=266117&r2=266118&view=diff == --- lldb/trunk/source/Host/macosx/Host.mm (original) +++ lldb/trunk/source/Host/macosx/Host.mm Tue Apr 12 15:26:41 2016 @@ -1448,8 +1448,6 @@ Host::ShellExpandArguments (ProcessLaunc return error; } -#include - HostThread Host::StartMonitoringChildProcess(Host::MonitorChildProcessCallback callback, void *callback_baton, lldb::pid_t pid, bool monitor_signals) { @@ -1460,20 +1458,30 @@ Host::StartMonitoringChildProcess(Host:: Log *log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_HOST | LIBLLDB_LOG_PROCESS)); +dispatch_source_t source = ::dispatch_source_create (DISPATCH_SOURCE_TYPE_PROC, + pid, + mask, + ::dispatch_get_global_queue (DISPATCH_QUEUE_PRIORITY_DEFAULT,0)); + if (log) -log->Printf ("Host::StartMonitoringChildProcess (callback=%p, baton=%p, pid=%i, monitor_signals=%i)\n", - callback, - callback_baton, - (int)pid, - monitor_signals); - -std::thread GrimReaper([callback, pid, log, callback_baton]() { -int status= 0; -int wait_pid = 0; -bool cancel = false; -bool exited = false; -while(!exited) -{ +log->Printf ("Host::StartMonitoringChildProcess (callback=%p, baton=%p, pid=%i, monitor_signals=%i) source = %p\n", + callback, + callback_baton, + (int)pid, + monitor_signals, + source); + +if (source) +{ +::dispatch_source_set_cancel_handler (source, ^{ +::dispatch_release (source); +}); +::dispatch_source_set_event_handler (source, ^{ + +int status= 0; +int wait_pid = 0; +bool cancel = false; +bool exited = false; do { wait_pid = ::waitpid (pid, &status, 0); @@ -1515,15 +1523,19 @@ Host::StartMonitoringChildProcess(Host:: status_cstr, signal, exit_status); - + if (callback) cancel = callback (callback_baton, pid, exited, signal, exit_status); + +if (exited || cancel) +{ +::dispatch_source_cancel(source); +} } -} -}); - -GrimReaper.detach(); - +}); + +::dispatch_resume (source); +} return HostThread(); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266129 - Cleanup the arguments for 'memory find' such that the help system reflects the real way to invoke it
Author: enrico Date: Tue Apr 12 16:26:48 2016 New Revision: 266129 URL: http://llvm.org/viewvc/llvm-project?rev=266129&view=rev Log: Cleanup the arguments for 'memory find' such that the help system reflects the real way to invoke it Modified: lldb/trunk/source/Commands/CommandObjectMemory.cpp Modified: lldb/trunk/source/Commands/CommandObjectMemory.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectMemory.cpp?rev=266129&r1=266128&r2=266129&view=diff == --- lldb/trunk/source/Commands/CommandObjectMemory.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectMemory.cpp Tue Apr 12 16:26:48 2016 @@ -955,10 +955,10 @@ protected: OptionDefinition g_memory_find_option_table[] = { -{ LLDB_OPT_SET_1, false, "expression", 'e', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeExpression, "Evaluate an expression to obtain a byte pattern."}, -{ LLDB_OPT_SET_2, false, "string", 's', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeName, "Use text to find a byte pattern."}, -{ LLDB_OPT_SET_1|LLDB_OPT_SET_2, false, "count", 'c', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeCount, "How many times to perform the search."}, -{ LLDB_OPT_SET_1|LLDB_OPT_SET_2, false, "dump-offset", 'o', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeOffset, "When dumping memory for a match, an offset from the match location to start dumping from."}, +{ LLDB_OPT_SET_1, true, "expression", 'e', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeExpression, "Evaluate an expression to obtain a byte pattern."}, +{ LLDB_OPT_SET_2, true, "string", 's', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeName, "Use text to find a byte pattern."}, +{ LLDB_OPT_SET_ALL, false, "count", 'c', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeCount, "How many times to perform the search."}, +{ LLDB_OPT_SET_ALL, false, "dump-offset", 'o', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeOffset, "When dumping memory for a match, an offset from the match location to start dumping from."}, }; //-- @@ -1072,7 +1072,7 @@ public: m_arguments.push_back (arg1); m_arguments.push_back (arg2); -m_option_group.Append (&m_memory_options, LLDB_OPT_SET_ALL, LLDB_OPT_SET_2); +m_option_group.Append (&m_memory_options); m_option_group.Finalize(); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266138 - Use the FormatEntity work for great good - parse summary strings before accepting them, and fail to add any strings that fail parsing
Author: enrico Date: Tue Apr 12 16:57:11 2016 New Revision: 266138 URL: http://llvm.org/viewvc/llvm-project?rev=266138&view=rev Log: Use the FormatEntity work for great good - parse summary strings before accepting them, and fail to add any strings that fail parsing Modified: lldb/trunk/source/Commands/CommandObjectType.cpp Modified: lldb/trunk/source/Commands/CommandObjectType.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectType.cpp?rev=266138&r1=266137&r2=266138&view=diff == --- lldb/trunk/source/Commands/CommandObjectType.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectType.cpp Tue Apr 12 16:57:11 2016 @@ -1718,20 +1718,23 @@ CommandObjectTypeSummaryAdd::Execute_Str return false; } -Error error; - -lldb::TypeSummaryImplSP entry(new StringSummaryFormat(m_options.m_flags, -format_cstr)); - -if (error.Fail()) +std::unique_ptr string_format(new StringSummaryFormat(m_options.m_flags, format_cstr)); +if (!string_format) { -result.AppendError(error.AsCString()); +result.AppendError("summary creation failed"); result.SetStatus(eReturnStatusFailed); return false; } +if (string_format->m_error.Fail()) +{ +result.AppendErrorWithFormat("syntax error: %s", string_format->m_error.AsCString("")); +result.SetStatus(eReturnStatusFailed); +return false; +} +lldb::TypeSummaryImplSP entry(string_format.release()); // now I have a valid format, let's add it to every type - +Error error; for (size_t i = 0; i < argc; i++) { const char* typeA = command.GetArgumentAtIndex(i); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266139 - Fixed being able to set breakpoints on destructors when we don't fully specify the demangled name. So all of the following now work:
Author: gclayton Date: Tue Apr 12 17:02:37 2016 New Revision: 266139 URL: http://llvm.org/viewvc/llvm-project?rev=266139&view=rev Log: Fixed being able to set breakpoints on destructors when we don't fully specify the demangled name. So all of the following now work: (lldb) b ~Foo (lldb) b Foo::~Foo (lldb) b Bar::Foo::~Foo Improved out C++ breakpoint locations tests as well to cover this issue. Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py?rev=266139&r1=266138&r2=266139&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py Tue Apr 12 17:02:37 2016 @@ -48,15 +48,18 @@ class TestCPPBreakpointLocations(TestBas { 'name' : 'func1', 'loc_names' : [ 'a::c::func1()', 'b::c::func1()'] }, { 'name' : 'func2', 'loc_names' : [ 'a::c::func2()', 'c::d::func2()'] }, { 'name' : 'func3', 'loc_names' : [ 'a::c::func3()', 'b::c::func3()', 'c::d::func3()'] }, +{ 'name' : '~c', 'loc_names' : [ 'a::c::~c()', 'b::c::~c()', 'a::c::~c()', 'b::c::~c()'] }, { 'name' : 'c::func1', 'loc_names' : [ 'a::c::func1()', 'b::c::func1()'] }, { 'name' : 'c::func2', 'loc_names' : [ 'a::c::func2()'] }, { 'name' : 'c::func3', 'loc_names' : [ 'a::c::func3()', 'b::c::func3()'] }, +{ 'name' : 'c::~c', 'loc_names' : [ 'a::c::~c()', 'b::c::~c()', 'a::c::~c()', 'b::c::~c()'] }, { 'name' : 'a::c::func1', 'loc_names' : [ 'a::c::func1()'] }, { 'name' : 'b::c::func1', 'loc_names' : [ 'b::c::func1()'] }, { 'name' : 'c::d::func2', 'loc_names' : [ 'c::d::func2()'] }, { 'name' : 'a::c::func1()', 'loc_names' : [ 'a::c::func1()'] }, { 'name' : 'b::c::func1()', 'loc_names' : [ 'b::c::func1()'] }, { 'name' : 'c::d::func2()', 'loc_names' : [ 'c::d::func2()'] }, +{ 'name' : 'c::~c()', 'loc_names' : [ 'a::c::~c()', 'b::c::~c()', 'a::c::~c()', 'b::c::~c()'] }, ] for bp_dict in bp_dicts: 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=266139&r1=266138&r2=266139&view=diff == --- lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp (original) +++ lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp Tue Apr 12 17:02:37 2016 @@ -330,7 +330,7 @@ CPlusPlusLanguage::IsCPPMangledName (con bool CPlusPlusLanguage::ExtractContextAndIdentifier (const char *name, llvm::StringRef &context, llvm::StringRef &identifier) { -static RegularExpression g_basename_regex("^(([A-Za-z_][A-Za-z_0-9]*::)*)([A-Za-z_][A-Za-z_0-9]*)$"); +static RegularExpression g_basename_regex("^(([A-Za-z_][A-Za-z_0-9]*::)*)(~?[A-Za-z_~][A-Za-z_0-9]*)$"); RegularExpression::Match match(4); if (g_basename_regex.Execute (name, &match)) { ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D19035: Fix breakpoint_set_restart test for Windows
amccarth created this revision. amccarth added reviewers: ovyalov, labath. amccarth added a subscriber: lldb-commits. When run with the multiprocess test runner, the getchar() trick doesn't work, so `ninja check-lldb` would fail on this test, but running the test directly worked fine. This replaces the getchar() call with an infinite loop. Thanks to Pavel for the insight. http://reviews.llvm.org/D19035 Files: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp Index: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp === --- packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp +++ packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp @@ -7,12 +7,19 @@ // //===--===// +#include #include #include +#include + int main(int argc, char const *argv[]) { -getchar(); +static bool done = false; +while (!done) +{ + std::this_thread::sleep_for(std::chrono::milliseconds{100}); +} printf("Set a breakpoint here.\n"); return 0; } Index: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp === --- packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp +++ packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp @@ -7,12 +7,19 @@ // //===--===// +#include #include #include +#include + int main(int argc, char const *argv[]) { -getchar(); +static bool done = false; +while (!done) +{ + std::this_thread::sleep_for(std::chrono::milliseconds{100}); +} printf("Set a breakpoint here.\n"); return 0; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19035: Fix breakpoint_set_restart test for Windows
ovyalov accepted this revision. This revision is now accepted and ready to land. Comment at: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp:11 @@ -10,2 +10,3 @@ +#include #include #include You may remove iostream since getchar is no longer needed. http://reviews.llvm.org/D19035 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19035: Fix breakpoint_set_restart test for Windows
amccarth marked an inline comment as done. Comment at: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp:11 @@ -10,2 +10,3 @@ +#include #include #include ovyalov wrote: > You may remove iostream since getchar is no longer needed. Done since getchar actually comes from stdio.h anyway. http://reviews.llvm.org/D19035 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19035: Fix breakpoint_set_restart test for Windows
This revision was automatically updated to reflect the committed changes. amccarth marked an inline comment as done. Closed by commit rL266145: Fix breakpoint_set_restart test for Windows (authored by amccarth). Changed prior to commit: http://reviews.llvm.org/D19035?vs=53472&id=53482#toc Repository: rL LLVM http://reviews.llvm.org/D19035 Files: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp === --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp @@ -7,12 +7,18 @@ // //===--===// -#include +#include #include +#include + int main(int argc, char const *argv[]) { -getchar(); +static bool done = false; +while (!done) +{ + std::this_thread::sleep_for(std::chrono::milliseconds{100}); +} printf("Set a breakpoint here.\n"); return 0; } Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp === --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp @@ -7,12 +7,18 @@ // //===--===// -#include +#include #include +#include + int main(int argc, char const *argv[]) { -getchar(); +static bool done = false; +while (!done) +{ + std::this_thread::sleep_for(std::chrono::milliseconds{100}); +} printf("Set a breakpoint here.\n"); return 0; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266145 - Fix breakpoint_set_restart test for Windows
Author: amccarth Date: Tue Apr 12 17:45:03 2016 New Revision: 266145 URL: http://llvm.org/viewvc/llvm-project?rev=266145&view=rev Log: Fix breakpoint_set_restart test for Windows When run with the multiprocess test runner, the getchar() trick doesn't work, so ninja check-lldb would fail on this test, but running the test directly worked fine. Differential Revision: http://reviews.llvm.org/D19035 Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp?rev=266145&r1=266144&r2=266145&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp Tue Apr 12 17:45:03 2016 @@ -7,12 +7,18 @@ // //===--===// -#include +#include #include +#include + int main(int argc, char const *argv[]) { -getchar(); +static bool done = false; +while (!done) +{ + std::this_thread::sleep_for(std::chrono::milliseconds{100}); +} printf("Set a breakpoint here.\n"); return 0; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18984: Fix ARM instruction emulation tests on big-endian systems
uweigand updated this revision to Diff 53500. http://reviews.llvm.org/D18984 Files: source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp source/Plugins/Instruction/ARM/EmulationStateARM.cpp source/Plugins/Instruction/ARM/EmulationStateARM.h Index: source/Plugins/Instruction/ARM/EmulationStateARM.h === --- source/Plugins/Instruction/ARM/EmulationStateARM.h +++ source/Plugins/Instruction/ARM/EmulationStateARM.h @@ -30,10 +30,10 @@ ReadPseudoRegisterValue (uint32_t reg_num, bool &success); bool -StoreToPseudoAddress (lldb::addr_t p_address, uint64_t value, uint32_t size); +StoreToPseudoAddress (lldb::addr_t p_address, uint32_t value); uint32_t -ReadFromPseudoAddress (lldb::addr_t p_address, uint32_t size, bool &success); +ReadFromPseudoAddress (lldb::addr_t p_address, bool &success); void ClearPseudoRegisters (); @@ -82,11 +82,7 @@ uint32_t m_gpr[17]; struct _sd_regs { -union -{ -uint32_t s_reg[2]; -uint64_t d_reg; -} sd_regs[16]; // sregs 0 - 31 & dregs 0 - 15 +uint32_t s_regs[32]; // sregs 0 - 31 & dregs 0 - 15 uint64_t d_regs[16]; // dregs 16-31 Index: source/Plugins/Instruction/ARM/EmulationStateARM.cpp === --- source/Plugins/Instruction/ARM/EmulationStateARM.cpp +++ source/Plugins/Instruction/ARM/EmulationStateARM.cpp @@ -61,11 +61,15 @@ if (reg_ctx->ReadRegister (reg_info, reg_value)) { +uint64_t value = reg_value.GetAsUInt64(); uint32_t idx = i - dwarf_d0; if (i < 16) -m_vfp_regs.sd_regs[idx].d_reg = reg_value.GetAsUInt64(); +{ +m_vfp_regs.s_regs[idx * 2] = (uint32_t)value; +m_vfp_regs.s_regs[idx * 2 + 1] = (uint32_t)(value >> 32); +} else -m_vfp_regs.d_regs[idx - 16] = reg_value.GetAsUInt64(); +m_vfp_regs.d_regs[idx - 16] = value; } else success = false; @@ -82,16 +86,18 @@ else if ((dwarf_s0 <= reg_num) && (reg_num <= dwarf_s31)) { uint32_t idx = reg_num - dwarf_s0; -m_vfp_regs.sd_regs[idx / 2].s_reg[idx % 2] = (uint32_t) value; +m_vfp_regs.s_regs[idx] = (uint32_t)value; } else if ((dwarf_d0 <= reg_num) && (reg_num <= dwarf_d31)) { -if ((reg_num - dwarf_d0) < 16) +uint32_t idx = reg_num - dwarf_d0; +if (idx < 16) { -m_vfp_regs.sd_regs[reg_num - dwarf_d0].d_reg = value; +m_vfp_regs.s_regs[idx * 2] = (uint32_t)value; +m_vfp_regs.s_regs[idx * 2 + 1] = (uint32_t)(value >> 32); } else -m_vfp_regs.d_regs[reg_num - dwarf_d16] = value; +m_vfp_regs.d_regs[idx - 16] = value; } else return false; @@ -110,14 +116,15 @@ else if ((dwarf_s0 <= reg_num) && (reg_num <= dwarf_s31)) { uint32_t idx = reg_num - dwarf_s0; -value = m_vfp_regs.sd_regs[idx / 2].s_reg[idx % 2]; +value = m_vfp_regs.d_regs[idx]; } else if ((dwarf_d0 <= reg_num) && (reg_num <= dwarf_d31)) { -if ((reg_num - dwarf_d0) < 16) -value = m_vfp_regs.sd_regs[reg_num - dwarf_d0].d_reg; +uint32_t idx = reg_num - dwarf_d0; +if (idx < 16) +value = (uint64_t)m_vfp_regs.s_regs[idx * 2] | ((uint64_t)m_vfp_regs.s_regs[idx * 2 + 1] >> 32); else -value = m_vfp_regs.d_regs[reg_num - dwarf_d16]; +value = m_vfp_regs.d_regs[idx - 16]; } else success = false; @@ -131,8 +138,8 @@ for (int i = 0; i < 17; ++i) m_gpr[i] = 0; -for (int i = 0; i < 16; ++i) -m_vfp_regs.sd_regs[i].d_reg = 0; +for (int i = 0; i < 32; ++i) +m_vfp_regs.s_regs[i] = 0; for (int i = 0; i < 16; ++i) m_vfp_regs.d_regs[i] = 0; @@ -145,23 +152,14 @@ } bool -EmulationStateARM::StoreToPseudoAddress (lldb::addr_t p_address, uint64_t value, uint32_t size) +EmulationStateARM::StoreToPseudoAddress (lldb::addr_t p_address, uint32_t value) { -if (size > 8) -return false; - -if (size <= 4) -m_memory[p_address] = value; -else if (size == 8) -{ -m_memory[p_address] = (value << 32) >> 32; -m_memory[p_address + 4] = value << 32; -} +m_memory[p_address] = value; return true; } uint32_t -EmulationStateARM::ReadFromPseudoAddress (lldb::addr_t p_address, uint32_t size, bool &success) +EmulationStateARM::ReadFromPseudoAddress (lldb::addr_t p_address, bool &success) { std::map::iterator pos; uint32_t ret_val = 0; @@ -191,25 +189,31 @@ EmulationStateARM *pseudo_state = (EmulationStateARM *) baton; if (length <= 4) { -uint32_t value
Re: [Lldb-commits] [PATCH] D18984: Fix ARM instruction emulation tests on big-endian systems
uweigand added a comment. In http://reviews.llvm.org/D18984#398486, @tberghammer wrote: > Generally looks good with 2 minor comment inline. I also run the test suite > on Android ARM (little endian) and everything looked fine Thanks for the review and test! Comment at: source/Plugins/Instruction/ARM/EmulationStateARM.cpp:127 @@ -119,3 +126,3 @@ else -value = m_vfp_regs.d_regs[reg_num - dwarf_d16]; +value = m_vfp_regs.d_regs[idx - 16]; } Good catch! Now fixed. Comment at: source/Plugins/Instruction/ARM/EmulationStateARM.cpp:157 @@ -149,12 +156,3 @@ { -if (size > 8) -return false; - -if (size <= 4) -m_memory[p_address] = value; -else if (size == 8) -{ -m_memory[p_address] = (value << 32) >> 32; -m_memory[p_address + 4] = value << 32; -} +m_memory[p_address] = value; return true; Right, that was my intention, just forgot to actually do it ... Now fixed. http://reviews.llvm.org/D18984 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18985: Fix test cases for big-endian systems
uweigand updated the summary for this revision. uweigand updated this revision to Diff 53501. http://reviews.llvm.org/D18985 Files: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-python-synth/main.cpp packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-smart-array/TestDataFormatterSmartArray.py packages/Python/lldbsuite/test/functionalities/data-formatter/synthcapping/TestSyntheticCapping.py packages/Python/lldbsuite/test/functionalities/data-formatter/synthcapping/main.cpp packages/Python/lldbsuite/test/functionalities/memory/cache/TestMemoryCache.py packages/Python/lldbsuite/test/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py packages/Python/lldbsuite/test/python_api/process/TestProcessAPI.py packages/Python/lldbsuite/test/python_api/sbdata/TestSBData.py packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py Index: packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py === --- packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py +++ packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py @@ -387,7 +387,10 @@ return retval elif endian == 'big': -retval = value.encode("hex") +retval = "" +while value != 0: +retval = "{:02x}".format(value & 0xff) + retval +value = value >> 8 if byte_size: # Add zero-fill to the left/front (MSB side) of the value. retval = ("00" * (byte_size - len(retval)/2)) + retval Index: packages/Python/lldbsuite/test/python_api/sbdata/TestSBData.py === --- packages/Python/lldbsuite/test/python_api/sbdata/TestSBData.py +++ packages/Python/lldbsuite/test/python_api/sbdata/TestSBData.py @@ -186,7 +186,10 @@ self.assertTrue(new_object.GetValue() == "1", 'new_object == 1') -data.SetData(error, 'A\0\0\0', data.GetByteOrder(), data.GetAddressByteSize()) +if data.GetByteOrder() == lldb.eByteOrderBig: +data.SetData(error, '\0\0\0A', data.GetByteOrder(), data.GetAddressByteSize()) +else: +data.SetData(error, 'A\0\0\0', data.GetByteOrder(), data.GetAddressByteSize()) self.assertTrue(error.Success()) data2 = lldb.SBData() @@ -311,8 +314,8 @@ self.assert_data(data2.GetSignedInt32, 4, -2) data2.SetDataFromSInt64Array([2, -2]) -self.assert_data(data2.GetSignedInt32, 0, 2) -self.assert_data(data2.GetSignedInt32, 8, -2) +self.assert_data(data2.GetSignedInt64, 0, 2) +self.assert_data(data2.GetSignedInt64, 8, -2) data2.SetDataFromUInt32Array([1,2,3,4,5]) self.assert_data(data2.GetUnsignedInt32, 0, 1) Index: packages/Python/lldbsuite/test/python_api/process/TestProcessAPI.py === --- packages/Python/lldbsuite/test/python_api/process/TestProcessAPI.py +++ packages/Python/lldbsuite/test/python_api/process/TestProcessAPI.py @@ -232,6 +232,7 @@ # The bytearray_to_int utility function expects a little endian bytearray. if byteOrder == lldb.eByteOrderBig: +content = bytearray(content, 'ascii') content.reverse() new_value = bytearray_to_int(content, byteSize) Index: packages/Python/lldbsuite/test/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py === --- packages/Python/lldbsuite/test/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py +++ packages/Python/lldbsuite/test/lang/cpp/frame-var-anon-unions/TestFrameVariableAnonymousUnions.py @@ -19,7 +19,13 @@ self.runCmd("process launch", RUN_SUCCEEDED) -self.expect('frame variable -f x i', substrs=['ff41']) +process = self.dbg.GetSelectedTarget().GetProcess() + +if process.GetByteOrder() == lldb.eByteOrderLittle: +self.expect('frame variable -f x i', substrs=['ff41']) +else: +self.expect('frame variable -f x i', substrs=['4100']) + self.expect('frame variable c', substrs=["'A"]) self.expect('frame variable x', matching=False, substrs=['3']) Index: packages/Python/lldbsuite/test/functionalities/memory/cache/TestMemoryCache.py === --- packages/Python/lldbsuite/test/functionalities/memory/cache/TestMemoryCache.py +++ packages/Python/lldbsuite/test/functionalities/memory/cache/TestMemoryCache.py @@ -51,7 +51,7 @@ self.assertTrue(0x0042 == int(line.split(':')[1], 0)) # Change the value o
Re: [Lldb-commits] [PATCH] D18985: Fix test cases for big-endian systems
uweigand added a comment. In http://reviews.llvm.org/D18985#397674, @clayborg wrote: > So many tests above are going to accept either a little endian or big endian > value. This will make most of these tests useless since if a little endian > machine fails with a big endian number we won't catch it and vice versa. So > we need to expect the correct value for little endian and a different but > correct one for big endian tests and only accept the correct one. The new version should address this. Each test now only accepts either the little-endian or big-endian result, depending on the target byte order. > Zero seems like a bad alternate value as it could cover up a real failure. > Can we improve this test so that we are testing for actual values? Or can we > change the test by endianness and test for 16777216 for little endian and > something else that is not zero for big endian? We don't want zero to be > valid for little endian tests. I've changed the test source value to use values such that none of the data elements tested by the test case is ever zero, neither on little-endian nor on big-endian. Tested on both System z and Intel to verify it still works with either byte order. http://reviews.llvm.org/D18985 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18977: Add new ABI callback to provide fallback unwind register locations
uweigand retitled this revision from "Add new ABI callback to return CFA offset" to "Add new ABI callback to provide fallback unwind register locations". uweigand updated the summary for this revision. uweigand updated this revision to Diff 53502. http://reviews.llvm.org/D18977 Files: include/lldb/Symbol/UnwindPlan.h include/lldb/Target/ABI.h source/Plugins/Process/Utility/RegisterContextLLDB.cpp source/Target/ABI.cpp Index: source/Target/ABI.cpp === --- source/Target/ABI.cpp +++ source/Target/ABI.cpp @@ -205,3 +205,27 @@ assert( !"Should never get here!" ); return false; } + +bool +ABI::GetFallbackRegisterLocation (const RegisterInfo *reg_info, + UnwindPlan::Row::RegisterLocation &unwind_regloc) +{ +// Did the UnwindPlan fail to give us the caller's stack pointer? +// The stack pointer is defined to be the same as THIS frame's CFA, so return the CFA value as +// the caller's stack pointer. This is true on x86-32/x86-64 at least. +if (reg_info->kinds[eRegisterKindGeneric] == LLDB_REGNUM_GENERIC_SP) +{ +unwind_regloc.SetIsCFAPlusOffset(0); +return true; +} + +// If a volatile register is being requested, we don't want to forward the next frame's register contents +// up the stack -- the register is not retrievable at this frame. +if (RegisterIsVolatile(reg_info)) +{ +unwind_regloc.SetUndefined(); +return true; +} + +return false; +} Index: source/Plugins/Process/Utility/RegisterContextLLDB.cpp === --- source/Plugins/Process/Utility/RegisterContextLLDB.cpp +++ source/Plugins/Process/Utility/RegisterContextLLDB.cpp @@ -1390,45 +1390,28 @@ } } -if (have_unwindplan_regloc == false) -{ -// Did the UnwindPlan fail to give us the caller's stack pointer? -// The stack pointer is defined to be the same as THIS frame's CFA, so return the CFA value as -// the caller's stack pointer. This is true on x86-32/x86-64 at least. - -RegisterNumber sp_regnum (m_thread, eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP); -if (sp_regnum.GetAsKind (eRegisterKindLLDB) != LLDB_INVALID_REGNUM -&& sp_regnum.GetAsKind (eRegisterKindLLDB) == regnum.GetAsKind (eRegisterKindLLDB)) -{ -// make sure we won't lose precision copying an addr_t (m_cfa) into a uint64_t (.inferred_value) -assert (sizeof (addr_t) <= sizeof (uint64_t)); -regloc.type = UnwindLLDB::RegisterLocation::eRegisterValueInferred; -regloc.location.inferred_value = m_cfa; -m_registers[regnum.GetAsKind (eRegisterKindLLDB)] = regloc; -UnwindLogMsg ("supplying caller's stack pointer %s (%d) value, computed from CFA", -regnum.GetName(), regnum.GetAsKind (eRegisterKindLLDB)); -return UnwindLLDB::RegisterSearchResult::eRegisterFound; -} -} - ExecutionContext exe_ctx(m_thread.shared_from_this()); Process *process = exe_ctx.GetProcessPtr(); if (have_unwindplan_regloc == false) { -// If a volatile register is being requested, we don't want to forward the next frame's register contents -// up the stack -- the register is not retrievable at this frame. +// If the UnwindPlan failed to give us an unwind location for this register, we may be able to fall back +// to some ABI-defined default. For example, some ABIs allow to determine the caller's SP via the CFA. +// Also, the ABI may set volatile registers to the undefined state. ABI *abi = process ? process->GetABI().get() : NULL; if (abi) { const RegisterInfo *reg_info = GetRegisterInfoAtIndex(regnum.GetAsKind (eRegisterKindLLDB)); -if (reg_info && abi->RegisterIsVolatile (reg_info)) +if (reg_info && abi->GetFallbackRegisterLocation (reg_info, unwindplan_regloc)) { -UnwindLogMsg ("did not supply reg location for %s (%d) because it is volatile", +UnwindLogMsg ("supplying caller's saved %s (%d)'s location using ABI default", regnum.GetName(), regnum.GetAsKind (eRegisterKindLLDB)); -return UnwindLLDB::RegisterSearchResult::eRegisterIsVolatile; +have_unwindplan_regloc = true; } } +} +if (have_unwindplan_regloc == false) +{ if (IsFrameZero ()) { // This is frame 0 - we should return the actual live register context value @@ -1468,6 +1451,13 @@ return UnwindLLDB::RegisterSearchResult::eRegisterNotFound; } +if (unwindplan_regloc.IsUndefined()) +{ + UnwindLogMsg ("did not supply reg location for %s (%d) because it is volatile", + regnum.G
Re: [Lldb-commits] [PATCH] D18977: Add new ABI callback to provide fallback unwind register locations
uweigand added a comment. OK, here's an implementation of the new approach. In addition to the changes discussed, I had to add handling of undefined register locations -- this might now theoretically also trigger when CFI contains an explicit DW_CFA_undefined, but it should actually do the right thing then as well. Everything still works on SystemZ using this approach, and I've also tested on Intel with no regressions. http://reviews.llvm.org/D18977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18977: Add new ABI callback to provide fallback unwind register locations
jasonmolenda added a comment. Thanks for doing this work Ulrich, the assumption that SP==CFA was mine, I should have put this in the ABI from the start. I worry a little about the change to RegisterContextLLDB::SavedLocationForRegister() where you've moved the ABI is-volatile check into ABI::GetFallbackRegisterLocation. If this function can't find an unwind rule for a non-volatile register, it will return UnwindLLDB::RegisterSearchResult::eRegisterNotFound and UnwindLLDB will continue searching for a definition down the stack. You're changing the call to ABI::RegisterIsVolatile to unwindplan_regloc.IsUndefined(), assuming that GetFallbackRegisterLocation set it to undefined. But could a register have an undefined state from the eh_frame rules? I'm not sure what that would indicate - but presumably this function doesn't use that register and lldb should continue its search. I may not be correct about this point - but what do you think about this possibility? http://reviews.llvm.org/D18977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18977: Add new ABI callback to provide fallback unwind register locations
jasonmolenda added a comment. Just to be clear, my concern is we get an register location which is "undefined" (I've never seen it, I don't know what it means) and now SavedLocationForRegister() will say "this register is volatile, not looking any further" when it used to say "register not found in this stack frame" and the unwinder would keep looking down the stack for a save location. http://reviews.llvm.org/D18977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r266164 - Attempt to fix TestCPPBreakpointLocations on Linux/Android.
Author: ovyalov Date: Tue Apr 12 23:21:05 2016 New Revision: 266164 URL: http://llvm.org/viewvc/llvm-project?rev=266164&view=rev Log: Attempt to fix TestCPPBreakpointLocations on Linux/Android. Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py?rev=266164&r1=266163&r2=266164&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py Tue Apr 12 23:21:05 2016 @@ -26,7 +26,7 @@ class TestCPPBreakpointLocations(TestBas name = bp_dict['name'] names = bp_dict['loc_names'] bp = target.BreakpointCreateByName (name) -self.assertTrue (bp.GetNumLocations() == len(names), "Make sure we find the right number of breakpoint locations") +self.assertTrue (bp.GetNumLocations() <= len(names), "Make sure we find the right number of breakpoint locations") bp_loc_names = list() for bp_loc in bp: ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D19004: Use the section sizes to determine symbols sizes in the Symtab instead of just using the following symbol's address
jasonmolenda updated this revision to Diff 53517. jasonmolenda added a comment. Updated the patch to address Greg's feedback. Repository: rL LLVM http://reviews.llvm.org/D19004 Files: include/lldb/Core/RangeMap.h source/Symbol/Symtab.cpp Index: source/Symbol/Symtab.cpp === --- source/Symbol/Symtab.cpp +++ source/Symbol/Symtab.cpp @@ -894,6 +894,38 @@ addr_t match_offset; } SymbolSearchInfo; +// Add all the section file start address & size to the RangeVector, +// recusively adding any children sections. +static void +AddSectionsToRangeMap (SectionList *sectlist, RangeVector §ion_ranges) +{ +const int num_sections = sectlist->GetNumSections (0); +for (int i = 0; i < num_sections; i++) +{ +SectionSP sect_sp = sectlist->GetSectionAtIndex (i); +if (sect_sp) +{ +SectionList &child_sectlist = sect_sp->GetChildren(); + +// If this section has children, add the children to the RangeVector. +// Else add this section to the RangeVector. +if (child_sectlist.GetNumSections (0) > 0) +{ +AddSectionsToRangeMap (&child_sectlist, section_ranges); +} +else +{ +addr_t base_addr = sect_sp->GetFileAddress(); +size_t size = sect_sp->GetByteSize(); +RangeVector::Entry entry; +entry.SetRangeBase (base_addr); +entry.SetByteSize (size); +section_ranges.Append (entry); +} +} +} +} + void Symtab::InitAddressIndexes() { @@ -915,26 +947,75 @@ m_file_addr_to_index.Append(entry); } } - const size_t num_entries = m_file_addr_to_index.GetSize(); if (num_entries > 0) { m_file_addr_to_index.Sort(); -// Now our last address range might not have had sizes because there -// was no subsequent symbol to calculate the size from. If this is -// the case, then calculate the size by capping it at the end of the -// section in which the symbol resides -lldb::addr_t total_size = 0; -const FileRangeToIndexMap::Entry* entry = m_file_addr_to_index.Back(); -if (entry->GetByteSize() == 0) +// Create a RangeVector with the start & size of all the sections for +// this objfile. We'll need to check this for any FileRangeToIndexMap +// entries with an uninitialized size, which could potentially be a +// large number so reconstituting the weak pointer is busywork when it +// is invariant information. +SectionList *sectlist = m_objfile->GetSectionList(); +RangeVector section_ranges; +if (sectlist) { -const Address& address = m_symbols[entry->data].GetAddressRef(); -if (SectionSP section_sp = address.GetSection()) -total_size = entry->base + section_sp->GetByteSize() - address.GetOffset(); +AddSectionsToRangeMap (sectlist, section_ranges); +section_ranges.Sort(); } -m_file_addr_to_index.CalculateSizesOfZeroByteSizeRanges(total_size); +// Iterate through the FileRangeToIndexMap and fill in the size for any +// entries that didn't already have a size from the Symbol (e.g. if we +// have a plain linker symbol with an address only, instead of debug info +// where we get an address and a size and a type, etc.) +for (int i = 0; i < num_entries; i++) +{ +FileRangeToIndexMap::Entry *entry = m_file_addr_to_index.GetMutableEntryAtIndex (i); +if (entry->GetByteSize() == 0) +{ +addr_t curr_base_addr = entry->GetRangeBase(); +const RangeVector::Entry *containing_section = + section_ranges.FindEntryThatContains (curr_base_addr); + +// Use the end of the section as the default max size of the symbol +addr_t sym_size = 0; +if (containing_section) +{ +sym_size = containing_section->GetByteSize() - +(entry->GetRangeBase() - containing_section->GetRangeBase()); +} + +for (int j = i; j < num_entries; j++) +{ +FileRangeToIndexMap::Entry *next_entry = m_file_addr_to_index.GetMutableEntryAtIndex (j); +addr_t next_base_addr = next_entry->GetRangeBase(); +if (next_base_addr > curr_base_addr) +{ +addr_
[Lldb-commits] [lldb] r266165 - Update Symtab::InitAddressIndexes so that computed symbol sizes
Author: jmolenda Date: Tue Apr 12 23:32:49 2016 New Revision: 266165 URL: http://llvm.org/viewvc/llvm-project?rev=266165&view=rev Log: Update Symtab::InitAddressIndexes so that computed symbol sizes will not exceed the bounds of their Section. This is addressing a problem where a file had a large space between two sections that were not used by this module - the last symbol in the text section had an enormous size because the distance between that and the first symbol in the data section were used to compute the size. http://reviews.llvm.org/D19004 Modified: lldb/trunk/include/lldb/Core/RangeMap.h lldb/trunk/source/Symbol/Symtab.cpp Modified: lldb/trunk/include/lldb/Core/RangeMap.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/RangeMap.h?rev=266165&r1=266164&r2=266165&view=diff == --- lldb/trunk/include/lldb/Core/RangeMap.h (original) +++ lldb/trunk/include/lldb/Core/RangeMap.h Tue Apr 12 23:32:49 2016 @@ -1195,7 +1195,13 @@ namespace lldb_private { { return ((i < m_entries.size()) ? &m_entries[i] : nullptr); } - + +Entry * +GetMutableEntryAtIndex (size_t i) +{ +return ((i < m_entries.size()) ? &m_entries[i] : nullptr); +} + // Clients must ensure that "i" is a valid index prior to calling this function const Entry & GetEntryRef (size_t i) const Modified: lldb/trunk/source/Symbol/Symtab.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Symtab.cpp?rev=266165&r1=266164&r2=266165&view=diff == --- lldb/trunk/source/Symbol/Symtab.cpp (original) +++ lldb/trunk/source/Symbol/Symtab.cpp Tue Apr 12 23:32:49 2016 @@ -894,6 +894,38 @@ typedef struct addr_t match_offset; } SymbolSearchInfo; +// Add all the section file start address & size to the RangeVector, +// recusively adding any children sections. +static void +AddSectionsToRangeMap (SectionList *sectlist, RangeVector §ion_ranges) +{ +const int num_sections = sectlist->GetNumSections (0); +for (int i = 0; i < num_sections; i++) +{ +SectionSP sect_sp = sectlist->GetSectionAtIndex (i); +if (sect_sp) +{ +SectionList &child_sectlist = sect_sp->GetChildren(); + +// If this section has children, add the children to the RangeVector. +// Else add this section to the RangeVector. +if (child_sectlist.GetNumSections (0) > 0) +{ +AddSectionsToRangeMap (&child_sectlist, section_ranges); +} +else +{ +addr_t base_addr = sect_sp->GetFileAddress(); +size_t size = sect_sp->GetByteSize(); +RangeVector::Entry entry; +entry.SetRangeBase (base_addr); +entry.SetByteSize (size); +section_ranges.Append (entry); +} +} +} +} + void Symtab::InitAddressIndexes() { @@ -915,26 +947,75 @@ Symtab::InitAddressIndexes() m_file_addr_to_index.Append(entry); } } - const size_t num_entries = m_file_addr_to_index.GetSize(); if (num_entries > 0) { m_file_addr_to_index.Sort(); -// Now our last address range might not have had sizes because there -// was no subsequent symbol to calculate the size from. If this is -// the case, then calculate the size by capping it at the end of the -// section in which the symbol resides -lldb::addr_t total_size = 0; -const FileRangeToIndexMap::Entry* entry = m_file_addr_to_index.Back(); -if (entry->GetByteSize() == 0) +// Create a RangeVector with the start & size of all the sections for +// this objfile. We'll need to check this for any FileRangeToIndexMap +// entries with an uninitialized size, which could potentially be a +// large number so reconstituting the weak pointer is busywork when it +// is invariant information. +SectionList *sectlist = m_objfile->GetSectionList(); +RangeVector section_ranges; +if (sectlist) +{ +AddSectionsToRangeMap (sectlist, section_ranges); +section_ranges.Sort(); +} + +// Iterate through the FileRangeToIndexMap and fill in the size for any +// entries that didn't already have a size from the Symbol (e.g. if we +// have a plain linker symbol with an address only, instead of debug info +// where we get an address and a size and a type, etc.) +for (int i = 0; i < num_entries; i++) { -const Address& address = m_symbols[entry->data].Ge