Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received

2016-04-12 Thread Pavel Labath via lldb-commits
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

2016-04-12 Thread Pavel Labath via lldb-commits
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"

2016-04-12 Thread Pavel Labath via lldb-commits
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

2016-04-12 Thread Pavel Labath via lldb-commits
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

2016-04-12 Thread Ulrich Weigand via lldb-commits
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

2016-04-12 Thread Ulrich Weigand via lldb-commits
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

2016-04-12 Thread Pavel Labath via lldb-commits
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

2016-04-12 Thread Pavel Labath via lldb-commits
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

2016-04-12 Thread Pavel Labath via lldb-commits
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

2016-04-12 Thread Pavel Labath via lldb-commits
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

2016-04-12 Thread Pavel Labath via lldb-commits
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

2016-04-12 Thread Pavel Labath via lldb-commits
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

2016-04-12 Thread Muhammad Omair Javaid via lldb-commits
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

2016-04-12 Thread Ulrich Weigand via lldb-commits
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

2016-04-12 Thread Pavel Labath via lldb-commits
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

2016-04-12 Thread Tamas Berghammer via lldb-commits
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

2016-04-12 Thread Tamas Berghammer via lldb-commits
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

2016-04-12 Thread Tamas Berghammer via lldb-commits
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

2016-04-12 Thread Greg Clayton via lldb-commits
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

2016-04-12 Thread Greg Clayton via lldb-commits
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

2016-04-12 Thread Jim Ingham via lldb-commits
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

2016-04-12 Thread Greg Clayton via lldb-commits
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.

2016-04-12 Thread Jim Ingham via lldb-commits
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

2016-04-12 Thread Ulrich Weigand via lldb-commits
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

2016-04-12 Thread Enrico Granata via lldb-commits
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

2016-04-12 Thread Zachary Turner via lldb-commits
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

2016-04-12 Thread Greg Clayton via lldb-commits
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

2016-04-12 Thread Enrico Granata via lldb-commits
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)

2016-04-12 Thread Enrico Granata via lldb-commits
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

2016-04-12 Thread Ulrich Weigand via lldb-commits
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.

2016-04-12 Thread Greg Clayton via lldb-commits
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

2016-04-12 Thread Enrico Granata via lldb-commits
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

2016-04-12 Thread Enrico Granata via lldb-commits
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:

2016-04-12 Thread Greg Clayton via lldb-commits
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

2016-04-12 Thread Adrian McCarthy via lldb-commits
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

2016-04-12 Thread Oleksiy Vyalov via lldb-commits
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

2016-04-12 Thread Adrian McCarthy via lldb-commits
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

2016-04-12 Thread Adrian McCarthy via lldb-commits
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

2016-04-12 Thread Adrian McCarthy via lldb-commits
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

2016-04-12 Thread Ulrich Weigand via lldb-commits
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

2016-04-12 Thread Ulrich Weigand via lldb-commits
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

2016-04-12 Thread Ulrich Weigand via lldb-commits
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

2016-04-12 Thread Ulrich Weigand via lldb-commits
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

2016-04-12 Thread Ulrich Weigand via lldb-commits
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

2016-04-12 Thread Ulrich Weigand via lldb-commits
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

2016-04-12 Thread Jason Molenda via lldb-commits
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

2016-04-12 Thread Jason Molenda via lldb-commits
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.

2016-04-12 Thread Oleksiy Vyalov via lldb-commits
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

2016-04-12 Thread Jason Molenda via lldb-commits
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

2016-04-12 Thread Jason Molenda via lldb-commits
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