[Lldb-commits] [PATCH] D26676: Patch for lldb bug 26322 “core load hangs”

2016-11-17 Thread Howard Hellyer via lldb-commits 
hhellyer added a comment.

This is marked ready to land and I can land it now but I'm still working on the 
testcases.
I can either:

- Land the code changes and do the tests under another patch.
- Hold off on delivering this until I've finished the tests.

I'm not sure which is the preferred option. The tests will probably need to be 
delivered with core files so they will be much larger deltas than just the 
functional code changes currently contained in this patch. Is there a preferred 
option?


https://reviews.llvm.org/D26676



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D26676: Patch for lldb bug 26322 “core load hangs”

2016-11-17 Thread Pavel Labath via lldb-commits 
labath added a comment.

I can't say we're being very consistent in enforcing it, but general llvm 
policy is for tests to go together with the changes.

I am curious, how do you go about creating these core files?

The core files I created were very tiny as I did not require any syscalls, so I 
could avoid linking libc. Here it sounds like you would need at least one 
(clone(2)). I am considering reimplementing the required clone(2) bits inline, 
but I am afraid that may end up being a bit hairy.


https://reviews.llvm.org/D26676



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D26676: Patch for lldb bug 26322 “core load hangs”

2016-11-17 Thread Howard Hellyer via lldb-commits 
hhellyer added a comment.

I haven't solved that yet!  ;-)

I'm currently ending up with cores of ~500kb which is probably too big. I'm 
seeing what I can do to bring them down but it might be that I can't shrink 
them that much. I'm attempting to write two tests, one for multiple threads and 
one for cores generated by gcore but the actual C program for both is very 
similar.

What would you say a sensible target size actually is?

The other option is to run these tests just on Linux then it might be possible 
to create the core dumps as part of the test. There's a lot of drawbacks to 
that - not least just that they wouldn't be run every where but also that 
depending on how the OS is setup it's not always possible to create a core on a 
signal or attach gcore. I don't really want to add flakey tests.


https://reviews.llvm.org/D26676



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D26676: Patch for lldb bug 26322 “core load hangs”

2016-11-17 Thread Pavel Labath via lldb-commits 
labath added a comment.

In https://reviews.llvm.org/D26676#598559, @hhellyer wrote:

> I haven't solved that yet!  ;-)
>
> I'm currently ending up with cores of ~500kb which is probably too big. I'm 
> seeing what I can do to bring them down but it might be that I can't shrink 
> them that much. I'm attempting to write two tests, one for multiple threads 
> and one for cores generated by gcore but the actual C program for both is 
> very similar.
>
> What would you say a sensible target size actually is?


I would say about 100k is OK, but of course, the less, the better.

> The other option is to run these tests just on Linux then it might be 
> possible to create the core dumps as part of the test. There's a lot of 
> drawbacks to that - not least just that they wouldn't be run every where but 
> also that depending on how the OS is setup it's not always possible to create 
> a core on a signal or attach gcore. I don't really want to add flakey tests.

Yeah, we can't rely on linux kernel to generate the core files, as there are 
many ways that could be disabled. If we had a core file writer in LLDB (I don't 
know if we do, or if it works), we could possibly do that.

That said, I was able to trivially generate a ~20k core file by setting 
/proc/$PID/coredump_filter to 0. You won't get any memory regions that way, but 
it does not sound like you need any for this test anyway. It could probably be 
even improved by some tricks like static linking (to avoid having a long list 
of modules, etc). Have you tried that?


https://reviews.llvm.org/D26676



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D26676: Patch for lldb bug 26322 “core load hangs”

2016-11-17 Thread Howard Hellyer via lldb-commits 
hhellyer added a comment.

I'd assumed that gcore would ignore the coredump_filter setting but it turns 
out it doesn't so I should be able to use that. lldb seems happy enough opening 
the core files produced with a filter of 0 so it's probably the simplest 
solution.

I don't think lldb has a core file writer for Linux/ELF, process save-core 
doesn't seem to think it does. I'm not sure it would help in this case though - 
I need to make sure we can open core files created via gcore. Opening core 
files generated by lldb itself would be a separate test case. It's still quite 
a desirable feature, Linux does not really have a good API for creating core 
dumps. gcore is useful but it still means exec-ing another process.

I'll tidy up my test cases and try to generate at least 64 and 32 bit intel 
cores to go with them before adding them to this patch.


https://reviews.llvm.org/D26676



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D26676: Patch for lldb bug 26322 “core load hangs”

2016-11-17 Thread Greg Clayton via lldb-commits 
clayborg added a comment.

Checking core files into the repository will grow all git based repositories 
over time as every version of every core file will be included in the git 
clone. We have avoided checking in core files or any other larger files for 
this very reason. Not sure what the right answer is here.


https://reviews.llvm.org/D26676



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r287242 - Rewrite all Property related functions in terms of StringRef.

2016-11-17 Thread Zachary Turner via lldb-commits 
Author: zturner
Date: Thu Nov 17 12:08:12 2016
New Revision: 287242

URL: http://llvm.org/viewvc/llvm-project?rev=287242&view=rev
Log:
Rewrite all Property related functions in terms of StringRef.

This was a bit tricky, especially for things like
OptionValueArray and OptionValueDictionary since they do some
funky string parsing.  Rather than try to re-write line-by-line
I tried to make the StringRef usage idiomatic, even though
it meant often re-writing from scratch large blocks of code
in a different way while keeping true to the original intent.

The finished code is a big improvement though, and often much
shorter than the original code.  All tests and unit tests
pass on Windows and Linux.

Modified:
lldb/trunk/include/lldb/Core/DataBufferHeap.h
lldb/trunk/include/lldb/Core/Debugger.h
lldb/trunk/include/lldb/Core/Disassembler.h
lldb/trunk/include/lldb/Core/UserSettingsController.h
lldb/trunk/include/lldb/Interpreter/OptionValue.h
lldb/trunk/include/lldb/Interpreter/OptionValueArray.h
lldb/trunk/include/lldb/Interpreter/OptionValueDictionary.h
lldb/trunk/include/lldb/Interpreter/OptionValueProperties.h
lldb/trunk/include/lldb/Target/ProcessInfo.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/source/Commands/CommandObjectMemory.cpp
lldb/trunk/source/Commands/CommandObjectProcess.cpp
lldb/trunk/source/Commands/CommandObjectSettings.cpp
lldb/trunk/source/Core/Debugger.cpp
lldb/trunk/source/Core/Disassembler.cpp
lldb/trunk/source/Core/UserSettingsController.cpp
lldb/trunk/source/Interpreter/OptionValue.cpp
lldb/trunk/source/Interpreter/OptionValueArgs.cpp
lldb/trunk/source/Interpreter/OptionValueArray.cpp
lldb/trunk/source/Interpreter/OptionValueDictionary.cpp
lldb/trunk/source/Interpreter/OptionValueProperties.cpp

lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
lldb/trunk/source/Target/ProcessInfo.cpp
lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/include/lldb/Core/DataBufferHeap.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/DataBufferHeap.h?rev=287242&r1=287241&r2=287242&view=diff
==
--- lldb/trunk/include/lldb/Core/DataBufferHeap.h (original)
+++ lldb/trunk/include/lldb/Core/DataBufferHeap.h Thu Nov 17 12:08:12 2016
@@ -112,6 +112,7 @@ public:
   /// The number of bytes in \a src to copy.
   //--
   void CopyData(const void *src, lldb::offset_t src_len);
+  void CopyData(llvm::StringRef src) { CopyData(src.data(), src.size()); }
 
   void AppendData(const void *src, uint64_t src_len);
 

Modified: lldb/trunk/include/lldb/Core/Debugger.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Debugger.h?rev=287242&r1=287241&r2=287242&view=diff
==
--- lldb/trunk/include/lldb/Core/Debugger.h (original)
+++ lldb/trunk/include/lldb/Core/Debugger.h Thu Nov 17 12:08:12 2016
@@ -207,8 +207,8 @@ public:
   };
 
   Error SetPropertyValue(const ExecutionContext *exe_ctx,
- VarSetOperationType op, const char *property_path,
- const char *value) override;
+ VarSetOperationType op, llvm::StringRef property_path,
+llvm::StringRef value) override;
 
   bool GetAutoConfirm() const;
 

Modified: lldb/trunk/include/lldb/Core/Disassembler.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Disassembler.h?rev=287242&r1=287241&r2=287242&view=diff
==
--- lldb/trunk/include/lldb/Core/Disassembler.h (original)
+++ lldb/trunk/include/lldb/Core/Disassembler.h Thu Nov 17 12:08:12 2016
@@ -131,7 +131,7 @@ public:
 const DataExtractor &data,
 lldb::offset_t data_offset) = 0;
 
-  virtual void SetDescription(const char *) {
+  virtual void SetDescription(llvm::StringRef) {
   } // May be overridden in sub-classes that have descriptions.
 
   lldb::OptionValueSP ReadArray(FILE *in_file, Stream *out_stream,
@@ -290,7 +290,7 @@ public:
 
   void SetOpcode(size_t opcode_size, void *opcode_data);
 
-  void SetDescription(const char *description) override;
+  void SetDescription(llvm::StringRef description) override;
 
 protected:
   std::string m_description;

Modified: lldb/trunk/include/lldb/Core/UserSettingsController.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/UserSettingsController.h?rev=287242&r1=287241&r2=287242&view=diff
==
--- lldb/trunk/include/lldb/Core/UserSettingsController.h (original)
+++ lldb/trunk/include/lldb/Core/UserSettingsController.h Thu Nov 17 12:08:12 
2016
@@ -44,16 +44,16 @@ public:
   }
 
   vi

Re: [Lldb-commits] [lldb] r287242 - Rewrite all Property related functions in terms of StringRef.

2016-11-17 Thread Jim Ingham via lldb-commits 
I bet we don’t have enough test coverage in this area to ensure that these 
changes didn’t break things.  Did you check how much test coverage there was?  
If it isn’t good, which is my guess for properties, then it is good hygiene to 
add more tests that pass in the old way before embarking on this sort of 
refactoring.

Jim

> On Nov 17, 2016, at 10:08 AM, Zachary Turner via lldb-commits 
>  wrote:
> 
> Author: zturner
> Date: Thu Nov 17 12:08:12 2016
> New Revision: 287242
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=287242&view=rev
> Log:
> Rewrite all Property related functions in terms of StringRef.
> 
> This was a bit tricky, especially for things like
> OptionValueArray and OptionValueDictionary since they do some
> funky string parsing.  Rather than try to re-write line-by-line
> I tried to make the StringRef usage idiomatic, even though
> it meant often re-writing from scratch large blocks of code
> in a different way while keeping true to the original intent.
> 
> The finished code is a big improvement though, and often much
> shorter than the original code.  All tests and unit tests
> pass on Windows and Linux.
> 
> Modified:
>lldb/trunk/include/lldb/Core/DataBufferHeap.h
>lldb/trunk/include/lldb/Core/Debugger.h
>lldb/trunk/include/lldb/Core/Disassembler.h
>lldb/trunk/include/lldb/Core/UserSettingsController.h
>lldb/trunk/include/lldb/Interpreter/OptionValue.h
>lldb/trunk/include/lldb/Interpreter/OptionValueArray.h
>lldb/trunk/include/lldb/Interpreter/OptionValueDictionary.h
>lldb/trunk/include/lldb/Interpreter/OptionValueProperties.h
>lldb/trunk/include/lldb/Target/ProcessInfo.h
>lldb/trunk/include/lldb/Target/Target.h
>lldb/trunk/source/Commands/CommandObjectMemory.cpp
>lldb/trunk/source/Commands/CommandObjectProcess.cpp
>lldb/trunk/source/Commands/CommandObjectSettings.cpp
>lldb/trunk/source/Core/Debugger.cpp
>lldb/trunk/source/Core/Disassembler.cpp
>lldb/trunk/source/Core/UserSettingsController.cpp
>lldb/trunk/source/Interpreter/OptionValue.cpp
>lldb/trunk/source/Interpreter/OptionValueArgs.cpp
>lldb/trunk/source/Interpreter/OptionValueArray.cpp
>lldb/trunk/source/Interpreter/OptionValueDictionary.cpp
>lldb/trunk/source/Interpreter/OptionValueProperties.cpp
>
> lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
>lldb/trunk/source/Target/ProcessInfo.cpp
>lldb/trunk/source/Target/Target.cpp
> 
> Modified: lldb/trunk/include/lldb/Core/DataBufferHeap.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/DataBufferHeap.h?rev=287242&r1=287241&r2=287242&view=diff
> ==
> --- lldb/trunk/include/lldb/Core/DataBufferHeap.h (original)
> +++ lldb/trunk/include/lldb/Core/DataBufferHeap.h Thu Nov 17 12:08:12 2016
> @@ -112,6 +112,7 @@ public:
>   /// The number of bytes in \a src to copy.
>   //--
>   void CopyData(const void *src, lldb::offset_t src_len);
> +  void CopyData(llvm::StringRef src) { CopyData(src.data(), src.size()); }
> 
>   void AppendData(const void *src, uint64_t src_len);
> 
> 
> Modified: lldb/trunk/include/lldb/Core/Debugger.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Debugger.h?rev=287242&r1=287241&r2=287242&view=diff
> ==
> --- lldb/trunk/include/lldb/Core/Debugger.h (original)
> +++ lldb/trunk/include/lldb/Core/Debugger.h Thu Nov 17 12:08:12 2016
> @@ -207,8 +207,8 @@ public:
>   };
> 
>   Error SetPropertyValue(const ExecutionContext *exe_ctx,
> - VarSetOperationType op, const char *property_path,
> - const char *value) override;
> + VarSetOperationType op, llvm::StringRef 
> property_path,
> +llvm::StringRef value) override;
> 
>   bool GetAutoConfirm() const;
> 
> 
> Modified: lldb/trunk/include/lldb/Core/Disassembler.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Disassembler.h?rev=287242&r1=287241&r2=287242&view=diff
> ==
> --- lldb/trunk/include/lldb/Core/Disassembler.h (original)
> +++ lldb/trunk/include/lldb/Core/Disassembler.h Thu Nov 17 12:08:12 2016
> @@ -131,7 +131,7 @@ public:
> const DataExtractor &data,
> lldb::offset_t data_offset) = 0;
> 
> -  virtual void SetDescription(const char *) {
> +  virtual void SetDescription(llvm::StringRef) {
>   } // May be overridden in sub-classes that have descriptions.
> 
>   lldb::OptionValueSP ReadArray(FILE *in_file, Stream *out_stream,
> @@ -290,7 +290,7 @@ public:
> 
>   void SetOpcode(size_t opcode_size, void *opcode_data);
> 
> -  void SetDescription(const char *description) override;
> +  void SetDesc

Re: [Lldb-commits] [PATCH] D26804: Fix step-over when SymbolContext.function is missing and symbol is present.

2016-11-17 Thread Jim Ingham via lldb-commits 
Yes, comp unit equal, no function and symbol equal should be treated as 
equivalent.

However, your check would call two SymbolContexts equivalent that had different 
CompUnits but the same symbol.  I can’t see how that would happen in practice, 
but if it did something odd has gone on, and we probably should stop and let 
the user have a look.  So can you change this to add a check for symbol equal 
inside the “have a comp and it’s equal” instead?

Jim  


> On Nov 17, 2016, at 9:24 AM, Sam McCall  wrote:
> 
> sammccall created this revision.
> sammccall added a reviewer: jingham.
> sammccall added a subscriber: lldb-commits.
> 
> Fix step-over when SymbolContext.function is missing and symbol is present.
> 
> With targets from our build configuration,
> ThreadPlanStepOverRange::IsEquivalentContext fails to fire for relevant 
> frames,
> leading to ShouldStop() returning true prematurely.
> 
> The frame's SymbolContext, and m_addr_context have:
> 
> - comp_unit set and matching
> - function = nullptr
> - symbol set and matching (but this is never checked)
> 
> My naive guess is that the context should be equivalent in this case :-)
> 
> 
> https://reviews.llvm.org/D26804
> 
> Files:
>  source/Target/ThreadPlanStepOverRange.cpp
> 
> 
> Index: source/Target/ThreadPlanStepOverRange.cpp
> ===
> --- source/Target/ThreadPlanStepOverRange.cpp
> +++ source/Target/ThreadPlanStepOverRange.cpp
> @@ -106,22 +106,22 @@
>   // in so I left out the target check.  And sometimes the module comes in as
>   // the .o file from the
>   // inlined range, so I left that out too...
> -  if (m_addr_context.comp_unit) {
> -if (m_addr_context.comp_unit == context.comp_unit) {
> -  if (m_addr_context.function &&
> -  m_addr_context.function == context.function) {
> -// It is okay to return to a different block of a straight function, 
> we
> -// only have to
> -// be more careful if returning from one inlined block to another.
> -if (m_addr_context.block->GetInlinedFunctionInfo() == nullptr &&
> -context.block->GetInlinedFunctionInfo() == nullptr)
> -  return true;
> -
> -if (m_addr_context.block && m_addr_context.block == context.block)
> -  return true;
> -  }
> +  if (m_addr_context.comp_unit &&
> +  m_addr_context.comp_unit == context.comp_unit) {
> +if (m_addr_context.function &&
> +m_addr_context.function == context.function) {
> +  // It is okay to return to a different block of a straight function, we
> +  // only have to
> +  // be more careful if returning from one inlined block to another.
> +  if (m_addr_context.block->GetInlinedFunctionInfo() == nullptr &&
> +  context.block->GetInlinedFunctionInfo() == nullptr)
> +return true;
> +
> +  if (m_addr_context.block && m_addr_context.block == context.block)
> +return true;
> }
> -  } else if (m_addr_context.symbol && m_addr_context.symbol == 
> context.symbol) {
> +  }
> +  if (m_addr_context.symbol && m_addr_context.symbol == context.symbol) {
> return true;
>   }
>   return false;
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D26804: Fix step-over when SymbolContext.function is missing and symbol is present.

2016-11-17 Thread Jim Ingham via lldb-commits 
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

That way of doing it is fine too.


https://reviews.llvm.org/D26804



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r287259 - Convert Platform, Process, and Connection functions to StringRef.

2016-11-17 Thread Zachary Turner via lldb-commits 
Author: zturner
Date: Thu Nov 17 15:15:14 2016
New Revision: 287259

URL: http://llvm.org/viewvc/llvm-project?rev=287259&view=rev
Log:
Convert Platform, Process, and Connection functions to StringRef.

All tests pass on Linux and Windows.

Modified:
lldb/trunk/include/lldb/Core/Connection.h
lldb/trunk/include/lldb/Core/ConnectionSharedMemory.h
lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
lldb/trunk/include/lldb/Host/windows/ConnectionGenericFileWindows.h
lldb/trunk/include/lldb/Target/Platform.h
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Core/ConnectionSharedMemory.cpp
lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp

lldb/trunk/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
lldb/trunk/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/trunk/source/Target/Platform.cpp
lldb/trunk/source/Target/Process.cpp
lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/include/lldb/Core/Connection.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Connection.h?rev=287259&r1=287258&r2=287259&view=diff
==
--- lldb/trunk/include/lldb/Core/Connection.h (original)
+++ lldb/trunk/include/lldb/Core/Connection.h Thu Nov 17 15:15:14 2016
@@ -66,7 +66,8 @@ public:
   ///
   /// @see Error& Communication::GetError ();
   //--
-  virtual lldb::ConnectionStatus Connect(const char *url, Error *error_ptr) = 
0;
+  virtual lldb::ConnectionStatus Connect(llvm::StringRef url,
+ Error *error_ptr) = 0;
 
   //--
   /// Disconnect the communications connection if one is currently

Modified: lldb/trunk/include/lldb/Core/ConnectionSharedMemory.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ConnectionSharedMemory.h?rev=287259&r1=287258&r2=287259&view=diff
==
--- lldb/trunk/include/lldb/Core/ConnectionSharedMemory.h (original)
+++ lldb/trunk/include/lldb/Core/ConnectionSharedMemory.h Thu Nov 17 15:15:14 
2016
@@ -32,7 +32,7 @@ public:
   virtual lldb::ConnectionStatus BytesAvailable(uint32_t timeout_usec,
 Error *error_ptr);
 
-  lldb::ConnectionStatus Connect(const char *s, Error *error_ptr) override;
+  lldb::ConnectionStatus Connect(llvm::StringRef s, Error *error_ptr) override;
 
   lldb::ConnectionStatus Disconnect(Error *error_ptr) override;
 

Modified: lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h?rev=287259&r1=287258&r2=287259&view=diff
==
--- lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h 
(original)
+++ lldb/trunk/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h Thu Nov 
17 15:15:14 2016
@@ -53,7 +53,7 @@ public:
 
   bool IsConnected() const override;
 
-  lldb::ConnectionStatus Connect(const char *s, Error *error_ptr) override;
+  lldb::ConnectionStatus Connect(llvm::StringRef s, Error *error_ptr) override;
 
   lldb::ConnectionStatus Disconnect(Error *error_ptr) override;
 
@@ -82,21 +82,21 @@ protected:
 
   void CloseCommandPipe();
 
-  lldb::ConnectionStatus SocketListenAndAccept(const char *host_and_port,
+  lldb::ConnectionStatus SocketListenAndAccept(llvm::StringRef host_and_port,
Error *error_ptr);
 
-  lldb::ConnectionStatus ConnectTCP(const char *host_and_port,
+  lldb::ConnectionStatus ConnectTCP(llvm::StringRef host_and_port,
 Error *error_ptr);
 
-  lldb::ConnectionStatus ConnectUDP(const char *args, Error *error_ptr);
+  lldb::ConnectionStatus ConnectUDP(llvm::StringRef args, Error *error_ptr);
 
-  lldb::ConnectionStatus NamedSocketConnect(const char *socket_name,
+  lldb::ConnectionStatus NamedSocketConnect(llvm::StringRef socket_name,
 Error *error_ptr);
 
-  lldb::ConnectionStatus NamedSocketAcce

Re: [Lldb-commits] [PATCH] D26804: Fix step-over when SymbolContext.function is missing and symbol is present.

2016-11-17 Thread Jim Ingham via lldb-commits 
Ah, good catch.  That is not right.  If there’s inlined function information, 
you want to make sure you haven’t gone from one inlined function to another.  
But the symbol is always going to be the same in this case.

Actually, the old code is a overly restrictive by comparing the blocks 
directly.  What you really want to test is that if either side is in an inlined 
function, both sides are still inside the same inlined function instantiation.  
We should really compare the ContainingInlinedBlock of the current blocks if 
there is inlined info.

This stuff is a PITA to write tests for, too, since it is hard to predict how 
the compiler will emit inlined code...

Jim

> On Nov 17, 2016, at 1:13 PM, Sam McCall  wrote:
> 
> Thanks. I just noticed, this also changes the behavior (now returns true) if:
>  - comp_unit and function both match
>  - block->GetInlinedFunctionInfo != nullptr
>  - blocks aren't equal
>  - symbols are equal
> 
> Is this correct? Otherwise I'll fix this case to match the old behavior.
> 
> On Thu, Nov 17, 2016 at 10:09 PM, Jim Ingham  > wrote:
> jingham accepted this revision.
> jingham added a comment.
> This revision is now accepted and ready to land.
> 
> That way of doing it is fine too.
> 
> 
> https://reviews.llvm.org/D26804 
> 
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D26804: Fix step-over when SymbolContext.function is missing and symbol is present.

2016-11-17 Thread Jim Ingham via lldb-commits 

> On Nov 17, 2016, at 1:45 PM, Sam McCall  wrote:
> 
> On Thu, Nov 17, 2016 at 10:29 PM, Jim Ingham  > wrote:
> Ah, good catch.  That is not right.  If there’s inlined function information, 
> you want to make sure you haven’t gone from one inlined function to another.  
> But the symbol is always going to be the same in this case.
> OK, I've updated the patch to return false in this case.
> 
> Actually, the old code is a overly restrictive by comparing the blocks 
> directly.  What you really want to test is that if either side is in an 
> inlined function, both sides are still inside the same inlined function 
> instantiation.  We should really compare the ContainingInlinedBlock of the 
> current blocks if there is inlined info.
> Happy to take a stab at this - I'm not too familiar with the structures here 
> so I'll read a bit and send a new patch if that's OK!

That’s fine.

Jim


> 
> This stuff is a PITA to write tests for, too, since it is hard to predict how 
> the compiler will emit inlined code...
> 
> Jim
> 
> 
>> On Nov 17, 2016, at 1:13 PM, Sam McCall > > wrote:
>> 
>> Thanks. I just noticed, this also changes the behavior (now returns true) if:
>>  - comp_unit and function both match
>>  - block->GetInlinedFunctionInfo != nullptr
>>  - blocks aren't equal
>>  - symbols are equal
>> 
>> Is this correct? Otherwise I'll fix this case to match the old behavior.
>> 
>> On Thu, Nov 17, 2016 at 10:09 PM, Jim Ingham > > wrote:
>> jingham accepted this revision.
>> jingham added a comment.
>> This revision is now accepted and ready to land.
>> 
>> That way of doing it is fine too.
>> 
>> 
>> https://reviews.llvm.org/D26804 
>> 
>> 
>> 
>> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D26804: Fix step-over when SymbolContext.function is missing and symbol is present.

2016-11-17 Thread Jim Ingham via lldb-commits 
jingham added a comment.

Excellent, thanks for catching that.


https://reviews.llvm.org/D26804



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r287266 - Make GetRegisterByName() take a StringRef.

2016-11-17 Thread Zachary Turner via lldb-commits 
Author: zturner
Date: Thu Nov 17 15:54:37 2016
New Revision: 287266

URL: http://llvm.org/viewvc/llvm-project?rev=287266&view=rev
Log:
Make GetRegisterByName() take a StringRef.

This one is fairly trivial and only really involves changing
function signatures and a few simple call-sites.

Modified:
lldb/trunk/include/lldb/Host/common/NativeRegisterContext.h
lldb/trunk/include/lldb/Target/RegisterContext.h
lldb/trunk/source/Host/common/NativeRegisterContext.cpp
lldb/trunk/source/Target/RegisterContext.cpp

Modified: lldb/trunk/include/lldb/Host/common/NativeRegisterContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/NativeRegisterContext.h?rev=287266&r1=287265&r2=287266&view=diff
==
--- lldb/trunk/include/lldb/Host/common/NativeRegisterContext.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeRegisterContext.h Thu Nov 17 
15:54:37 2016
@@ -126,7 +126,7 @@ public:
 
   virtual NativeThreadProtocol &GetThread() { return m_thread; }
 
-  const RegisterInfo *GetRegisterInfoByName(const char *reg_name,
+  const RegisterInfo *GetRegisterInfoByName(llvm::StringRef reg_name,
 uint32_t start_idx = 0);
 
   const RegisterInfo *GetRegisterInfo(uint32_t reg_kind, uint32_t reg_num);

Modified: lldb/trunk/include/lldb/Target/RegisterContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/RegisterContext.h?rev=287266&r1=287265&r2=287266&view=diff
==
--- lldb/trunk/include/lldb/Target/RegisterContext.h (original)
+++ lldb/trunk/include/lldb/Target/RegisterContext.h Thu Nov 17 15:54:37 2016
@@ -155,7 +155,7 @@ public:
 
   virtual Thread &GetThread() { return m_thread; }
 
-  const RegisterInfo *GetRegisterInfoByName(const char *reg_name,
+  const RegisterInfo *GetRegisterInfoByName(llvm::StringRef reg_name,
 uint32_t start_idx = 0);
 
   const RegisterInfo *GetRegisterInfo(lldb::RegisterKind reg_kind,

Modified: lldb/trunk/source/Host/common/NativeRegisterContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeRegisterContext.cpp?rev=287266&r1=287265&r2=287266&view=diff
==
--- lldb/trunk/source/Host/common/NativeRegisterContext.cpp (original)
+++ lldb/trunk/source/Host/common/NativeRegisterContext.cpp Thu Nov 17 15:54:37 
2016
@@ -56,20 +56,18 @@ NativeRegisterContext::~NativeRegisterCo
 // }
 
 const RegisterInfo *
-NativeRegisterContext::GetRegisterInfoByName(const char *reg_name,
+NativeRegisterContext::GetRegisterInfoByName(llvm::StringRef reg_name,
  uint32_t start_idx) {
-  if (reg_name && reg_name[0]) {
-const uint32_t num_registers = GetRegisterCount();
-for (uint32_t reg = start_idx; reg < num_registers; ++reg) {
-  const RegisterInfo *reg_info = GetRegisterInfoAtIndex(reg);
+  if (reg_name.empty())
+return nullptr;
 
-  if ((reg_info->name != nullptr &&
-   ::strcasecmp(reg_info->name, reg_name) == 0) ||
-  (reg_info->alt_name != nullptr &&
-   ::strcasecmp(reg_info->alt_name, reg_name) == 0)) {
-return reg_info;
-  }
-}
+  const uint32_t num_registers = GetRegisterCount();
+  for (uint32_t reg = start_idx; reg < num_registers; ++reg) {
+const RegisterInfo *reg_info = GetRegisterInfoAtIndex(reg);
+
+if (reg_name.equals_lower(reg_info->name) ||
+reg_name.equals_lower(reg_info->alt_name))
+  return reg_info;
   }
   return nullptr;
 }

Modified: lldb/trunk/source/Target/RegisterContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/RegisterContext.cpp?rev=287266&r1=287265&r2=287266&view=diff
==
--- lldb/trunk/source/Target/RegisterContext.cpp (original)
+++ lldb/trunk/source/Target/RegisterContext.cpp Thu Nov 17 15:54:37 2016
@@ -53,20 +53,19 @@ void RegisterContext::InvalidateIfNeeded
   }
 }
 
-const RegisterInfo *RegisterContext::GetRegisterInfoByName(const char 
*reg_name,
-   uint32_t start_idx) 
{
-  if (reg_name && reg_name[0]) {
-const uint32_t num_registers = GetRegisterCount();
-for (uint32_t reg = start_idx; reg < num_registers; ++reg) {
-  const RegisterInfo *reg_info = GetRegisterInfoAtIndex(reg);
+const RegisterInfo *
+RegisterContext::GetRegisterInfoByName(llvm::StringRef reg_name,
+   uint32_t start_idx) {
+  if (reg_name.empty())
+return nullptr;
 
-  if ((reg_info->name != nullptr &&
-   ::strcasecmp(reg_info->name, reg_name) == 0) ||
-  (reg_info->alt_name != nullptr &&
-   ::strcasecmp(reg_info->alt_name, reg_nam

[Lldb-commits] [lldb] r287274 - Fix step-over when SymbolContext.function is missing and symbol is present.

2016-11-17 Thread Sam McCall via lldb-commits 
Author: sammccall
Date: Thu Nov 17 16:29:31 2016
New Revision: 287274

URL: http://llvm.org/viewvc/llvm-project?rev=287274&view=rev
Log:
Fix step-over when SymbolContext.function is missing and symbol is present.

Summary:
Fix step-over when SymbolContext.function is missing and symbol is present.

With targets from our build configuration,
ThreadPlanStepOverRange::IsEquivalentContext fails to fire for relevant frames,
leading to ShouldStop() returning true prematurely.

The frame's SymbolContext, and m_addr_context have:
  - comp_unit set and matching
  - function = nullptr
  - symbol set and matching (but this is never checked)
My naive guess is that the context should be equivalent in this case :-)

Reviewers: jingham

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D26804

Modified:
lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp

Modified: lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp?rev=287274&r1=287273&r2=287274&view=diff
==
--- lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp (original)
+++ lldb/trunk/source/Target/ThreadPlanStepOverRange.cpp Thu Nov 17 16:29:31 
2016
@@ -107,21 +107,22 @@ bool ThreadPlanStepOverRange::IsEquivale
   // the .o file from the
   // inlined range, so I left that out too...
   if (m_addr_context.comp_unit) {
-if (m_addr_context.comp_unit == context.comp_unit) {
-  if (m_addr_context.function &&
-  m_addr_context.function == context.function) {
-// It is okay to return to a different block of a straight function, we
-// only have to
-// be more careful if returning from one inlined block to another.
-if (m_addr_context.block->GetInlinedFunctionInfo() == nullptr &&
-context.block->GetInlinedFunctionInfo() == nullptr)
-  return true;
-
-if (m_addr_context.block && m_addr_context.block == context.block)
-  return true;
-  }
+if (m_addr_context.comp_unit != context.comp_unit)
+  return false;
+if (m_addr_context.function) {
+  if (m_addr_context.function != context.function)
+return false;
+  // It is okay to return to a different block of a straight function, we
+  // only have to
+  // be more careful if returning from one inlined block to another.
+  if (m_addr_context.block->GetInlinedFunctionInfo() == nullptr &&
+  context.block->GetInlinedFunctionInfo() == nullptr)
+return true;
+  return m_addr_context.block == context.block;
 }
-  } else if (m_addr_context.symbol && m_addr_context.symbol == context.symbol) 
{
+  }
+  // Fall back to symbol if we have no decision from comp_unit/function/block.
+  if (m_addr_context.symbol && m_addr_context.symbol == context.symbol) {
 return true;
   }
   return false;


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r287279 - Change RegisterValue getters / setters to use StringRef.

2016-11-17 Thread Zachary Turner via lldb-commits 
Author: zturner
Date: Thu Nov 17 17:05:28 2016
New Revision: 287279

URL: http://llvm.org/viewvc/llvm-project?rev=287279&view=rev
Log:
Change RegisterValue getters / setters to use StringRef.

In the process, found some functions that were duplicates of
existing StringRef member functions.  So deleted those functions
and used the StringRef functions instead.

Modified:
lldb/trunk/include/lldb/Core/RegisterValue.h
lldb/trunk/source/Commands/CommandObjectRegister.cpp
lldb/trunk/source/Core/RegisterValue.cpp
lldb/trunk/source/Core/ValueObjectRegister.cpp
lldb/trunk/source/Core/ValueObjectVariable.cpp

Modified: lldb/trunk/include/lldb/Core/RegisterValue.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/RegisterValue.h?rev=287279&r1=287278&r2=287279&view=diff
==
--- lldb/trunk/include/lldb/Core/RegisterValue.h (original)
+++ lldb/trunk/include/lldb/Core/RegisterValue.h Thu Nov 17 17:05:28 2016
@@ -233,8 +233,10 @@ public:
 
   bool SignExtend(uint32_t sign_bitpos);
 
-  Error SetValueFromCString(const RegisterInfo *reg_info,
-const char *value_str);
+  Error SetValueFromString(const RegisterInfo *reg_info,
+   llvm::StringRef value_str);
+  Error SetValueFromString(const RegisterInfo *reg_info,
+   const char *value_str) = delete;
 
   Error SetValueFromData(const RegisterInfo *reg_info, DataExtractor &data,
  lldb::offset_t offset, bool partial_data_ok);

Modified: lldb/trunk/source/Commands/CommandObjectRegister.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectRegister.cpp?rev=287279&r1=287278&r2=287279&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectRegister.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectRegister.cpp Thu Nov 17 17:05:28 
2016
@@ -356,7 +356,7 @@ protected:
   result.SetStatus(eReturnStatusFailed);
 } else {
   const char *reg_name = command.GetArgumentAtIndex(0);
-  const char *value_str = command.GetArgumentAtIndex(1);
+  llvm::StringRef value_str = command.GetArgumentAtIndex(1);
 
   // in most LLDB commands we accept $rbx as the name for register RBX - 
and
   // here we would
@@ -373,7 +373,7 @@ protected:
   if (reg_info) {
 RegisterValue reg_value;
 
-Error error(reg_value.SetValueFromCString(reg_info, value_str));
+Error error(reg_value.SetValueFromString(reg_info, value_str));
 if (error.Success()) {
   if (reg_ctx->WriteRegister(reg_info, reg_value)) {
 // Toss all frames and anything else in the thread
@@ -386,11 +386,11 @@ protected:
 if (error.AsCString()) {
   result.AppendErrorWithFormat(
   "Failed to write register '%s' with value '%s': %s\n", reg_name,
-  value_str, error.AsCString());
+  value_str.str().c_str(), error.AsCString());
 } else {
   result.AppendErrorWithFormat(
   "Failed to write register '%s' with value '%s'", reg_name,
-  value_str);
+  value_str.str().c_str());
 }
 result.SetStatus(eReturnStatusFailed);
   } else {

Modified: lldb/trunk/source/Core/RegisterValue.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegisterValue.cpp?rev=287279&r1=287278&r2=287279&view=diff
==
--- lldb/trunk/source/Core/RegisterValue.cpp (original)
+++ lldb/trunk/source/Core/RegisterValue.cpp Thu Nov 17 17:05:28 2016
@@ -347,51 +347,35 @@ Error RegisterValue::SetValueFromData(co
   return error;
 }
 
-static inline void StripSpaces(llvm::StringRef &Str) {
-  while (!Str.empty() && isspace(Str[0]))
-Str = Str.substr(1);
-  while (!Str.empty() && isspace(Str.back()))
-Str = Str.substr(0, Str.size() - 1);
-}
-
-static inline void LStrip(llvm::StringRef &Str, char c) {
-  if (!Str.empty() && Str.front() == c)
-Str = Str.substr(1);
-}
-
-static inline void RStrip(llvm::StringRef &Str, char c) {
-  if (!Str.empty() && Str.back() == c)
-Str = Str.substr(0, Str.size() - 1);
-}
-
-// Helper function for RegisterValue::SetValueFromCString()
+// Helper function for RegisterValue::SetValueFromString()
 static bool ParseVectorEncoding(const RegisterInfo *reg_info,
-const char *vector_str,
+llvm::StringRef vector_str,
 const uint32_t byte_size,
 RegisterValue *reg_value) {
   // Example: vector_str = "{0x2c 0x4b 0x2a 0x3e 0xd0 0x4f 0x2a 0x3e 0xac 0x4a
   // 0x2a 0x3e 0x84 0x4f 0x2a 0x3e}".
-  llvm::StringRef Str(vector_str);
-  StripSpaces(Str);
-  LStrip(Str, '{');
-  RStrip(Str, '}');
-  StripSpaces(Str);

[Lldb-commits] [lldb] r287281 - Revert "Change RegisterValue getters / setters to use StringRef."

2016-11-17 Thread Zachary Turner via lldb-commits 
Author: zturner
Date: Thu Nov 17 17:32:26 2016
New Revision: 287281

URL: http://llvm.org/viewvc/llvm-project?rev=287281&view=rev
Log:
Revert "Change RegisterValue getters / setters to use StringRef."

This reverts commit r287279, which breaks some register
tests on Linux.

Modified:
lldb/trunk/include/lldb/Core/RegisterValue.h
lldb/trunk/source/Commands/CommandObjectRegister.cpp
lldb/trunk/source/Core/RegisterValue.cpp
lldb/trunk/source/Core/ValueObjectRegister.cpp
lldb/trunk/source/Core/ValueObjectVariable.cpp

Modified: lldb/trunk/include/lldb/Core/RegisterValue.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/RegisterValue.h?rev=287281&r1=287280&r2=287281&view=diff
==
--- lldb/trunk/include/lldb/Core/RegisterValue.h (original)
+++ lldb/trunk/include/lldb/Core/RegisterValue.h Thu Nov 17 17:32:26 2016
@@ -233,10 +233,8 @@ public:
 
   bool SignExtend(uint32_t sign_bitpos);
 
-  Error SetValueFromString(const RegisterInfo *reg_info,
-   llvm::StringRef value_str);
-  Error SetValueFromString(const RegisterInfo *reg_info,
-   const char *value_str) = delete;
+  Error SetValueFromCString(const RegisterInfo *reg_info,
+const char *value_str);
 
   Error SetValueFromData(const RegisterInfo *reg_info, DataExtractor &data,
  lldb::offset_t offset, bool partial_data_ok);

Modified: lldb/trunk/source/Commands/CommandObjectRegister.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectRegister.cpp?rev=287281&r1=287280&r2=287281&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectRegister.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectRegister.cpp Thu Nov 17 17:32:26 
2016
@@ -356,7 +356,7 @@ protected:
   result.SetStatus(eReturnStatusFailed);
 } else {
   const char *reg_name = command.GetArgumentAtIndex(0);
-  llvm::StringRef value_str = command.GetArgumentAtIndex(1);
+  const char *value_str = command.GetArgumentAtIndex(1);
 
   // in most LLDB commands we accept $rbx as the name for register RBX - 
and
   // here we would
@@ -373,7 +373,7 @@ protected:
   if (reg_info) {
 RegisterValue reg_value;
 
-Error error(reg_value.SetValueFromString(reg_info, value_str));
+Error error(reg_value.SetValueFromCString(reg_info, value_str));
 if (error.Success()) {
   if (reg_ctx->WriteRegister(reg_info, reg_value)) {
 // Toss all frames and anything else in the thread
@@ -386,11 +386,11 @@ protected:
 if (error.AsCString()) {
   result.AppendErrorWithFormat(
   "Failed to write register '%s' with value '%s': %s\n", reg_name,
-  value_str.str().c_str(), error.AsCString());
+  value_str, error.AsCString());
 } else {
   result.AppendErrorWithFormat(
   "Failed to write register '%s' with value '%s'", reg_name,
-  value_str.str().c_str());
+  value_str);
 }
 result.SetStatus(eReturnStatusFailed);
   } else {

Modified: lldb/trunk/source/Core/RegisterValue.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegisterValue.cpp?rev=287281&r1=287280&r2=287281&view=diff
==
--- lldb/trunk/source/Core/RegisterValue.cpp (original)
+++ lldb/trunk/source/Core/RegisterValue.cpp Thu Nov 17 17:32:26 2016
@@ -347,35 +347,51 @@ Error RegisterValue::SetValueFromData(co
   return error;
 }
 
-// Helper function for RegisterValue::SetValueFromString()
+static inline void StripSpaces(llvm::StringRef &Str) {
+  while (!Str.empty() && isspace(Str[0]))
+Str = Str.substr(1);
+  while (!Str.empty() && isspace(Str.back()))
+Str = Str.substr(0, Str.size() - 1);
+}
+
+static inline void LStrip(llvm::StringRef &Str, char c) {
+  if (!Str.empty() && Str.front() == c)
+Str = Str.substr(1);
+}
+
+static inline void RStrip(llvm::StringRef &Str, char c) {
+  if (!Str.empty() && Str.back() == c)
+Str = Str.substr(0, Str.size() - 1);
+}
+
+// Helper function for RegisterValue::SetValueFromCString()
 static bool ParseVectorEncoding(const RegisterInfo *reg_info,
-llvm::StringRef vector_str,
+const char *vector_str,
 const uint32_t byte_size,
 RegisterValue *reg_value) {
   // Example: vector_str = "{0x2c 0x4b 0x2a 0x3e 0xd0 0x4f 0x2a 0x3e 0xac 0x4a
   // 0x2a 0x3e 0x84 0x4f 0x2a 0x3e}".
-  vector_str = vector_str.trim();
-  vector_str.consume_front("{");
-  vector_str.consume_back("}");
-  vector_str = vector_str.trim();
+  llvm::StringRef Str(vector_str);
+  StripSpaces(Str);
+  LStrip(St

[Lldb-commits] [lldb] r287282 - Resubmit "Change RegisterValue getters / setters to use StringRef."

2016-11-17 Thread Zachary Turner via lldb-commits 
Author: zturner
Date: Thu Nov 17 17:47:31 2016
New Revision: 287282

URL: http://llvm.org/viewvc/llvm-project?rev=287282&view=rev
Log:
Resubmit "Change RegisterValue getters / setters to use StringRef."

This resubmits r287279 with a fix for the original issue, which
was a trivial typo.

Modified:
lldb/trunk/include/lldb/Core/RegisterValue.h
lldb/trunk/source/Commands/CommandObjectRegister.cpp
lldb/trunk/source/Core/RegisterValue.cpp
lldb/trunk/source/Core/ValueObjectRegister.cpp
lldb/trunk/source/Core/ValueObjectVariable.cpp

Modified: lldb/trunk/include/lldb/Core/RegisterValue.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/RegisterValue.h?rev=287282&r1=287281&r2=287282&view=diff
==
--- lldb/trunk/include/lldb/Core/RegisterValue.h (original)
+++ lldb/trunk/include/lldb/Core/RegisterValue.h Thu Nov 17 17:47:31 2016
@@ -233,8 +233,10 @@ public:
 
   bool SignExtend(uint32_t sign_bitpos);
 
-  Error SetValueFromCString(const RegisterInfo *reg_info,
-const char *value_str);
+  Error SetValueFromString(const RegisterInfo *reg_info,
+   llvm::StringRef value_str);
+  Error SetValueFromString(const RegisterInfo *reg_info,
+   const char *value_str) = delete;
 
   Error SetValueFromData(const RegisterInfo *reg_info, DataExtractor &data,
  lldb::offset_t offset, bool partial_data_ok);

Modified: lldb/trunk/source/Commands/CommandObjectRegister.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectRegister.cpp?rev=287282&r1=287281&r2=287282&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectRegister.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectRegister.cpp Thu Nov 17 17:47:31 
2016
@@ -356,7 +356,7 @@ protected:
   result.SetStatus(eReturnStatusFailed);
 } else {
   const char *reg_name = command.GetArgumentAtIndex(0);
-  const char *value_str = command.GetArgumentAtIndex(1);
+  llvm::StringRef value_str = command.GetArgumentAtIndex(1);
 
   // in most LLDB commands we accept $rbx as the name for register RBX - 
and
   // here we would
@@ -373,7 +373,7 @@ protected:
   if (reg_info) {
 RegisterValue reg_value;
 
-Error error(reg_value.SetValueFromCString(reg_info, value_str));
+Error error(reg_value.SetValueFromString(reg_info, value_str));
 if (error.Success()) {
   if (reg_ctx->WriteRegister(reg_info, reg_value)) {
 // Toss all frames and anything else in the thread
@@ -386,11 +386,11 @@ protected:
 if (error.AsCString()) {
   result.AppendErrorWithFormat(
   "Failed to write register '%s' with value '%s': %s\n", reg_name,
-  value_str, error.AsCString());
+  value_str.str().c_str(), error.AsCString());
 } else {
   result.AppendErrorWithFormat(
   "Failed to write register '%s' with value '%s'", reg_name,
-  value_str);
+  value_str.str().c_str());
 }
 result.SetStatus(eReturnStatusFailed);
   } else {

Modified: lldb/trunk/source/Core/RegisterValue.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegisterValue.cpp?rev=287282&r1=287281&r2=287282&view=diff
==
--- lldb/trunk/source/Core/RegisterValue.cpp (original)
+++ lldb/trunk/source/Core/RegisterValue.cpp Thu Nov 17 17:47:31 2016
@@ -347,51 +347,35 @@ Error RegisterValue::SetValueFromData(co
   return error;
 }
 
-static inline void StripSpaces(llvm::StringRef &Str) {
-  while (!Str.empty() && isspace(Str[0]))
-Str = Str.substr(1);
-  while (!Str.empty() && isspace(Str.back()))
-Str = Str.substr(0, Str.size() - 1);
-}
-
-static inline void LStrip(llvm::StringRef &Str, char c) {
-  if (!Str.empty() && Str.front() == c)
-Str = Str.substr(1);
-}
-
-static inline void RStrip(llvm::StringRef &Str, char c) {
-  if (!Str.empty() && Str.back() == c)
-Str = Str.substr(0, Str.size() - 1);
-}
-
-// Helper function for RegisterValue::SetValueFromCString()
+// Helper function for RegisterValue::SetValueFromString()
 static bool ParseVectorEncoding(const RegisterInfo *reg_info,
-const char *vector_str,
+llvm::StringRef vector_str,
 const uint32_t byte_size,
 RegisterValue *reg_value) {
   // Example: vector_str = "{0x2c 0x4b 0x2a 0x3e 0xd0 0x4f 0x2a 0x3e 0xac 0x4a
   // 0x2a 0x3e 0x84 0x4f 0x2a 0x3e}".
-  llvm::StringRef Str(vector_str);
-  StripSpaces(Str);
-  LStrip(Str, '{');
-  RStrip(Str, '}');
-  StripSpaces(Str);
+  vector_str = vector_str.trim();
+  vector_str.consume_front("{");
+

[Lldb-commits] Buildbot numbers for the week of 10/30/2016 - 11/05/2016

2016-11-17 Thread Galina Kistanova via lldb-commits 
Hello everyone,

Below are some buildbot numbers for the week of 10/30/2016 - 11/05/2016.

Please see the same data in attached csv files:

The longest time each builder was red during the last week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the last week:

buildername |  was_red
+--
 clang-3stage-ubuntu| 102:45:51
 lld-x86_64-win7| 62:36:35
 clang-with-thin-lto-ubuntu | 50:21:30
 clang-with-lto-ubuntu  | 49:57:01
 llvm-sphinx-docs   | 46:47:20
 clang-cmake-mipsel | 31:07:29
 lldb-windows7-android  | 29:38:35
 lldb-x86_64-ubuntu-14.04-android   | 29:21:52
 lldb-x86_64-ubuntu-14.04-cmake | 28:53:25
 clang-native-aarch64-full  | 25:33:26
 clang-s390x-linux  | 19:33:55
 llvm-mips-linux| 19:18:24
 libcxx-libcxxabi-singlethreaded-x86_64-linux-debian| 18:33:38
 clang-cmake-mips   | 18:24:40
 clang-cmake-armv7-a15-selfhost-neon| 17:07:34
 clang-cmake-thumbv7-a15-full-sh| 12:32:39
 clang-cmake-armv7-a15-selfhost | 07:26:03
 clang-cmake-armv7-a15-full | 05:29:28
 clang-ppc64le-linux-multistage | 05:08:18
 clang-cmake-thumbv7-a15| 04:47:22
 clang-cmake-armv7-a15  | 04:47:11
 libcxx-libcxxabi-libunwind-arm-linux   | 04:02:26
 clang-ppc64be-linux| 03:34:45
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc49-cxx11   | 03:16:17
 libcxx-libcxxabi-libunwind-x86_64-linux-ubuntu | 03:15:46
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11 | 03:14:43
 clang-ppc64be-linux-multistage | 03:14:23
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14 | 03:13:21
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx1z | 03:05:44
 sanitizer-ppc64be-linux| 03:04:20
 sanitizer-x86_64-linux | 03:02:22
 clang-x86-windows-msvc2015 | 02:58:25
 libcxx-libcxxabi-x86_64-linux-debian   | 02:50:43
 libcxx-libcxxabi-x86_64-linux-debian-noexceptions  | 02:50:43
 libcxx-libcxxabi-libunwind-x86_64-linux-debian | 02:50:37
 clang-bpf-build| 02:48:34
 clang-x86_64-linux-selfhost-modules| 02:45:20
 clang-cmake-aarch64-full   | 02:44:48
 sanitizer-x86_64-linux-fuzzer  | 02:37:33
 clang-ppc64be-linux-lnt| 02:16:04
 perf-x86_64-penryn-O3-polly-unprofitable   | 02:15:46
 clang-cmake-aarch64-39vma  | 02:12:58
 clang-native-arm-lnt   | 02:09:46
 perf-x86_64-penryn-O3-polly-before-vectorizer-unprofitable | 02:09:16
 clang-x64-ninja-win7   | 02:08:28
 clang-cmake-aarch64-quick  | 02:00:09
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast   | 01:49:37
 libcxx-libcxxabi-x86_64-linux-ubuntu-ubsan | 01:39:03
 perf-x86_64-penryn-O3  | 01:31:03
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast | 01:21:23
 libcxx-libcxxabi-libunwind-arm-linux-noexceptions  | 01:18:46
 lld-x86_64-darwin13| 01:17:16
 clang-cmake-aarch64-42vma  | 01:14:37
 clang-hexagon-elf  | 01:09:16
 lld-x86_64-freebsd | 01:07:29
 clang-ppc64le-linux-lnt| 01:06:36
 perf-x86_64-penryn-O3-polly-parallel-fast  | 01:01:25
 lldb-x86_64-ubuntu-14.04-buildserver   | 01:00:38
 clang-ppc64le-linux| 00:54:41
 clang-cuda-build

[Lldb-commits] Buildbot numbers for the week of 11/06/2016 - 11/12/2016

2016-11-17 Thread Galina Kistanova via lldb-commits 
Hello everyone,

Below are some buildbot numbers for the last week of 11/06/2016 -
11/12/2016.

Please see the same data in attached csv files:

The longest time each builder was red during the last week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the last week:

buildername |  was_red
+--
 lldb-x86-windows-msvc2015  | 108:30:55
 lldb-x86_64-ubuntu-14.04-android   | 85:59:56
 sanitizer-x86_64-linux-bootstrap   | 43:55:59
 sanitizer-x86_64-linux-fast| 42:16:35
 libcxx-libcxxabi-libunwind-x86_64-linux-debian | 25:45:26
 libcxx-libcxxabi-x86_64-linux-debian-noexceptions  | 25:44:55
 clang-ppc64be-linux-multistage | 23:30:33
 clang-ppc64be-linux-lnt| 23:20:10
 clang-ppc64be-linux| 22:47:14
 clang-cmake-mipsel | 21:28:18
 lldb-windows7-android  | 18:34:37
 lldb-x86_64-ubuntu-14.04-cmake | 18:17:33
 sanitizer-ppc64le-linux| 16:32:23
 sanitizer-ppc64be-linux| 15:04:16
 clang-s390x-linux  | 12:08:15
 clang-bpf-build| 12:06:25
 clang-cmake-mips   | 12:05:12
 clang-cmake-aarch64-42vma  | 11:51:35
 clang-ppc64le-linux-multistage | 11:50:41
 clang-cmake-aarch64-39vma  | 11:48:43
 clang-ppc64le-linux-lnt| 11:43:54
 clang-ppc64le-linux| 11:38:49
 clang-cmake-armv7-a15-full | 11:22:50
 clang-3stage-ubuntu| 11:18:37
 sanitizer-x86_64-linux-autoconf| 11:18:36
 clang-cmake-aarch64-full   | 11:10:22
 clang-cmake-thumbv7-a15-full-sh| 10:58:01
 llvm-mips-linux| 10:38:01
 sanitizer-x86_64-linux | 10:20:27
 clang-with-lto-ubuntu  | 07:00:00
 lld-x86_64-darwin13| 06:46:01
 libcxx-libcxxabi-libunwind-arm-linux-noexceptions  | 06:44:50
 perf-x86_64-penryn-O3-polly-fast   | 06:39:56
 lld-x86_64-freebsd | 06:39:21
 clang-with-thin-lto-ubuntu | 06:10:54
 clang-x86_64-linux-selfhost-modules| 05:54:48
 clang-native-arm-lnt   | 05:06:35
 sanitizer-x86_64-linux-fuzzer  | 04:06:08
 libcxx-libcxxabi-libunwind-arm-linux   | 02:55:40
 clang-x64-ninja-win7   | 02:36:05
 polly-amd64-linux  | 02:34:16
 clang-hexagon-elf  | 02:11:00
 clang-cmake-aarch64-quick  | 02:10:56
 clang-x86-windows-msvc2015 | 02:10:44
 perf-x86_64-penryn-O3-polly-unprofitable   | 02:02:50
 libcxx-libcxxabi-libunwind-x86_64-linux-ubuntu | 01:53:59
 clang-cmake-armv7-a15  | 01:52:56
 clang-cmake-thumbv7-a15| 01:52:54
 libcxx-libcxxabi-x86_64-linux-ubuntu-ubsan | 01:51:30
 libcxx-libcxxabi-x86_64-linux-ubuntu-asan  | 01:46:08
 lldb-amd64-ninja-netbsd7   | 01:41:54
 clang-x86_64-linux-abi-test| 01:40:39
 lldb-x86_64-ubuntu-14.04-buildserver   | 01:35:27
 libcxx-libcxxabi-x86_64-linux-ubuntu-msan  | 01:34:56
 libcxx-libcxxabi-x86_64-linux-ubuntu-tsan  | 01:31:22
 perf-x86_64-penryn-O3-polly-parallel-fast  | 01:30:39
 perf-x86_64-penryn-O3-polly-before-vectorizer-unprofitable | 01:26:30
 llvm-sphinx-docs   | 01:19:51
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast   | 01:12:31
 libcxx-libcxxabi-x86_

[Lldb-commits] [lldb] r287307 - Remove some dead code in ValueObject.

2016-11-17 Thread Zachary Turner via lldb-commits 
Author: zturner
Date: Thu Nov 17 21:51:19 2016
New Revision: 287307

URL: http://llvm.org/viewvc/llvm-project?rev=287307&view=rev
Log:
Remove some dead code in ValueObject.

Originally I converted this entire function and all dependents
to use StringRef, but there were some test failures that
were tricky to track down, as this is a complicated function.
So I'm starting over, this time in smaller increments.

Modified:
lldb/trunk/source/Core/ValueObject.cpp

Modified: lldb/trunk/source/Core/ValueObject.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObject.cpp?rev=287307&r1=287306&r2=287307&view=diff
==
--- lldb/trunk/source/Core/ValueObject.cpp (original)
+++ lldb/trunk/source/Core/ValueObject.cpp Thu Nov 17 21:51:19 2016
@@ -2607,14 +2607,15 @@ ValueObjectSP ValueObject::GetValueForEx
 *final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
 return ValueObjectSP();
   }
-  if (!separator_position ||
-  separator_position > close_bracket_position) // if no separator, this
-   // is either [] or [N]
-  {
+
+  if (!separator_position || separator_position > close_bracket_position) {
+// if no separator, this is of the form [N].  Note that this cannot
+// be an unbounded range of the form [], because that case was handled
+// above with an unconditional return.
 char *end = NULL;
 unsigned long index = ::strtoul(expression_cstr + 1, &end, 0);
-if (!end || end != close_bracket_position) // if something weird is in
-   // our way return an error
+if (end != close_bracket_position) // if something weird is in
+   // our way return an error
 {
   *first_unparsed = expression_cstr;
   *reason_to_stop =
@@ -2622,24 +2623,6 @@ ValueObjectSP ValueObject::GetValueForEx
   *final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
   return ValueObjectSP();
 }
-if (end - expression_cstr ==
-1) // if this is [], only return a valid value for arrays
-{
-  if (root_compiler_type_info.Test(eTypeIsArray)) {
-*first_unparsed = expression_cstr + 2;
-*reason_to_stop =
-ValueObject::eExpressionPathScanEndReasonArrayRangeOperatorMet;
-*final_result =
-ValueObject::eExpressionPathEndResultTypeUnboundedRange;
-return root;
-  } else {
-*first_unparsed = expression_cstr;
-*reason_to_stop =
-ValueObject::eExpressionPathScanEndReasonEmptyRangeNotAllowed;
-*final_result = ValueObject::eExpressionPathEndResultTypeInvalid;
-return ValueObjectSP();
-  }
-}
 // from here on we do have a valid index
 if (root_compiler_type_info.Test(eTypeIsArray)) {
   ValueObjectSP child_valobj_sp = root->GetChildAtIndex(index, true);
@@ -2791,8 +2774,8 @@ ValueObjectSP ValueObject::GetValueForEx
   {
 char *end = NULL;
 unsigned long index_lower = ::strtoul(expression_cstr + 1, &end, 0);
-if (!end || end != separator_position) // if something weird is in our
-   // way return an error
+if (end != separator_position) // if something weird is in our
+   // way return an error
 {
   *first_unparsed = expression_cstr;
   *reason_to_stop =
@@ -2801,8 +2784,8 @@ ValueObjectSP ValueObject::GetValueForEx
   return ValueObjectSP();
 }
 unsigned long index_higher = ::strtoul(separator_position + 1, &end, 
0);
-if (!end || end != close_bracket_position) // if something weird is in
-   // our way return an error
+if (end != close_bracket_position) // if something weird is in
+   // our way return an error
 {
   *first_unparsed = expression_cstr;
   *reason_to_stop =
@@ -2811,11 +2794,8 @@ ValueObjectSP ValueObject::GetValueForEx
   return ValueObjectSP();
 }
 if (index_lower > index_higher) // swap indices if required
-{
-  unsigned long temp = index_lower;
-  index_lower = index_higher;
-  index_higher = temp;
-}
+  std::swap(index_lower, index_higher);
+
 if (root_compiler_type_info.Test(
 eTypeIsScalar)) // expansion only works for scalars
 {
@@ -2975,8 +2955,8 @@ int ValueObject::ExpandArraySliceExpress
   {
 char *end = NULL;
 unsigned long index = ::strtoul(expression_cstr + 1, &end, 0);
-if (!end || end != close_bracket_

[Lldb-commits] [lldb] r287308 - Delete more dead code in ValueObject.

2016-11-17 Thread Zachary Turner via lldb-commits 
Author: zturner
Date: Thu Nov 17 22:30:47 2016
New Revision: 287308

URL: http://llvm.org/viewvc/llvm-project?rev=287308&view=rev
Log:
Delete more dead code in ValueObject.

Apparently these two enormous functions were dead.  Which is
good, since one was largely a copy of another function with
only a few minor tweaks.

Modified:
lldb/trunk/include/lldb/Core/ValueObject.h
lldb/trunk/source/Core/ValueObject.cpp

Modified: lldb/trunk/include/lldb/Core/ValueObject.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ValueObject.h?rev=287308&r1=287307&r2=287308&view=diff
==
--- lldb/trunk/include/lldb/Core/ValueObject.h (original)
+++ lldb/trunk/include/lldb/Core/ValueObject.h Thu Nov 17 22:30:47 2016
@@ -401,15 +401,6 @@ public:
   GetValueForExpressionPathOptions::DefaultOptions(),
   ExpressionPathAftermath *final_task_on_target = nullptr);
 
-  int GetValuesForExpressionPath(
-  const char *expression, lldb::ValueObjectListSP &list,
-  const char **first_unparsed = nullptr,
-  ExpressionPathScanEndReason *reason_to_stop = nullptr,
-  ExpressionPathEndResultType *final_value_type = nullptr,
-  const GetValueForExpressionPathOptions &options =
-  GetValueForExpressionPathOptions::DefaultOptions(),
-  ExpressionPathAftermath *final_task_on_target = nullptr);
-
   virtual bool IsInScope() { return true; }
 
   virtual lldb::offset_t GetByteOffset() { return 0; }
@@ -1015,18 +1006,6 @@ private:
   ExpressionPathScanEndReason *reason_to_stop,
   ExpressionPathEndResultType *final_value_type,
   const GetValueForExpressionPathOptions &options,
-  ExpressionPathAftermath *final_task_on_target);
-
-  // this method will ONLY expand [] expressions into a VOList and return
-  // the number of elements it added to the VOList
-  // it will NOT loop through expanding the follow-up of the expression_cstr
-  // for all objects in the list
-  int ExpandArraySliceExpression(
-  const char *expression_cstr, const char **first_unparsed,
-  lldb::ValueObjectSP root, lldb::ValueObjectListSP &list,
-  ExpressionPathScanEndReason *reason_to_stop,
-  ExpressionPathEndResultType *final_value_type,
-  const GetValueForExpressionPathOptions &options,
   ExpressionPathAftermath *final_task_on_target);
 
   DISALLOW_COPY_AND_ASSIGN(ValueObject);

Modified: lldb/trunk/source/Core/ValueObject.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObject.cpp?rev=287308&r1=287307&r2=287308&view=diff
==
--- lldb/trunk/source/Core/ValueObject.cpp (original)
+++ lldb/trunk/source/Core/ValueObject.cpp Thu Nov 17 22:30:47 2016
@@ -2236,92 +2236,6 @@ ValueObjectSP ValueObject::GetValueForEx
   // you know I did not do it
 }
 
-int ValueObject::GetValuesForExpressionPath(
-const char *expression, ValueObjectListSP &list,
-const char **first_unparsed, ExpressionPathScanEndReason *reason_to_stop,
-ExpressionPathEndResultType *final_value_type,
-const GetValueForExpressionPathOptions &options,
-ExpressionPathAftermath *final_task_on_target) {
-  const char *dummy_first_unparsed;
-  ExpressionPathScanEndReason dummy_reason_to_stop;
-  ExpressionPathEndResultType dummy_final_value_type;
-  ExpressionPathAftermath dummy_final_task_on_target =
-  ValueObject::eExpressionPathAftermathNothing;
-
-  ValueObjectSP ret_val = GetValueForExpressionPath_Impl(
-  expression, first_unparsed ? first_unparsed : &dummy_first_unparsed,
-  reason_to_stop ? reason_to_stop : &dummy_reason_to_stop,
-  final_value_type ? final_value_type : &dummy_final_value_type, options,
-  final_task_on_target ? final_task_on_target
-   : &dummy_final_task_on_target);
-
-  if (!ret_val.get()) // if there are errors, I add nothing to the list
-return 0;
-
-  if ((reason_to_stop ? *reason_to_stop : dummy_reason_to_stop) !=
-  eExpressionPathScanEndReasonArrayRangeOperatorMet) {
-// I need not expand a range, just post-process the final value and return
-if (!final_task_on_target ||
-*final_task_on_target == ValueObject::eExpressionPathAftermathNothing) 
{
-  list->Append(ret_val);
-  return 1;
-}
-if (ret_val.get() &&
-(final_value_type ? *final_value_type : dummy_final_value_type) ==
-eExpressionPathEndResultTypePlain) // I can only deref and
-   // takeaddress of plain objects
-{
-  if (*final_task_on_target ==
-  ValueObject::eExpressionPathAftermathDereference) {
-Error error;
-ValueObjectSP final_value = ret_val->Dereference(error);
-if (error.Fail() || !final_value.get()) {
-  if (reason_to_stop)
-*reason_to_stop =
-ValueObject::eExpressionPathScanEn

[Lldb-commits] [lldb] r287315 - Remove an out param from ValueObject::GetValueForExpressionPath.

2016-11-17 Thread Zachary Turner via lldb-commits 
Author: zturner
Date: Thu Nov 17 23:45:41 2016
New Revision: 287315

URL: http://llvm.org/viewvc/llvm-project?rev=287315&view=rev
Log:
Remove an out param from ValueObject::GetValueForExpressionPath.

This argument was only used in one place in the codebase, and
it was in a non-critical log statement and can be easily
substituted for an equally meaningful field instead.  The
payoff of computing this value is not worth the added
complexity.

Modified:
lldb/trunk/include/lldb/Core/ValueObject.h
lldb/trunk/source/Core/FormatEntity.cpp
lldb/trunk/source/Core/ValueObject.cpp
lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/trunk/source/Symbol/Variable.cpp

Modified: lldb/trunk/include/lldb/Core/ValueObject.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ValueObject.h?rev=287315&r1=287314&r2=287315&view=diff
==
--- lldb/trunk/include/lldb/Core/ValueObject.h (original)
+++ lldb/trunk/include/lldb/Core/ValueObject.h Thu Nov 17 23:45:41 2016
@@ -394,7 +394,7 @@ public:
   GetExpressionPathFormat = eGetExpressionPathFormatDereferencePointers);
 
   lldb::ValueObjectSP GetValueForExpressionPath(
-  const char *expression, const char **first_unparsed = nullptr,
+  const char *expression,
   ExpressionPathScanEndReason *reason_to_stop = nullptr,
   ExpressionPathEndResultType *final_value_type = nullptr,
   const GetValueForExpressionPathOptions &options =
@@ -1002,8 +1002,7 @@ private:
   virtual CompilerType MaybeCalculateCompleteType();
 
   lldb::ValueObjectSP GetValueForExpressionPath_Impl(
-  const char *expression_cstr, const char **first_unparsed,
-  ExpressionPathScanEndReason *reason_to_stop,
+  const char *expression_cstr, ExpressionPathScanEndReason *reason_to_stop,
   ExpressionPathEndResultType *final_value_type,
   const GetValueForExpressionPathOptions &options,
   ExpressionPathAftermath *final_task_on_target);

Modified: lldb/trunk/source/Core/FormatEntity.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/FormatEntity.cpp?rev=287315&r1=287314&r2=287315&view=diff
==
--- lldb/trunk/source/Core/FormatEntity.cpp (original)
+++ lldb/trunk/source/Core/FormatEntity.cpp Thu Nov 17 23:45:41 2016
@@ -614,7 +614,6 @@ static ValueObjectSP ExpandIndexedExpres
   if (log)
 log->Printf("[ExpandIndexedExpression] name to deref: %s",
 ptr_deref_buffer.c_str());
-  const char *first_unparsed;
   ValueObject::GetValueForExpressionPathOptions options;
   ValueObject::ExpressionPathEndResultType final_value_type;
   ValueObject::ExpressionPathScanEndReason reason_to_stop;
@@ -622,20 +621,18 @@ static ValueObjectSP ExpandIndexedExpres
   (deref_pointer ? ValueObject::eExpressionPathAftermathDereference
  : ValueObject::eExpressionPathAftermathNothing);
   ValueObjectSP item = valobj->GetValueForExpressionPath(
-  ptr_deref_buffer.c_str(), &first_unparsed, &reason_to_stop,
-  &final_value_type, options, &what_next);
+  ptr_deref_buffer.c_str(), &reason_to_stop, &final_value_type, options,
+  &what_next);
   if (!item) {
 if (log)
-  log->Printf("[ExpandIndexedExpression] ERROR: unparsed portion = %s, why 
"
-  "stopping = %d,"
+  log->Printf("[ExpandIndexedExpression] ERROR: why stopping = %d,"
   " final_value_type %d",
-  first_unparsed, reason_to_stop, final_value_type);
+  reason_to_stop, final_value_type);
   } else {
 if (log)
-  log->Printf("[ExpandIndexedExpression] ALL RIGHT: unparsed portion = %s, 
"
-  "why stopping = %d,"
-  " final_value_type %d",
-  first_unparsed, reason_to_stop, final_value_type);
+  log->Printf("[ExpandIndexedExpression] ALL RIGHT: why stopping = %d, "
+  "final_value_type %d",
+  reason_to_stop, final_value_type);
   }
   return item;
 }
@@ -724,7 +721,6 @@ static bool DumpValue(Stream &s, const S
   int64_t index_lower = -1;
   int64_t index_higher = -1;
   bool is_array_range = false;
-  const char *first_unparsed;
   bool was_plain_var = false;
   bool was_var_format = false;
   bool was_var_indexed = false;
@@ -762,25 +758,23 @@ static bool DumpValue(Stream &s, const S
   log->Printf("[Debugger::FormatPrompt] symbol to expand: %s",
   expr_path.c_str());
 
-target = valobj
- ->GetValueForExpressionPath(expr_path.c_str(), 
&first_unparsed,
- &reason_to_stop, 
&final_value_type,
- options, &what_next)
- .get();
+target =
+valobj
+->GetValueForExpressionPath(expr_path.c_str(), &reason_to_stop,
+

[Lldb-commits] [lldb] r287320 - Revert "Remove an out param from ValueObject::GetValueForExpressionPath."

2016-11-17 Thread Zachary Turner via lldb-commits 
Author: zturner
Date: Fri Nov 18 00:34:45 2016
New Revision: 287320

URL: http://llvm.org/viewvc/llvm-project?rev=287320&view=rev
Log:
Revert "Remove an out param from ValueObject::GetValueForExpressionPath."

This reverts commit r287315, as it introduces a bug that breaks
many things.

Modified:
lldb/trunk/include/lldb/Core/ValueObject.h
lldb/trunk/source/Core/FormatEntity.cpp
lldb/trunk/source/Core/ValueObject.cpp
lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/trunk/source/Symbol/Variable.cpp

Modified: lldb/trunk/include/lldb/Core/ValueObject.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ValueObject.h?rev=287320&r1=287319&r2=287320&view=diff
==
--- lldb/trunk/include/lldb/Core/ValueObject.h (original)
+++ lldb/trunk/include/lldb/Core/ValueObject.h Fri Nov 18 00:34:45 2016
@@ -394,7 +394,7 @@ public:
   GetExpressionPathFormat = eGetExpressionPathFormatDereferencePointers);
 
   lldb::ValueObjectSP GetValueForExpressionPath(
-  const char *expression,
+  const char *expression, const char **first_unparsed = nullptr,
   ExpressionPathScanEndReason *reason_to_stop = nullptr,
   ExpressionPathEndResultType *final_value_type = nullptr,
   const GetValueForExpressionPathOptions &options =
@@ -1002,7 +1002,8 @@ private:
   virtual CompilerType MaybeCalculateCompleteType();
 
   lldb::ValueObjectSP GetValueForExpressionPath_Impl(
-  const char *expression_cstr, ExpressionPathScanEndReason *reason_to_stop,
+  const char *expression_cstr, const char **first_unparsed,
+  ExpressionPathScanEndReason *reason_to_stop,
   ExpressionPathEndResultType *final_value_type,
   const GetValueForExpressionPathOptions &options,
   ExpressionPathAftermath *final_task_on_target);

Modified: lldb/trunk/source/Core/FormatEntity.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/FormatEntity.cpp?rev=287320&r1=287319&r2=287320&view=diff
==
--- lldb/trunk/source/Core/FormatEntity.cpp (original)
+++ lldb/trunk/source/Core/FormatEntity.cpp Fri Nov 18 00:34:45 2016
@@ -614,6 +614,7 @@ static ValueObjectSP ExpandIndexedExpres
   if (log)
 log->Printf("[ExpandIndexedExpression] name to deref: %s",
 ptr_deref_buffer.c_str());
+  const char *first_unparsed;
   ValueObject::GetValueForExpressionPathOptions options;
   ValueObject::ExpressionPathEndResultType final_value_type;
   ValueObject::ExpressionPathScanEndReason reason_to_stop;
@@ -621,18 +622,20 @@ static ValueObjectSP ExpandIndexedExpres
   (deref_pointer ? ValueObject::eExpressionPathAftermathDereference
  : ValueObject::eExpressionPathAftermathNothing);
   ValueObjectSP item = valobj->GetValueForExpressionPath(
-  ptr_deref_buffer.c_str(), &reason_to_stop, &final_value_type, options,
-  &what_next);
+  ptr_deref_buffer.c_str(), &first_unparsed, &reason_to_stop,
+  &final_value_type, options, &what_next);
   if (!item) {
 if (log)
-  log->Printf("[ExpandIndexedExpression] ERROR: why stopping = %d,"
+  log->Printf("[ExpandIndexedExpression] ERROR: unparsed portion = %s, why 
"
+  "stopping = %d,"
   " final_value_type %d",
-  reason_to_stop, final_value_type);
+  first_unparsed, reason_to_stop, final_value_type);
   } else {
 if (log)
-  log->Printf("[ExpandIndexedExpression] ALL RIGHT: why stopping = %d, "
-  "final_value_type %d",
-  reason_to_stop, final_value_type);
+  log->Printf("[ExpandIndexedExpression] ALL RIGHT: unparsed portion = %s, 
"
+  "why stopping = %d,"
+  " final_value_type %d",
+  first_unparsed, reason_to_stop, final_value_type);
   }
   return item;
 }
@@ -721,6 +724,7 @@ static bool DumpValue(Stream &s, const S
   int64_t index_lower = -1;
   int64_t index_higher = -1;
   bool is_array_range = false;
+  const char *first_unparsed;
   bool was_plain_var = false;
   bool was_var_format = false;
   bool was_var_indexed = false;
@@ -758,23 +762,25 @@ static bool DumpValue(Stream &s, const S
   log->Printf("[Debugger::FormatPrompt] symbol to expand: %s",
   expr_path.c_str());
 
-target =
-valobj
-->GetValueForExpressionPath(expr_path.c_str(), &reason_to_stop,
-&final_value_type, options, &what_next)
-.get();
+target = valobj
+ ->GetValueForExpressionPath(expr_path.c_str(), 
&first_unparsed,
+ &reason_to_stop, 
&final_value_type,
+ options, &what_next)
+ .get();
 
 if (!target) {
   if (log)
-log->Printf("[Debugger::FormatPrompt] ERROR: