Re: [Lldb-commits] [lldb] 4c53d48 - [lldb/Test] Don't use the env to pass around configuration variables (NFC)

2020-06-03 Thread Pavel Labath via lldb-commits 
Woohoo!

On 03/06/2020 01:50, Jonas Devlieghere via lldb-commits wrote:
> 
> Author: Jonas Devlieghere
> Date: 2020-06-02T16:49:58-07:00
> New Revision: 4c53d4801cbbb1b573e4ef758f93ead12e1f59a2
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/4c53d4801cbbb1b573e4ef758f93ead12e1f59a2
> DIFF: 
> https://github.com/llvm/llvm-project/commit/4c53d4801cbbb1b573e4ef758f93ead12e1f59a2.diff
> 
> LOG: [lldb/Test] Don't use the env to pass around configuration variables 
> (NFC)
> 
> Don't use the environment to pass values to the builder. Use the
> configuration instead.
> 
> Added: 
> 
> 
> Modified: 
> lldb/packages/Python/lldbsuite/test/configuration.py
> lldb/packages/Python/lldbsuite/test/dotest.py
> lldb/packages/Python/lldbsuite/test/lldbtest.py
> lldb/packages/Python/lldbsuite/test/plugins/builder_base.py
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
> b/lldb/packages/Python/lldbsuite/test/configuration.py
> index 0439c4e8f1ac..f05152253c75 100644
> --- a/lldb/packages/Python/lldbsuite/test/configuration.py
> +++ b/lldb/packages/Python/lldbsuite/test/configuration.py
> @@ -42,8 +42,10 @@
>  count = 1
>  
>  # The 'arch' and 'compiler' can be specified via command line.
> -arch = None# Must be initialized after option parsing
> -compiler = None# Must be initialized after option parsing
> +arch = None
> +compiler = None
> +dsymutil = None
> +sdkroot = None
>  
>  # The overriden dwarf verison.
>  dwarf_version = 0
> 
> diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
> b/lldb/packages/Python/lldbsuite/test/dotest.py
> index 54e5d4bc2df5..80edad811bae 100644
> --- a/lldb/packages/Python/lldbsuite/test/dotest.py
> +++ b/lldb/packages/Python/lldbsuite/test/dotest.py
> @@ -275,9 +275,9 @@ def parseOptionsAndInitTestdirs():
>  break
>  
>  if args.dsymutil:
> -os.environ['DSYMUTIL'] = args.dsymutil
> +configuration.dsymutil = args.dsymutil
>  elif platform_system == 'Darwin':
> -os.environ['DSYMUTIL'] = seven.get_command_output(
> +configuration.dsymutil = seven.get_command_output(
>  'xcrun -find -toolchain default dsymutil')
>  
>  if args.filecheck:
> @@ -302,7 +302,7 @@ def parseOptionsAndInitTestdirs():
>  
>  # Set SDKROOT if we are using an Apple SDK
>  if platform_system == 'Darwin' and args.apple_sdk:
> -os.environ['SDKROOT'] = seven.get_command_output(
> +configuration.sdkroot = seven.get_command_output(
>  'xcrun --sdk "%s" --show-sdk-path 2> /dev/null' %
>  (args.apple_sdk))
>  
> @@ -310,10 +310,10 @@ def parseOptionsAndInitTestdirs():
>  configuration.arch = args.arch
>  if configuration.arch.startswith(
>  'arm') and platform_system == 'Darwin' and not 
> args.apple_sdk:
> -os.environ['SDKROOT'] = seven.get_command_output(
> +configuration.sdkroot = seven.get_command_output(
>  'xcrun --sdk iphoneos.internal --show-sdk-path 2> /dev/null')
> -if not os.path.exists(os.environ['SDKROOT']):
> -os.environ['SDKROOT'] = seven.get_command_output(
> +if not os.path.exists(configuration.sdkroot):
> +configuration.sdkroot = seven.get_command_output(
>  'xcrun --sdk iphoneos --show-sdk-path 2> /dev/null')
>  else:
>  configuration.arch = platform_machine
> @@ -522,7 +522,7 @@ def setupSysPath():
>  # Set up the root build directory.
>  if not configuration.test_build_dir:
>  raise Exception("test_build_dir is not set")
> -os.environ["LLDB_BUILD"] = os.path.abspath(configuration.test_build_dir)
> +configuration.test_build_dir = 
> os.path.abspath(configuration.test_build_dir)
>  
>  # Set up the LLDB_SRC environment variable, so that the tests can locate
>  # the LLDB source code.
> @@ -1041,8 +1041,7 @@ def run_suite():
>  
>  # Set up the working directory.
>  # Note that it's not dotest's job to clean this directory.
> -build_dir = configuration.test_build_dir
> -lldbutil.mkdir_p(build_dir)
> +lldbutil.mkdir_p(configuration.test_build_dir)
>  
>  target_platform = lldb.selected_platform.GetTriple().split('-')[2]
>  
> 
> diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
> b/lldb/packages/Python/lldbsuite/test/lldbtest.py
> index 04ba7ea02d09..0a640d2d5c93 100644
> --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
> +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
> @@ -668,7 +668,7 @@ def getBuildDirBasename(self):
>  
>  def getBuildDir(self):
>  """Return the full path to the current test."""
> -return os.path.join(os.environ["LLDB_BUILD"], self.mydir,
> +return os.path.join(configuration.test_build_dir, self.mydir,
>

[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-03 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added reviewers: LLDB, labath, teemperor, clayborg.
JDevlieghere added a project: LLDB.
Herald added a subscriber: abidh.
JDevlieghere added a parent revision: D81056: [Support] Replace 'DisableColors' 
boolean with 'ColorMode' enum.
JDevlieghere retitled this revision from "[Interpreter] Color "error:" and 
"warning:" in the CommandReturnObject output." to "[lldb/Interpreter] Color 
"error:" and "warning:" in the CommandReturnObject output.".

Color the `error:` and `warning:` part of the CommandReturnObject output, 
similar to how an error is printed from the driver when colors are enabled.

Here's a screenshot of the output: 
https://jonasdevlieghere.com/static/colors.png


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D81058

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/include/lldb/Utility/Stream.h
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp

Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -28,8 +28,9 @@
 strm.EOL();
 }
 
-CommandReturnObject::CommandReturnObject()
+CommandReturnObject::CommandReturnObject(bool colors)
 : m_out_stream(), m_err_stream(), m_status(eReturnStatusStarted),
+  m_colors(colors ? llvm::ColorMode::Enable : llvm::ColorMode::Disable),
   m_did_change_process_state(false), m_interactive(true) {}
 
 CommandReturnObject::~CommandReturnObject() {}
@@ -45,9 +46,10 @@
 
   const std::string &s = std::string(sstrm.GetString());
   if (!s.empty()) {
-Stream &error_strm = GetErrorStream();
-error_strm.PutCString("error: ");
-DumpStringToStreamWithNewline(error_strm, s);
+llvm::WithColor(GetErrorStream().AsRawOstream(),
+llvm::HighlightColor::Error, m_colors)
+<< "error: ";
+DumpStringToStreamWithNewline(GetErrorStream(), s);
   }
 }
 
@@ -72,7 +74,9 @@
   sstrm.PrintfVarArg(format, args);
   va_end(args);
 
-  GetErrorStream() << "warning: " << sstrm.GetString();
+  llvm::WithColor(GetErrorStream().AsRawOstream(),
+  llvm::HighlightColor::Warning, m_colors)
+  << "warning: " << sstrm.GetString();
 }
 
 void CommandReturnObject::AppendMessage(llvm::StringRef in_string) {
@@ -84,7 +88,9 @@
 void CommandReturnObject::AppendWarning(llvm::StringRef in_string) {
   if (in_string.empty())
 return;
-  GetErrorStream() << "warning: " << in_string << "\n";
+  llvm::WithColor(GetErrorStream().AsRawOstream(),
+  llvm::HighlightColor::Warning, m_colors)
+  << "warning: " << in_string << '\n';
 }
 
 // Similar to AppendWarning, but do not prepend 'warning: ' to message, and
@@ -99,7 +105,9 @@
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
   if (in_string.empty())
 return;
-  GetErrorStream() << "error: " << in_string << "\n";
+  llvm::WithColor(GetErrorStream().AsRawOstream(), llvm::HighlightColor::Error,
+  m_colors)
+  << "error: " << in_string << '\n';
 }
 
 void CommandReturnObject::SetError(const Status &error,
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -209,7 +209,7 @@
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
 
-  CommandReturnObject result;
+  CommandReturnObject result(m_debugger.GetUseColor());
 
   LoadCommandDictionary();
 
@@ -2792,7 +2792,7 @@
 
   StartHandlingCommand();
 
-  lldb_private::CommandReturnObject result;
+  lldb_private::CommandReturnObject result(m_debugger.GetUseColor());
   HandleCommand(line.c_str(), eLazyBoolCalculate, result);
 
   // Now emit the command output text from the command we just executed
Index: lldb/include/lldb/Utility/Stream.h
===
--- lldb/include/lldb/Utility/Stream.h
+++ lldb/include/lldb/Utility/Stream.h
@@ -14,6 +14,7 @@
 #include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include 
@@ -402,6 +403,43 @@
   return m_target.GetWrittenBytes();
 }
 
+raw_ostream &changeColor(enum Colors colors, bool bold = false,
+ bool bg = false) override {
+  if (llvm::sys::Process::ColorNeedsFlush())
+flush();
+  const char *colorcode = (colors == SAVEDCOLOR)
+  ? llvm::sys::Process::OutputBold(bg)
+  : llvm::sys::Process::OutputColor(
+static_cast(colors), bold, bg);
+  if (colorcode) {
+siz

[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-03 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

That was also on my list but it seems you beat me to it. I couldn't resist some 
smaller nit-picks but otherwise this LGTM beside that.




Comment at: lldb/include/lldb/Interpreter/CommandReturnObject.h:27
 public:
-  CommandReturnObject();
+  CommandReturnObject(bool colors = false);
 

I think a documentation line like "use_colors Use colors to highlight parts of 
the command output" would make this more obvious (also `use_colors` would bring 
this in line to what we call this setting in other parts of LLDB).



Comment at: lldb/include/lldb/Utility/Stream.h:417
+write(colorcode, len);
+  }
+  return *this;

Nit: This can all be `PutCString(colorcode)` (which handles the nullptr check 
and the strlen). Same as below.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81058/new/

https://reviews.llvm.org/D81058



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

[Lldb-commits] [PATCH] D81032: [lldb/Test] Don't print 'command invoked'

2020-06-03 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

Your patches continue to bring me joy and happiness.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81032/new/

https://reviews.llvm.org/D81032



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

[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-03 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a reviewer: amccarth.
labath added a comment.

I'm wondering if it would be possible/acceptable to move the color-related code 
from `llvm::raw_fd_ostream` (where the `RawOstreamForward` code is copied from) 
into the `raw_ostream` base class (except that it would default to false, of 
course). Outputting the color escape codes to a non-terminal, is not a 
completely unreasonable thing to do. For example, one gather the codes into a 
string stream, and later output the string to a real terminal -- which, 
incidentally, is exactly what we are doing here. Then all we would need is to 
plumb the "color" flag into the appropriate stream object.

One possible objection to that idea might be that this trick will work on 
windows only if we are using ansi emulation, which isn't always possible (I 
believe it's not available before windows 10). However, that objection applies 
regardless of where the feature is implemented -- the only difference is that 
we may be more willing to throw windows under the bus.

Another thing that would be good to check is whether these color codes make 
their way through the SB API (SBCommandReturnObject). I don't think that's a 
bad thing -- the embedding application might actually want to print lldb 
command results in color. And also, it provides a nice way to test this feature.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81058/new/

https://reviews.llvm.org/D81058



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

[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-03 Thread Pavel Labath via Phabricator via lldb-commits 
labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.

Let's talk about this more. I don't think we should cargo-cult code from llvm, 
especially when the cargo-culting won't actually work for the given use case. 
On windows, the `llvm::sys::Process::***` functions will be messing with the 
terminal  which does not sound like a good thing to do from a generic place 
like this.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81058/new/

https://reviews.llvm.org/D81058



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

[Lldb-commits] [PATCH] D81010: [lldb/DWARF] Fix PC value for artificial tail call frames for the "GNU" case

2020-06-03 Thread Pavel Labath via Phabricator via lldb-commits 
labath marked an inline comment as done.
labath added inline comments.



Comment at: lldb/include/lldb/Symbol/Function.h:332
 
-  /// The address of the call instruction. Usually an invalid address, unless
-  /// this is a tail call.
-  lldb::addr_t call_inst_pc;
+  bool caller_address_type : 1, is_tail_call : 1;
 

vsk wrote:
> It seems like there's going to be around sizeof(void *) bytes worth of 
> padding here. Maybe it'd be simpler to store these flags as AddrType and bool 
> respectively? Alternatively, the struct can be marked 'packed'.
Indeed. I was optimizing prematurely.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81010/new/

https://reviews.llvm.org/D81010



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

[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-03 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor added inline comments.



Comment at: lldb/include/lldb/Utility/Stream.h:417
+write(colorcode, len);
+  }
+  return *this;

teemperor wrote:
> Nit: This can all be `PutCString(colorcode)` (which handles the nullptr check 
> and the strlen). Same as below.
Actually nvm, I thought this is in the Stream class (not our own raw_ostream 
subclass).


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81058/new/

https://reviews.llvm.org/D81058



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

[Lldb-commits] [PATCH] D81010: [lldb/DWARF] Fix PC value for artificial tail call frames for the "GNU" case

2020-06-03 Thread Pavel Labath via Phabricator via lldb-commits 
labath updated this revision to Diff 268102.
labath added a comment.

- Unpack the CallEdge structure


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81010/new/

https://reviews.llvm.org/D81010

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp

Index: lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
===
--- lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
+++ lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
@@ -3,12 +3,22 @@
 void __attribute__((noinline)) sink() {
   x++; //% self.filecheck("bt", "main.cpp", "-implicit-check-not=artificial")
   // CHECK: frame #0: 0x{{[0-9a-f]+}} a.out`sink() at main.cpp:[[@LINE-1]]:4
-  // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3() at main.cpp:16:3
+  // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3() at main.cpp:26:3
   // CHECK-SAME: [artificial]
   // CHECK-NEXT: frame #2: 0x{{[0-9a-f]+}} a.out`func2()
-  // CHECK-NEXT: frame #3: 0x{{[0-9a-f]+}} a.out`func1() at main.cpp:25:3
+  // CHECK-NEXT: frame #3: 0x{{[0-9a-f]+}} a.out`func1() at main.cpp:35:3
   // CHECK-SAME: [artificial]
   // CHECK-NEXT: frame #4: 0x{{[0-9a-f]+}} a.out`main
+  // In the GNU style, the artificial frames will point after the tail call
+  // instruction. In v5 they should point to the instruction itself.
+  //% frame1 = self.thread().GetFrameAtIndex(1)
+  //% func3 = frame1.GetFunction()
+  //% func3_insns = func3.GetInstructions(self.target())
+  //% self.trace("func3:\n%s"%func3_insns)
+  //% last_insn = func3_insns.GetInstructionAtIndex(func3_insns.GetSize()-1)
+  //% addr = last_insn.GetAddress()
+  //% if "GNU" in self.name: addr.OffsetAddress(last_insn.GetByteSize())
+  //% self.assertEquals(frame1.GetPCAddress(), addr)
 }
 
 void __attribute__((noinline)) func3() {
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -239,7 +239,12 @@
 /// A sequence of calls that comprise some portion of a backtrace. Each frame
 /// is represented as a pair of a callee (Function *) and an address within the
 /// callee.
-using CallSequence = std::vector>;
+struct CallDescriptor {
+  Function *func;
+  CallEdge::AddrType address_type;
+  addr_t address;
+};
+using CallSequence = std::vector;
 
 /// Find the unique path through the call graph from \p begin (with return PC
 /// \p return_pc) to \p end. On success this path is stored into \p path, and 
@@ -319,14 +324,14 @@
   }
 
   // Search the calls made from this callee.
-  active_path.emplace_back(&callee, LLDB_INVALID_ADDRESS);
+  active_path.push_back(CallDescriptor{&callee});
   for (const auto &edge : callee.GetTailCallingEdges()) {
 Function *next_callee = edge->GetCallee(images, context);
 if (!next_callee)
   continue;
 
-addr_t tail_call_pc = edge->GetCallInstPC(callee, target);
-active_path.back().second = tail_call_pc;
+std::tie(active_path.back().address_type, active_path.back().address) =
+edge->GetCallerAddress(callee, target);
 
 dfs(*edge, *next_callee);
 if (ambiguous)
@@ -400,16 +405,16 @@
 
   // Push synthetic tail call frames.
   for (auto calleeInfo : llvm::reverse(path)) {
-Function *callee = calleeInfo.first;
+Function *callee = calleeInfo.func;
 uint32_t frame_idx = m_frames.size();
 uint32_t concrete_frame_idx = next_frame.GetConcreteFrameIndex();
 addr_t cfa = LLDB_INVALID_ADDRESS;
 bool cfa_is_valid = false;
-addr_t pc = calleeInfo.second;
-// We do not want to subtract 1 from this PC, as it's the actual address
-// of the tail-calling branch instruction. This address is provided by the
-// compiler via DW_AT_call_pc.
-constexpr bool behaves_like_zeroth_frame = true;
+addr_t pc = calleeInfo.address;
+// If the callee address refers to the call instruction, we do not want to
+// subtract 1 from this value.
+const bool behaves_like_zeroth_frame =
+calleeInfo.address_type == CallEdge::AddrType::Call;
 SymbolContext sc;
 callee->CalculateSymbolContext(&sc);
 auto synth_frame = std::make_shared(
Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -145,11 +145,7 @@
 
 lldb::addr_t CallEdge::GetReturnPCAddress(Function &caller,
   Target &target) const {
-  return GetLoadAddress(return_pc, caller, target);
-}
-
-lldb::addr_t CallEdge::GetCa

[Lldb-commits] [lldb] c0ccb58 - [lldb] Pass fewer parameters by non-const reference to DWARFASTParserClang::ParseSingleMember

2020-06-03 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-06-03T12:42:09+02:00
New Revision: c0ccb582c35f29df55ccc421dd5d03a7220456cf

URL: 
https://github.com/llvm/llvm-project/commit/c0ccb582c35f29df55ccc421dd5d03a7220456cf
DIFF: 
https://github.com/llvm/llvm-project/commit/c0ccb582c35f29df55ccc421dd5d03a7220456cf.diff

LOG: [lldb] Pass fewer parameters by non-const reference to 
DWARFASTParserClang::ParseSingleMember

These parameters are only passed on by value or const reference, so we should
do the same when calling this function.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 1540bdbea6ab..2b51be84752a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2385,10 +2385,10 @@ Function 
*DWARFASTParserClang::ParseFunctionFromDWARF(CompileUnit &comp_unit,
 
 void DWARFASTParserClang::ParseSingleMember(
 const DWARFDIE &die, const DWARFDIE &parent_die,
-lldb_private::CompilerType &class_clang_type,
+const lldb_private::CompilerType &class_clang_type,
 const lldb::LanguageType class_language,
 std::vector &member_accessibilities,
-lldb::AccessType &default_accessibility,
+lldb::AccessType default_accessibility,
 DelayedPropertyList &delayed_properties,
 lldb_private::ClangASTImporter::LayoutInfo &layout_info,
 FieldInfo &last_field_info) {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 020d29d65eae..71d978536f8f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -194,10 +194,10 @@ class DWARFASTParserClang : public DWARFASTParser {
 
   void
   ParseSingleMember(const DWARFDIE &die, const DWARFDIE &parent_die,
-lldb_private::CompilerType &class_clang_type,
+const lldb_private::CompilerType &class_clang_type,
 const lldb::LanguageType class_language,
 std::vector &member_accessibilities,
-lldb::AccessType &default_accessibility,
+lldb::AccessType default_accessibility,
 DelayedPropertyList &delayed_properties,
 lldb_private::ClangASTImporter::LayoutInfo &layout_info,
 FieldInfo &last_field_info);



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

[Lldb-commits] [lldb] 884aaf7 - [LLDB] skip TestCreateDuringInstructionStep on arm/linux

2020-06-03 Thread Muhammad Omair Javaid via lldb-commits 

Author: Muhammad Omair Javaid
Date: 2020-06-03T16:18:41+05:00
New Revision: 884aaf7f645a27239923e21112c9817836c696df

URL: 
https://github.com/llvm/llvm-project/commit/884aaf7f645a27239923e21112c9817836c696df
DIFF: 
https://github.com/llvm/llvm-project/commit/884aaf7f645a27239923e21112c9817836c696df.diff

LOG: [LLDB] skip TestCreateDuringInstructionStep on arm/linux

There are sporadic failures in this test on arm. I am marking it
skipped as labath suggested flaky decorators no longer work.

Added: 


Modified: 

lldb/test/API/linux/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py

Removed: 




diff  --git 
a/lldb/test/API/linux/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py
 
b/lldb/test/API/linux/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py
index 59473a3f924e..6e19aa830cd3 100644
--- 
a/lldb/test/API/linux/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py
+++ 
b/lldb/test/API/linux/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py
@@ -18,7 +18,7 @@ class CreateDuringInstructionStepTestCase(TestBase):
 
 @skipUnlessPlatform(['linux'])
 @expectedFailureAndroid('llvm.org/pr24737', archs=['arm'])
-@expectedFlakeyLinux(bugnumber="llvm.org/pr24737")
+@skipIf(oslist=["linux"], archs=["arm"], bugnumber="llvm.org/pr24737")
 def test_step_inst(self):
 self.build(dictionary=self.getBuildFlags())
 exe = self.getBuildArtifact("a.out")



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

Re: [Lldb-commits] [lldb] cbfae97 - [LLDB] Mark TestCreateDuringInstructionStep as flaky on Linux

2020-06-03 Thread Omair Javaid via lldb-commits 
On Tue, 2 Jun 2020 at 14:36, Pavel Labath  wrote:

> On 02/06/2020 11:10, Muhammad Omair Javaid via lldb-commits wrote:
> >
> > Author: Muhammad Omair Javaid
> > Date: 2020-06-02T14:08:28+05:00
> > New Revision: cbfae97ca82b11ab2b54562055c49817baa1e26b
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/cbfae97ca82b11ab2b54562055c49817baa1e26b
> > DIFF:
> https://github.com/llvm/llvm-project/commit/cbfae97ca82b11ab2b54562055c49817baa1e26b.diff
> >
> > LOG: [LLDB] Mark TestCreateDuringInstructionStep as flaky on Linux
> >
> > This patch marks TestCreateDuringInstructionStep.py as flakey for Linux.
> > This is failing randomly on arm/aarch64. I will monitor buildbot and
> > skip it if it fails again.
> >
>
> Hi Omair,
>
> I'd like to note two things:
> - I haven't seen this fail on x86 in a very long time. So, whatever the
> problem is, it seems limited to arm architectures
> - since we've switched to using lit as the test driver, I believe the
> "flaky" decorators don't actually do anything (we should really delete
> them if that's the case).
>
> So all in all, I believe you're better off just skipping this on
> arm+aarch64 until someone can investigate this -- which I am hoping will
> not be too hard, given that this fails fairly frequently.
>

Ack. I have marked it skipped as it was sporadically failing only on
arm/linux. Passes all the time if run independently.


>
> pl
>


-- 
Omair Javaid
www.linaro.org
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 798644e - [Scalar] Fix assignment operator for long long.

2020-06-03 Thread Pavel Labath via lldb-commits 

Author: Andy Yankovsky
Date: 2020-06-03T13:26:25+02:00
New Revision: 798644e0a4524e73e1f95202951f10f2086217c3

URL: 
https://github.com/llvm/llvm-project/commit/798644e0a4524e73e1f95202951f10f2086217c3
DIFF: 
https://github.com/llvm/llvm-project/commit/798644e0a4524e73e1f95202951f10f2086217c3.diff

LOG: [Scalar] Fix assignment operator for long long.

Summary:
Assignment operator `operator=(long long)` currently allocates `sizeof(long)`.
On some platforms it works as they have `sizeof(long) == sizeof(long long)`,
but on others (e.g. Windows) it's not the case.

Reviewed By: labath

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

Added: 


Modified: 
lldb/source/Utility/Scalar.cpp
lldb/unittests/Utility/ScalarTest.cpp

Removed: 




diff  --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index b6db5ccaebc4..e55aa2d4d5a4 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -331,7 +331,7 @@ Scalar &Scalar::operator=(unsigned long v) {
 
 Scalar &Scalar::operator=(long long v) {
   m_type = e_slonglong;
-  m_integer = llvm::APInt(sizeof(long) * 8, v, true);
+  m_integer = llvm::APInt(sizeof(long long) * 8, v, true);
   return *this;
 }
 

diff  --git a/lldb/unittests/Utility/ScalarTest.cpp 
b/lldb/unittests/Utility/ScalarTest.cpp
index f62173fb1401..baf1de98c121 100644
--- a/lldb/unittests/Utility/ScalarTest.cpp
+++ b/lldb/unittests/Utility/ScalarTest.cpp
@@ -188,6 +188,16 @@ TEST(ScalarTest, GetValue) {
 ScalarGetValue(std::numeric_limits::max()));
 }
 
+TEST(ScalarTest, LongLongAssigmentOperator) {
+  Scalar ull;
+  ull = std::numeric_limits::max();
+  EXPECT_EQ(std::numeric_limits::max(), ull.ULongLong());
+
+  Scalar sll;
+  sll = std::numeric_limits::max();
+  EXPECT_EQ(std::numeric_limits::max(), sll.SLongLong());
+}
+
 TEST(ScalarTest, Division) {
   Scalar lhs(5.0);
   Scalar rhs(2.0);



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

[Lldb-commits] [lldb] a48c76c - [lldb/cmake] Tweak descriptions of swig rules

2020-06-03 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2020-06-03T13:26:25+02:00
New Revision: a48c76cf43806c30bab5e8eacd5a08058c83c578

URL: 
https://github.com/llvm/llvm-project/commit/a48c76cf43806c30bab5e8eacd5a08058c83c578
DIFF: 
https://github.com/llvm/llvm-project/commit/a48c76cf43806c30bab5e8eacd5a08058c83c578.diff

LOG: [lldb/cmake] Tweak descriptions of swig rules

This descriptions are printed while running the command, and so the
continuous tense is more appropriate and consistent.

Added: 


Modified: 
lldb/bindings/CMakeLists.txt

Removed: 




diff  --git a/lldb/bindings/CMakeLists.txt b/lldb/bindings/CMakeLists.txt
index ace34191a3c5..a2e51c263f7e 100644
--- a/lldb/bindings/CMakeLists.txt
+++ b/lldb/bindings/CMakeLists.txt
@@ -48,7 +48,7 @@ if (LLDB_ENABLE_PYTHON)
 -o ${CMAKE_CURRENT_BINARY_DIR}/LLDBWrapPython.cpp
 ${LLDB_SOURCE_DIR}/bindings/python.swig
 VERBATIM
-COMMENT "Builds LLDB Python wrapper")
+COMMENT "Building LLDB Python wrapper")
 
   add_custom_target(swig_wrapper ALL DEPENDS
 ${CMAKE_CURRENT_BINARY_DIR}/LLDBWrapPython.cpp
@@ -69,7 +69,7 @@ if (LLDB_ENABLE_LUA)
 -o ${CMAKE_CURRENT_BINARY_DIR}/LLDBWrapLua.cpp
 ${LLDB_SOURCE_DIR}/bindings/lua.swig
 VERBATIM
-COMMENT "Builds LLDB Lua wrapper")
+COMMENT "Building LLDB Lua wrapper")
 
   add_custom_target(swig_wrapper_lua ALL DEPENDS
 ${CMAKE_CURRENT_BINARY_DIR}/LLDBWrapLua.cpp



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

[Lldb-commits] [PATCH] D80995: [Scalar] Fix assignment operator for long long.

2020-06-03 Thread Pavel Labath via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG798644e0a452: [Scalar] Fix assignment operator for long 
long. (authored by werat, committed by labath).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80995/new/

https://reviews.llvm.org/D80995

Files:
  lldb/source/Utility/Scalar.cpp
  lldb/unittests/Utility/ScalarTest.cpp


Index: lldb/unittests/Utility/ScalarTest.cpp
===
--- lldb/unittests/Utility/ScalarTest.cpp
+++ lldb/unittests/Utility/ScalarTest.cpp
@@ -188,6 +188,16 @@
 ScalarGetValue(std::numeric_limits::max()));
 }
 
+TEST(ScalarTest, LongLongAssigmentOperator) {
+  Scalar ull;
+  ull = std::numeric_limits::max();
+  EXPECT_EQ(std::numeric_limits::max(), ull.ULongLong());
+
+  Scalar sll;
+  sll = std::numeric_limits::max();
+  EXPECT_EQ(std::numeric_limits::max(), sll.SLongLong());
+}
+
 TEST(ScalarTest, Division) {
   Scalar lhs(5.0);
   Scalar rhs(2.0);
Index: lldb/source/Utility/Scalar.cpp
===
--- lldb/source/Utility/Scalar.cpp
+++ lldb/source/Utility/Scalar.cpp
@@ -331,7 +331,7 @@
 
 Scalar &Scalar::operator=(long long v) {
   m_type = e_slonglong;
-  m_integer = llvm::APInt(sizeof(long) * 8, v, true);
+  m_integer = llvm::APInt(sizeof(long long) * 8, v, true);
   return *this;
 }
 


Index: lldb/unittests/Utility/ScalarTest.cpp
===
--- lldb/unittests/Utility/ScalarTest.cpp
+++ lldb/unittests/Utility/ScalarTest.cpp
@@ -188,6 +188,16 @@
 ScalarGetValue(std::numeric_limits::max()));
 }
 
+TEST(ScalarTest, LongLongAssigmentOperator) {
+  Scalar ull;
+  ull = std::numeric_limits::max();
+  EXPECT_EQ(std::numeric_limits::max(), ull.ULongLong());
+
+  Scalar sll;
+  sll = std::numeric_limits::max();
+  EXPECT_EQ(std::numeric_limits::max(), sll.SLongLong());
+}
+
 TEST(ScalarTest, Division) {
   Scalar lhs(5.0);
   Scalar rhs(2.0);
Index: lldb/source/Utility/Scalar.cpp
===
--- lldb/source/Utility/Scalar.cpp
+++ lldb/source/Utility/Scalar.cpp
@@ -331,7 +331,7 @@
 
 Scalar &Scalar::operator=(long long v) {
   m_type = e_slonglong;
-  m_integer = llvm::APInt(sizeof(long) * 8, v, true);
+  m_integer = llvm::APInt(sizeof(long long) * 8, v, true);
   return *this;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 96e7d19 - [lldb/Test] Don't print 'command invoked'

2020-06-03 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2020-06-03T09:35:09-07:00
New Revision: 96e7d19a1f22f5d2018377b70b4c98e0f59ea412

URL: 
https://github.com/llvm/llvm-project/commit/96e7d19a1f22f5d2018377b70b4c98e0f59ea412
DIFF: 
https://github.com/llvm/llvm-project/commit/96e7d19a1f22f5d2018377b70b4c98e0f59ea412.diff

LOG: [lldb/Test] Don't print 'command invoked'

The different tools constructing dotest invocations (lit and
lldb-dotest) already print the command invocation so there's no need to
print it again in the dotest output.

My motivation for removing it is that it doesn't include the Python
interpreter and every time I accidentally copy it, the command fails
with an `ImportError`.

Differential revision: https://reviews.llvm.org/D81032

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/dotest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 80edad811bae..8668ddb0fb82 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -49,9 +49,6 @@
 from lldbsuite.test_event.event_builder import EventBuilder
 from ..support import seven
 
-def get_dotest_invocation():
-return ' '.join(sys.argv)
-
 
 def is_exe(fpath):
 """Returns true if fpath is an executable."""
@@ -220,7 +217,6 @@ def parseOptionsAndInitTestdirs():
 parser = dotest_args.create_parser()
 args = parser.parse_args()
 except:
-print(get_dotest_invocation())
 raise
 
 if args.unset_env_varnames:
@@ -243,10 +239,6 @@ def parseOptionsAndInitTestdirs():
 if args.set_inferior_env_vars:
 lldbtest_config.inferior_env = ' '.join(args.set_inferior_env_vars)
 
-# Only print the args if being verbose.
-if args.v:
-print(get_dotest_invocation())
-
 if args.h:
 do_help = True
 
@@ -1081,7 +1073,6 @@ def run_suite():
 "\nSession logs for test failures/errors/unexpected successes"
 " will go into directory '%s'\n" %
 configuration.sdir_name)
-sys.stderr.write("Command invoked: %s\n" % get_dotest_invocation())
 
 #
 # Invoke the default TextTestRunner to run the test suite



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

[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-03 Thread Adrian McCarthy via Phabricator via lldb-commits 
amccarth added a comment.

I'm just responding to the questions @labath raised without really looking at 
the details of the code.

I agree that there's nothing wrong with having the ability to output color 
codes to a file, but it's a surprising default and experience tells me it would 
break a lot of tests that are rightfully looking just at the content rather 
than the styling.  But having the ability to turn on color for file output 
would make it possible to also test styling, an ability we don't currently have.

Plumbing the option for color and other niceties down to a low level sounds 
like a great idea, as long as we get the defaults right.  For Windows (for now) 
and for output redirected to a file, the default should be off.  That said, I 
could see lowering of the color option being a separate effort outside the 
scope of this patch.  For one thing, we'd have to get buy-in from the more 
general LLVM folks.  And several of the llvm utilities currently use the 
`WithColor` approach, so there could be a lot of work updating those (or they 
would continue to use the old approach indefinitely).


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81058/new/

https://reviews.llvm.org/D81058



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

[Lldb-commits] [PATCH] D81032: [lldb/Test] Don't print 'command invoked'

2020-06-03 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG96e7d19a1f22: [lldb/Test] Don't print 'command 
invoked' (authored by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81032/new/

https://reviews.llvm.org/D81032

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -49,9 +49,6 @@
 from lldbsuite.test_event.event_builder import EventBuilder
 from ..support import seven
 
-def get_dotest_invocation():
-return ' '.join(sys.argv)
-
 
 def is_exe(fpath):
 """Returns true if fpath is an executable."""
@@ -220,7 +217,6 @@
 parser = dotest_args.create_parser()
 args = parser.parse_args()
 except:
-print(get_dotest_invocation())
 raise
 
 if args.unset_env_varnames:
@@ -243,10 +239,6 @@
 if args.set_inferior_env_vars:
 lldbtest_config.inferior_env = ' '.join(args.set_inferior_env_vars)
 
-# Only print the args if being verbose.
-if args.v:
-print(get_dotest_invocation())
-
 if args.h:
 do_help = True
 
@@ -1081,7 +1073,6 @@
 "\nSession logs for test failures/errors/unexpected successes"
 " will go into directory '%s'\n" %
 configuration.sdir_name)
-sys.stderr.write("Command invoked: %s\n" % get_dotest_invocation())
 
 #
 # Invoke the default TextTestRunner to run the test suite


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -49,9 +49,6 @@
 from lldbsuite.test_event.event_builder import EventBuilder
 from ..support import seven
 
-def get_dotest_invocation():
-return ' '.join(sys.argv)
-
 
 def is_exe(fpath):
 """Returns true if fpath is an executable."""
@@ -220,7 +217,6 @@
 parser = dotest_args.create_parser()
 args = parser.parse_args()
 except:
-print(get_dotest_invocation())
 raise
 
 if args.unset_env_varnames:
@@ -243,10 +239,6 @@
 if args.set_inferior_env_vars:
 lldbtest_config.inferior_env = ' '.join(args.set_inferior_env_vars)
 
-# Only print the args if being verbose.
-if args.v:
-print(get_dotest_invocation())
-
 if args.h:
 do_help = True
 
@@ -1081,7 +1073,6 @@
 "\nSession logs for test failures/errors/unexpected successes"
 " will go into directory '%s'\n" %
 configuration.sdir_name)
-sys.stderr.write("Command invoked: %s\n" % get_dotest_invocation())
 
 #
 # Invoke the default TextTestRunner to run the test suite
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D81058: [lldb/Interpreter] Color "error:" and "warning:" in the CommandReturnObject output.

2020-06-03 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 268258.
JDevlieghere added a comment.

Rebase on top of parent patches.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81058/new/

https://reviews.llvm.org/D81058

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/include/lldb/Utility/Stream.h
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp

Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -28,8 +28,9 @@
 strm.EOL();
 }
 
-CommandReturnObject::CommandReturnObject()
+CommandReturnObject::CommandReturnObject(bool colors)
 : m_out_stream(), m_err_stream(), m_status(eReturnStatusStarted),
+  m_colors(colors ? llvm::ColorMode::Enable : llvm::ColorMode::Disable),
   m_did_change_process_state(false), m_interactive(true) {}
 
 CommandReturnObject::~CommandReturnObject() {}
@@ -45,9 +46,10 @@
 
   const std::string &s = std::string(sstrm.GetString());
   if (!s.empty()) {
-Stream &error_strm = GetErrorStream();
-error_strm.PutCString("error: ");
-DumpStringToStreamWithNewline(error_strm, s);
+llvm::WithColor(GetErrorStream().AsRawOstream(),
+llvm::HighlightColor::Error, m_colors)
+<< "error: ";
+DumpStringToStreamWithNewline(GetErrorStream(), s);
   }
 }
 
@@ -72,7 +74,9 @@
   sstrm.PrintfVarArg(format, args);
   va_end(args);
 
-  GetErrorStream() << "warning: " << sstrm.GetString();
+  llvm::WithColor(GetErrorStream().AsRawOstream(),
+  llvm::HighlightColor::Warning, m_colors)
+  << "warning: " << sstrm.GetString();
 }
 
 void CommandReturnObject::AppendMessage(llvm::StringRef in_string) {
@@ -84,7 +88,9 @@
 void CommandReturnObject::AppendWarning(llvm::StringRef in_string) {
   if (in_string.empty())
 return;
-  GetErrorStream() << "warning: " << in_string << "\n";
+  llvm::WithColor(GetErrorStream().AsRawOstream(),
+  llvm::HighlightColor::Warning, m_colors)
+  << "warning: " << in_string << '\n';
 }
 
 // Similar to AppendWarning, but do not prepend 'warning: ' to message, and
@@ -99,7 +105,9 @@
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
   if (in_string.empty())
 return;
-  GetErrorStream() << "error: " << in_string << "\n";
+  llvm::WithColor(GetErrorStream().AsRawOstream(), llvm::HighlightColor::Error,
+  m_colors)
+  << "error: " << in_string << '\n';
 }
 
 void CommandReturnObject::SetError(const Status &error,
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -209,7 +209,7 @@
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
 
-  CommandReturnObject result;
+  CommandReturnObject result(m_debugger.GetUseColor());
 
   LoadCommandDictionary();
 
@@ -2792,7 +2792,7 @@
 
   StartHandlingCommand();
 
-  lldb_private::CommandReturnObject result;
+  lldb_private::CommandReturnObject result(m_debugger.GetUseColor());
   HandleCommand(line.c_str(), eLazyBoolCalculate, result);
 
   // Now emit the command output text from the command we just executed
Index: lldb/include/lldb/Utility/Stream.h
===
--- lldb/include/lldb/Utility/Stream.h
+++ lldb/include/lldb/Utility/Stream.h
@@ -404,7 +404,9 @@
 
   public:
 RawOstreamForward(Stream &target)
-: llvm::raw_ostream(/*unbuffered*/ true), m_target(target) {}
+: llvm::raw_ostream(/*unbuffered*/ true), m_target(target) {
+  enable_colors(true);
+}
   };
   RawOstreamForward m_forwarder;
 };
Index: lldb/include/lldb/Interpreter/CommandReturnObject.h
===
--- lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -16,6 +16,7 @@
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/WithColor.h"
 
 #include 
 
@@ -23,7 +24,7 @@
 
 class CommandReturnObject {
 public:
-  CommandReturnObject();
+  CommandReturnObject(bool colors = false);
 
   ~CommandReturnObject();
 
@@ -150,6 +151,7 @@
   StreamTee m_err_stream;
 
   lldb::ReturnStatus m_status;
+  llvm::ColorMode m_colors;
   bool m_did_change_process_state;
   bool m_interactive; // If true, then the input handle from the debugger will
   // be hooked up
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D81010: [lldb/DWARF] Fix PC value for artificial tail call frames for the "GNU" case

2020-06-03 Thread Vedant Kumar via Phabricator via lldb-commits 
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Lgtm.




Comment at: lldb/source/Symbol/Function.cpp:321
 llvm::ArrayRef> Function::GetTailCallingEdges() {
   // Call edges are sorted by return PC, and tail calling edges have invalid
   // return PCs. Find them at the end of the list.

Could you please update this comment?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81010/new/

https://reviews.llvm.org/D81010



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

[Lldb-commits] [lldb] 4699a7e - [lldb/StringPrinter] Support strings with invalid utf8 sub-sequences

2020-06-03 Thread Vedant Kumar via lldb-commits 

Author: Vedant Kumar
Date: 2020-06-03T12:24:23-07:00
New Revision: 4699a7e23010b7c0df49b64f8bea63919825a787

URL: 
https://github.com/llvm/llvm-project/commit/4699a7e23010b7c0df49b64f8bea63919825a787
DIFF: 
https://github.com/llvm/llvm-project/commit/4699a7e23010b7c0df49b64f8bea63919825a787.diff

LOG: [lldb/StringPrinter] Support strings with invalid utf8 sub-sequences

Support printing strings which contain invalid utf8 sub-sequences, e.g.
strings like "hello world \xfe", instead of bailing out with "Summary
Unavailable".

I took the opportunity here to delete some hand-rolled utf8 -> utf32
conversion code and replace it with calls into llvm's Support library.

rdar://61554346

Added: 


Modified: 
lldb/source/DataFormatters/StringPrinter.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
lldb/unittests/DataFormatter/StringPrinterTests.cpp

Removed: 




diff  --git a/lldb/source/DataFormatters/StringPrinter.cpp 
b/lldb/source/DataFormatters/StringPrinter.cpp
index 7f7d6c1639f0..139f1ec0554f 100644
--- a/lldb/source/DataFormatters/StringPrinter.cpp
+++ b/lldb/source/DataFormatters/StringPrinter.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Status.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ConvertUTF.h"
 
 #include 
@@ -92,7 +93,7 @@ static bool isprint32(char32_t codepoint) {
   return true;
 }
 
-DecodedCharBuffer attemptASCIIEscape(char32_t c,
+DecodedCharBuffer attemptASCIIEscape(llvm::UTF32 c,
  StringPrinter::EscapeStyle escape_style) {
   const bool is_swift_escape_style =
   escape_style == StringPrinter::EscapeStyle::Swift;
@@ -141,7 +142,10 @@ DecodedCharBuffer 
GetPrintableImpl(
   DecodedCharBuffer retval = attemptASCIIEscape(*buffer, escape_style);
   if (retval.GetSize())
 return retval;
-  if (isprint(*buffer))
+
+  // Use llvm's locale-independent isPrint(char), instead of the libc
+  // implementation which may give 
diff erent results on 
diff erent platforms.
+  if (llvm::isPrint(*buffer))
 return {buffer, 1};
 
   unsigned escaped_len;
@@ -161,60 +165,30 @@ DecodedCharBuffer 
GetPrintableImpl(
   return {data, escaped_len};
 }
 
-static char32_t ConvertUTF8ToCodePoint(unsigned char c0, unsigned char c1) {
-  return (c0 - 192) * 64 + (c1 - 128);
-}
-static char32_t ConvertUTF8ToCodePoint(unsigned char c0, unsigned char c1,
-   unsigned char c2) {
-  return (c0 - 224) * 4096 + (c1 - 128) * 64 + (c2 - 128);
-}
-static char32_t ConvertUTF8ToCodePoint(unsigned char c0, unsigned char c1,
-   unsigned char c2, unsigned char c3) {
-  return (c0 - 240) * 262144 + (c2 - 128) * 4096 + (c2 - 128) * 64 + (c3 - 
128);
-}
-
 template <>
 DecodedCharBuffer GetPrintableImpl(
 uint8_t *buffer, uint8_t *buffer_end, uint8_t *&next,
 StringPrinter::EscapeStyle escape_style) {
-  const unsigned utf8_encoded_len = llvm::getNumBytesForUTF8(*buffer);
-
-  // If the utf8 encoded length is invalid, or if there aren't enough bytes to
-  // print, this is some kind of corrupted string.
-  if (utf8_encoded_len == 0 || utf8_encoded_len > 4)
-return nullptr;
-  if ((buffer_end - buffer) < utf8_encoded_len)
-// There's no room in the buffer for the utf8 sequence.
-return nullptr;
-
-  char32_t codepoint = 0;
-  switch (utf8_encoded_len) {
-  case 1:
-// this is just an ASCII byte - ask ASCII
+  // If the utf8 encoded length is invalid (i.e., not in the closed interval
+  // [1;4]), or if there aren't enough bytes to print, or if the subsequence
+  // isn't valid utf8, fall back to printing an ASCII-escaped subsequence.
+  if (!llvm::isLegalUTF8Sequence(buffer, buffer_end))
 return GetPrintableImpl(buffer, buffer_end, next,
   escape_style);
-  case 2:
-codepoint = ConvertUTF8ToCodePoint((unsigned char)*buffer,
-   (unsigned char)*(buffer + 1));
-break;
-  case 3:
-codepoint = ConvertUTF8ToCodePoint((unsigned char)*buffer,
-   (unsigned char)*(buffer + 1),
-   (unsigned char)*(buffer + 2));
-break;
-  case 4:
-codepoint = ConvertUTF8ToCodePoint(
-(unsigned char)*buffer, (unsigned char)*(buffer + 1),
-(unsigned char)*(buffer + 2), (unsigned char)*(buffer + 3));
-break;
-  }
 
-  // We couldn't figure out how to print this codepoint.
-  if (!codepoint)
-return nullptr;
+  // Convert the valid utf8 sequence to a utf32 codepoint. This cannot fail.
+  llvm::UTF32 codepoint = 0;
+  const llvm::UTF8 *buffer_for_conversion = buffer;
+  llvm::ConversionResult result = llvm::co

[Lldb-commits] [lldb] 7822b8a - [lldb/StringPrinter] Convert DecodedCharBuffer to a class, NFC

2020-06-03 Thread Vedant Kumar via lldb-commits 

Author: Vedant Kumar
Date: 2020-06-03T12:24:23-07:00
New Revision: 7822b8a817d85827110f3047f4ec63f6825743a4

URL: 
https://github.com/llvm/llvm-project/commit/7822b8a817d85827110f3047f4ec63f6825743a4
DIFF: 
https://github.com/llvm/llvm-project/commit/7822b8a817d85827110f3047f4ec63f6825743a4.diff

LOG: [lldb/StringPrinter] Convert DecodedCharBuffer to a class, NFC

The m_size and m_data members of DecodedCharBuffer are meant to be
private.

Added: 


Modified: 
lldb/source/DataFormatters/StringPrinter.cpp

Removed: 




diff  --git a/lldb/source/DataFormatters/StringPrinter.cpp 
b/lldb/source/DataFormatters/StringPrinter.cpp
index 53dbc8d76a99..7f7d6c1639f0 100644
--- a/lldb/source/DataFormatters/StringPrinter.cpp
+++ b/lldb/source/DataFormatters/StringPrinter.cpp
@@ -30,9 +30,7 @@ using StringElementType = StringPrinter::StringElementType;
 /// DecodedCharBuffer stores the decoded contents of a single character. It
 /// avoids managing memory on the heap by copying decoded bytes into an in-line
 /// buffer.
-struct DecodedCharBuffer {
-  static constexpr unsigned MaxLength = 16;
-
+class DecodedCharBuffer {
 public:
   DecodedCharBuffer(std::nullptr_t) {}
 
@@ -50,6 +48,8 @@ struct DecodedCharBuffer {
   size_t GetSize() const { return m_size; }
 
 private:
+  static constexpr unsigned MaxLength = 16;
+
   size_t m_size = 0;
   uint8_t m_data[MaxLength] = {0};
 };



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

[Lldb-commits] [lldb] a23d0a0 - [lldb][NFC] Fix documentation formatting in ASTResultSynthesizer

2020-06-03 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-06-03T21:34:23+02:00
New Revision: a23d0a06d4b7996a86c3d1a7e47a4e56d6181b10

URL: 
https://github.com/llvm/llvm-project/commit/a23d0a06d4b7996a86c3d1a7e47a4e56d6181b10
DIFF: 
https://github.com/llvm/llvm-project/commit/a23d0a06d4b7996a86c3d1a7e47a4e56d6181b10.diff

LOG: [lldb][NFC] Fix documentation formatting in ASTResultSynthesizer

This comment apparently didn't survive the great LLDB reformatting unharmed.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
index bd705af0efcd..39ba5f4e9e4f 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
@@ -248,48 +248,37 @@ bool 
ASTResultSynthesizer::SynthesizeBodyResult(CompoundStmt *Body,
   // For Lvalues
   //
   //   - In AST result synthesis (here!) the expression E is transformed into 
an
-  //   initialization
-  // T *$__lldb_expr_result_ptr = &E.
+  // initialization T *$__lldb_expr_result_ptr = &E.
   //
   //   - In structure allocation, a pointer-sized slot is allocated in the
-  //   struct that is to be
-  // passed into the expression.
+  // struct that is to be passed into the expression.
   //
   //   - In IR transformations, reads and writes to $__lldb_expr_result_ptr are
-  //   redirected at
-  // an entry in the struct ($__lldb_arg) passed into the expression.
-  // (Other persistent
-  // variables are treated similarly, having been materialized as
-  // references, but in those
-  // cases the value of the reference itself is never modified.)
+  // redirected at an entry in the struct ($__lldb_arg) passed into the
+  // expression. (Other persistent variables are treated similarly, having
+  // been materialized as references, but in those cases the value of the
+  // reference itself is never modified.)
   //
   //   - During materialization, $0 (the result persistent variable) is 
ignored.
   //
   //   - During dematerialization, $0 is marked up as a load address with value
-  //   equal to the
-  // contents of the structure entry.
+  // equal to the contents of the structure entry.
   //
   // For Rvalues
   //
   //   - In AST result synthesis the expression E is transformed into an
-  //   initialization
-  // static T $__lldb_expr_result = E.
+  // initialization static T $__lldb_expr_result = E.
   //
   //   - In structure allocation, a pointer-sized slot is allocated in the
-  //   struct that is to be
-  // passed into the expression.
+  // struct that is to be passed into the expression.
   //
   //   - In IR transformations, an instruction is inserted at the beginning of
-  //   the function to
-  // dereference the pointer resident in the slot.  Reads and writes to
-  // $__lldb_expr_result
-  // are redirected at that dereferenced version.  Guard variables for the
-  // static variable
-  // are excised.
+  // the function to dereference the pointer resident in the slot. Reads 
and
+  // writes to $__lldb_expr_result are redirected at that dereferenced
+  // version. Guard variables for the static variable are excised.
   //
   //   - During materialization, $0 (the result persistent variable) is
-  //   populated with the location
-  // of a newly-allocated area of memory.
+  // populated with the location of a newly-allocated area of memory.
   //
   //   - During dematerialization, $0 is ignored.
 



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

[Lldb-commits] [lldb] 7c1b060 - [IRInterpreter] Unused. Drive-by cleanup. NFCI.

2020-06-03 Thread Davide Italiano via lldb-commits 

Author: Davide Italiano
Date: 2020-06-03T13:31:58-07:00
New Revision: 7c1b060c3c65cac50edec5cceb6653957c344d96

URL: 
https://github.com/llvm/llvm-project/commit/7c1b060c3c65cac50edec5cceb6653957c344d96
DIFF: 
https://github.com/llvm/llvm-project/commit/7c1b060c3c65cac50edec5cceb6653957c344d96.diff

LOG: [IRInterpreter] Unused. Drive-by cleanup. NFCI.

Added: 


Modified: 
lldb/source/Expression/IRInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Expression/IRInterpreter.cpp 
b/lldb/source/Expression/IRInterpreter.cpp
index 6f61a4bd26bf..0af767116b74 100644
--- a/lldb/source/Expression/IRInterpreter.cpp
+++ b/lldb/source/Expression/IRInterpreter.cpp
@@ -433,8 +433,6 @@ static const char *unsupported_opcode_error =
 "Interpreter doesn't handle one of the expression's opcodes";
 static const char *unsupported_operand_error =
 "Interpreter doesn't handle one of the expression's operands";
-// static const char *interpreter_initialization_error = "Interpreter couldn't
-// be initialized";
 static const char *interpreter_internal_error =
 "Interpreter encountered an internal error";
 static const char *bad_value_error =
@@ -444,8 +442,6 @@ static const char *memory_allocation_error =
 static const char *memory_write_error = "Interpreter couldn't write to memory";
 static const char *memory_read_error = "Interpreter couldn't read from memory";
 static const char *infinite_loop_error = "Interpreter ran for too many cycles";
-// static const char *bad_result_error = "Result of expression
-// is in bad memory";
 static const char *too_many_functions_error =
 "Interpreter doesn't handle modules with multiple function bodies.";
 



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

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

2020-06-03 Thread Jan Kratochvil via Phabricator via lldb-commits 
jankratochvil created this revision.
jankratochvil added reviewers: dblaikie, davide.
jankratochvil added a project: LLDB.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.
jankratochvil retitled this revision from "Fix SLEB128 decoding" to "[lldb] Fix 
SLEB128 decoding".
davide added reviewers: clayborg, labath.
davide added a comment.

Greg & Pavel might have opinions on this patch. I'm not qualified to review it.


Bug 46181  shows SLEB128 
`0xED9A924C00011151` decoded as `0x80011151`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81119

Files:
  lldb/source/Utility/DataExtractor.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s

Index: lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/dwarf-sleb128.s
@@ -0,0 +1,181 @@
+# This tests that lldb is able to process 64-bit wide negative SLEB128.
+
+# REQUIRES: lld, x86
+
+# RUN: llvm-mc -g -dwarf-version=5 -triple x86_64-unknown-linux-gnu %s -filetype=obj > %t.o
+# RUN: ld.lld -m elf_x86_64 %t.o -o %t 
+# RUN: %lldb %t -o "p/x magic64" -o exit | FileCheck %s
+
+# CHECK: (const long) $0 = 0xed9a924c00011151
+
+# It is built from this source with a few #-marked patched lines:
+# static const long magic64 = 0xED9A924C00011151;
+# int main(void) { return magic64; }
+
+	.text
+	.file	"lldb46181c.c"
+	.file	1 "" "lldb46181c.c"
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	1 2 0   # lldb46181c.c:2:0
+	.cfi_startproc
+# %bb.0:
+	.loc	1 2 18 prologue_end # lldb46181c.c:2:18
+	movl	$69969, %eax# imm = 0x11151
+	retq
+.Ltmp0:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+# -- End function
+	.section	.debug_str,"MS",@progbits,1
+.Linfo_string0:
+	.asciz	"clang version 10.0.0 (Fedora 10.0.0-1.fc32)" # string offset=0
+.Linfo_string1:
+	.asciz	"lldb46181c.c"  # string offset=44
+.Linfo_string2:
+	.asciz	""  # string offset=57
+.Linfo_string3:
+	.asciz	"magic64"   # string offset=74
+.Linfo_string4:
+	.asciz	"long int"  # string offset=82
+.Linfo_string5:
+	.asciz	"main"  # string offset=91
+.Linfo_string6:
+	.asciz	"int"   # string offset=96
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	17  # DW_TAG_compile_unit
+	.byte	1   # DW_CHILDREN_yes
+	.byte	37  # DW_AT_producer
+	.byte	14  # DW_FORM_strp
+	.byte	19  # DW_AT_language
+	.byte	5   # DW_FORM_data2
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	16  # DW_AT_stmt_list
+	.byte	23  # DW_FORM_sec_offset
+	.byte	27  # DW_AT_comp_dir
+	.byte	14  # DW_FORM_strp
+	.byte	17  # DW_AT_low_pc
+	.byte	1   # DW_FORM_addr
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	2   # Abbreviation Code
+	.byte	52  # DW_TAG_variable
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	58  # DW_AT_decl_file
+	.byte	11  # DW_FORM_data1
+	.byte	59  # DW_AT_decl_line
+#	.byte	11  # DW_FORM_data1
+	.byte	5   # DW_FORM_data2
+	.byte	28  # DW_AT_const_value
+#	.byte	15  # DW_FORM_udata
+	.byte	13  # DW_FORM_sdata
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	3   # Abbreviation Code
+	.byte	38  # DW_TAG_const_type
+	.byte	0   # DW_CHILDREN_no
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	4   # Abbreviation Code
+	.byte	36  # DW_TAG_base_type
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	62  # DW_AT_encoding
+	.byte	11  # DW_FORM_data1
+	.byte	11  # DW_AT_byte_size
+	.byte

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

2020-06-03 Thread Davide Italiano via Phabricator via lldb-commits 
davide added a comment.

Greg & Pavel might have opinions on this patch. I'm not qualified to review it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81119/new/

https://reviews.llvm.org/D81119



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

[Lldb-commits] [lldb] f4d4273 - Add a test for preserving state on the non-expr thread across expression evaluation.

2020-06-03 Thread Jim Ingham via lldb-commits 

Author: Jim Ingham
Date: 2020-06-03T14:30:35-07:00
New Revision: f4d427326539f007b05378eaf66018c10b651ad0

URL: 
https://github.com/llvm/llvm-project/commit/f4d427326539f007b05378eaf66018c10b651ad0
DIFF: 
https://github.com/llvm/llvm-project/commit/f4d427326539f007b05378eaf66018c10b651ad0.diff

LOG: Add a test for preserving state on the non-expr thread across expression 
evaluation.

There may be another test that tests this but I couldn't find one.

Added: 
lldb/test/API/functionalities/thread/state_after_expression/Makefile

lldb/test/API/functionalities/thread/state_after_expression/TestStateAfterExpression.py
lldb/test/API/functionalities/thread/state_after_expression/main.cpp

Modified: 


Removed: 




diff  --git 
a/lldb/test/API/functionalities/thread/state_after_expression/Makefile 
b/lldb/test/API/functionalities/thread/state_after_expression/Makefile
new file mode 100644
index ..d7aace51bc82
--- /dev/null
+++ b/lldb/test/API/functionalities/thread/state_after_expression/Makefile
@@ -0,0 +1,6 @@
+CXX_SOURCES := main.cpp
+CFLAGS_EXTRAS := -std=c++11
+
+ENABLE_THREADS := YES
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/functionalities/thread/state_after_expression/TestStateAfterExpression.py
 
b/lldb/test/API/functionalities/thread/state_after_expression/TestStateAfterExpression.py
new file mode 100644
index ..082b556dbdce
--- /dev/null
+++ 
b/lldb/test/API/functionalities/thread/state_after_expression/TestStateAfterExpression.py
@@ -0,0 +1,53 @@
+"""
+Make sure the stop reason of a thread that did not run
+during an expression is not changed by running the expression
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestStopReasonAfterExpression(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_thread_state_after_expr(self):
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.cpp")
+self.do_test()
+
+def do_test(self):
+(target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
+"Set a breakpoint here", self.main_source_file)
+
+self.assertEqual(bkpt.GetNumLocations(), 2, "Got two locations")
+
+# So now thread holds the main thread.  Continue to hit the
+# breakpoint again on the spawned thread:
+
+threads = lldbutil.continue_to_breakpoint(process, bkpt)
+self.assertEqual(len(threads), 1, "Hit the breakpoint the second time")
+other_thread = threads[0]
+
+self.assertNotEqual(thread.GetThreadID(), other_thread.GetThreadID(),
+"A 
diff erent thread")
+# Run an expression ONLY on other_thread.  Don't let thread run:
+options = lldb.SBExpressionOptions()
+options.SetTryAllThreads(False)
+options.SetStopOthers(True)
+
+result = thread.frames[0].EvaluateExpression('(int) 
printf("Hello\\n")', options)
+self.assertTrue(result.GetError().Success(),
+"Expression failed: 
'%s'"%(result.GetError().GetCString()))
+
+stop_reason = other_thread.GetStopReason()
+
+self.assertEqual(stop_reason, lldb.eStopReasonBreakpoint,
+ "Still records stopped at breakpoint: %s"
+ %(lldbutil.stop_reason_to_str(stop_reason)))
+self.assertEqual(other_thread.GetStopReasonDataAtIndex(0), 1,
+ "Still records stopped at right breakpoint")
+

diff  --git 
a/lldb/test/API/functionalities/thread/state_after_expression/main.cpp 
b/lldb/test/API/functionalities/thread/state_after_expression/main.cpp
new file mode 100644
index ..338232ece632
--- /dev/null
+++ b/lldb/test/API/functionalities/thread/state_after_expression/main.cpp
@@ -0,0 +1,14 @@
+#include 
+
+void thread_func() {
+  // Set a breakpoint here
+}
+
+int
+main()
+{
+  // Set a breakpoint here
+  std::thread stopped_thread(thread_func);
+  stopped_thread.join();
+  return 0;
+}



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

[Lldb-commits] [lldb] 9caa34a - [lldb/Test] Remove un(used|needed|maintained) files from lldbsuite.

2020-06-03 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2020-06-03T14:54:39-07:00
New Revision: 9caa34a24cb7d20a129143281cc0e1b2f44bd95c

URL: 
https://github.com/llvm/llvm-project/commit/9caa34a24cb7d20a129143281cc0e1b2f44bd95c
DIFF: 
https://github.com/llvm/llvm-project/commit/9caa34a24cb7d20a129143281cc0e1b2f44bd95c.diff

LOG: [lldb/Test] Remove un(used|needed|maintained) files from lldbsuite.

Added: 


Modified: 


Removed: 
lldb/packages/Python/lldbsuite/.clang-format
lldb/packages/Python/lldbsuite/test/lock.py
lldb/packages/Python/lldbsuite/test/redo.py



diff  --git a/lldb/packages/Python/lldbsuite/.clang-format 
b/lldb/packages/Python/lldbsuite/.clang-format
deleted file mode 100644
index 7de7a512ad7e..
--- a/lldb/packages/Python/lldbsuite/.clang-format
+++ /dev/null
@@ -1,4 +0,0 @@
-DisableFormat: true
-
-# Disabling formatting doesn't implicitly disable include sorting
-SortIncludes: false

diff  --git a/lldb/packages/Python/lldbsuite/test/lock.py 
b/lldb/packages/Python/lldbsuite/test/lock.py
deleted file mode 100644
index e9970e135f93..
--- a/lldb/packages/Python/lldbsuite/test/lock.py
+++ /dev/null
@@ -1,27 +0,0 @@
-"""
-Interprocess mutex based on file locks
-"""
-
-import fcntl
-
-
-class Lock:
-
-def __init__(self, filename):
-self.filename = filename
-# This will create it if it does not exist already
-unbuffered = 0
-self.handle = open(filename, 'a+', unbuffered)
-
-def acquire(self):
-fcntl.flock(self.handle, fcntl.LOCK_EX)
-
-# will throw IOError if unavailable
-def try_acquire(self):
-fcntl.flock(self.handle, fcntl.LOCK_NB | fcntl.LOCK_EX)
-
-def release(self):
-fcntl.flock(self.handle, fcntl.LOCK_UN)
-
-def __del__(self):
-self.handle.close()

diff  --git a/lldb/packages/Python/lldbsuite/test/redo.py 
b/lldb/packages/Python/lldbsuite/test/redo.py
deleted file mode 100644
index 03052c3a08ca..
--- a/lldb/packages/Python/lldbsuite/test/redo.py
+++ /dev/null
@@ -1,203 +0,0 @@
-#!/usr/bin/env python
-
-"""
-A simple utility to redo the failed/errored tests.
-
-You need to specify the session directory in order for this script to locate 
the
-tests which need to be re-run.
-
-See also dotest.py, the test driver running the test suite.
-
-Type:
-
-./dotest.py -h
-
-for help.
-"""
-
-from __future__ import print_function
-
-import os
-import sys
-import datetime
-import re
-
-# If True, redo with no '-t' option for the test driver.
-no_trace = False
-
-# To be filled with the filterspecs found in the session logs.
-redo_specs = []
-
-# The filename components to match for.  Only files with the contained 
component names
-# will be considered for re-run.  Examples: ['X86_64', 'clang'].
-filename_components = []
-
-do_delay = False
-
-# There is a known bug with respect to comp_specs and arch_specs, in that if we
-# encountered "-C clang" and "-C gcc" when visiting the session files, both
-# compilers will end up in the invocation of the test driver when rerunning.
-# That is: ./dotest -v -C clang^gcc ... -f ...".  Ditto for "-A" flags.
-
-# The "-C compiler" for comp_specs.
-comp_specs = set()
-# The "-A arch" for arch_specs.
-arch_specs = set()
-
-
-def usage():
-print("""\
-Usage: redo.py [-F filename_component] [-n] [session_dir] [-d]
-where options:
--F : only consider the test for re-run if the session filename contains the 
filename component
- for example: -F x86_64
--n : when running the tests, do not turn on trace mode, i.e, no '-t' option
- is passed to the test driver (this will run the tests faster)
--d : pass -d down to the test driver (introduces a delay so you can attach 
with a debugger)
-
-and session_dir specifies the session directory which contains previously
-recorded session infos for all the test cases which either failed or errored.
-
-If sessin_dir is left unspecified, this script uses the heuristic to find the
-possible session directories with names starting with %Y-%m-%d- (for example,
-2012-01-23-) and employs the one with the latest timestamp.""")
-sys.exit(0)
-
-
-def where(session_dir, test_dir):
-"""Returns the full path to the session directory; None if non-existent."""
-abspath = os.path.abspath(session_dir)
-if os.path.isdir(abspath):
-return abspath
-
-session_dir_path = os.path.join(test_dir, session_dir)
-if os.path.isdir(session_dir_path):
-return session_dir_path
-
-return None
-
-# This is the pattern for the line from the log file to redo a test.
-# We want the filter spec.
-filter_pattern = re.compile("^\./dotest\.py.*-f (.*)$")
-comp_pattern = re.compile(" -C ([^ ]+) ")
-arch_pattern = re.compile(" -A ([^ ]+) ")
-
-
-def redo(suffix, dir, names):
-"""Visitor function for os.path.walk(path, visit, arg)."""
-global redo_specs
-global comp_specs
-global arch_specs
-global filter

[Lldb-commits] [lldb] 5fa9c9d - [lldb/Test] Python <3.5 requires **kwargs to come last

2020-06-03 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2020-06-03T14:54:39-07:00
New Revision: 5fa9c9d7f276b44b3da949382e0d0b5dbfd0ac8b

URL: 
https://github.com/llvm/llvm-project/commit/5fa9c9d7f276b44b3da949382e0d0b5dbfd0ac8b
DIFF: 
https://github.com/llvm/llvm-project/commit/5fa9c9d7f276b44b3da949382e0d0b5dbfd0ac8b.diff

LOG: [lldb/Test] Python <3.5 requires **kwargs to come last

Thanks Martin Böhme for pointing this out.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 0a640d2d5c93..cad7a127e752 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -507,7 +507,7 @@ def TraceOn(self):
 
 def trace(self, *args,**kwargs):
 with recording(self, self.TraceOn()) as sbuf:
-print(*args, **kwargs, file=sbuf)
+print(*args, file=sbuf, **kwargs)
 
 @classmethod
 def setUpClass(cls):



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

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-03 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

The one scenario I can think of where this might do the wrong thing is:

1. Hit breakpoint location 1.1 on thread A
2. Switch to thread B
3. Run a function on thread B, has to only run on thread B and has to actually 
run code in the target, like:

  (lldb) expr -a 0 -- (int) printf("Hello\n")

The stop reason for thread A should still stay "breakpoint 1.1" and not "no 
reason".

This one is a little tricky because we have to run the target to execute the 
function, but if running the expression didn't cause other threads to run, we 
don't want to lose the program state when we stopped.

I looked for a test that ensures this result, and didn't find one, so I just 
added one: TestStateAfterExpression.py.  Can you make sure that your change 
still passes this test.  If it does, I can't think of anything else that might 
go wrong with this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80112/new/

https://reviews.llvm.org/D80112



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

[Lldb-commits] [PATCH] D80448: [lldb/Test] Add a trace method to replace (commented out) print statements.

2020-06-03 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:510
+with recording(self, self.TraceOn()) as sbuf:
+print(args, kwargs, file=sbuf)
+

mboehme wrote:
> labath wrote:
> > JDevlieghere wrote:
> > > This should be 
> > > ```
> > > print(*args, kwargs, file=sbuf)
> > > ```
> > Actually, it should be `print(*args, **kwargs, file=sbuf)`
> With both Python 2 and 3, this, is giving me an error:
> 
> ```
> print(*args, **kwargs, file=sbuf)
>  ^
> SyntaxError: invalid syntax
> ```
> 
> I believe this should be `print(*args, file=sbuf, **kwargs)`. Can you fix 
> this?
Thanks, fixed by `5fa9c9d7f276b44b3da949382e0d0b5dbfd0ac8b`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80448/new/

https://reviews.llvm.org/D80448



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

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

2020-06-03 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

Seems like it would be easier to add a unit test to 
lldb/unittests/Utility/DataExtractorTest.cpp. Then you can verify any edge case 
without having to make DWARF in a .s file?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81119/new/

https://reviews.llvm.org/D81119



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

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

2020-06-03 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik added a comment.

I would also like to see a unit test, unless the DWARF test somehow covers as 
aspect that a unit test would and even in that case the unit test is still 
important.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81119/new/

https://reviews.llvm.org/D81119



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

[Lldb-commits] [PATCH] D80448: [lldb/Test] Add a trace method to replace (commented out) print statements.

2020-06-03 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added a comment.

nice!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80448/new/

https://reviews.llvm.org/D80448



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

[Lldb-commits] [PATCH] D80448: [lldb/Test] Add a trace method to replace (commented out) print statements.

2020-06-03 Thread Martin Böhme via Phabricator via lldb-commits 
mboehme added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80448/new/

https://reviews.llvm.org/D80448



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