[Lldb-commits] [lldb] 60cbbb3 - [lldb][NFC] Remove misleading class_language variable in DWARFASTParserClang

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-07-13T13:10:12+02:00
New Revision: 60cbbb306d29f882e18d6293177d694c11c67e84

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

LOG: [lldb][NFC] Remove misleading class_language variable in 
DWARFASTParserClang

There is a local 'class_language' veriable in DWARFASTParserClang which is named
as if it is related to the 'class_language' member of ParsedDWARFTypeAttributes.
However, it actually only has two possible enum values: 'ObjC' (which means the
current record is a Objective-C class) or 'Unknown' (which covers all other
cases).

This is confusing for the reader and also lead to some strange code where we
have several comparisons against the value "ObjC_plus_plus" (which is always
false).

This replaces the variable with either a const bool variable (if there are
multiple checks for that condition in a function) or a direct call to the
TypeSystemClang utility method for checking if it's a Objective-C
Object/Interface type.

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 7de88274ccf6..929001671af7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1958,9 +1958,9 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
   ClangASTImporter::LayoutInfo layout_info;
 
   if (die.HasChildren()) {
-LanguageType class_language = eLanguageTypeUnknown;
-if (TypeSystemClang::IsObjCObjectOrInterfaceType(clang_type)) {
-  class_language = eLanguageTypeObjC;
+const bool type_is_objc_object_or_interface =
+TypeSystemClang::IsObjCObjectOrInterfaceType(clang_type);
+if (type_is_objc_object_or_interface) {
   // For objective C we don't start the definition when the class is
   // created.
   TypeSystemClang::StartTagDeclarationDefinition(clang_type);
@@ -1986,16 +1986,15 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
 std::vector member_function_dies;
 
 DelayedPropertyList delayed_properties;
-ParseChildMembers(die, clang_type, class_language, bases,
-  member_accessibilities, member_function_dies,
-  delayed_properties, default_accessibility, is_a_class,
-  layout_info);
+ParseChildMembers(die, clang_type, bases, member_accessibilities,
+  member_function_dies, delayed_properties,
+  default_accessibility, is_a_class, layout_info);
 
 // Now parse any methods if there were any...
 for (const DWARFDIE &die : member_function_dies)
   dwarf->ResolveType(die);
 
-if (class_language == eLanguageTypeObjC) {
+if (type_is_objc_object_or_interface) {
   ConstString class_name(clang_type.GetTypeName());
   if (class_name) {
 dwarf->GetObjCMethods(class_name, [&](DWARFDIE method_die) {
@@ -2012,7 +2011,7 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
 
 // If we have a DW_TAG_structure_type instead of a DW_TAG_class_type we
 // need to tell the clang type it is actually a class.
-if (class_language != eLanguageTypeObjC) {
+if (!type_is_objc_object_or_interface) {
   if (is_a_class && tag_decl_kind != clang::TTK_Class)
 m_ast.SetTagTypeKind(ClangUtil::GetQualType(clang_type),
  clang::TTK_Class);
@@ -2346,7 +2345,6 @@ Function 
*DWARFASTParserClang::ParseFunctionFromDWARF(CompileUnit &comp_unit,
 void DWARFASTParserClang::ParseSingleMember(
 const DWARFDIE &die, const DWARFDIE &parent_die,
 const lldb_private::CompilerType &class_clang_type,
-const lldb::LanguageType class_language,
 std::vector &member_accessibilities,
 lldb::AccessType default_accessibility,
 DelayedPropertyList &delayed_properties,
@@ -2520,9 +2518,11 @@ void DWARFASTParserClang::ParseSingleMember(
   bit_offset = 0;
 }
 
+const bool class_is_objc_object_or_interface =
+TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type);
+
 // FIXME: Make Clang ignore Objective-C accessibility for expressions
-if (class_language == eLanguageTypeObjC ||
-class_language == eLanguageTypeObjC_plus_plus)
+if (class_is_objc_object_or_interface)
   accessibility = eAccessNone;
 
 // Handle static members
@@ -2599,8 +2599,7 @@ void DWARFASTParserClang::ParseSingleMember(
 // unnamed bitfields if we have a new enough clang.
 bool detect_unnamed_bitfields = true;
 
-   

[Lldb-commits] [lldb] aa933d8 - [lldb][NFC] Early-exit in DWARFASTParserClang::ParseSingleMember

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-07-13T13:21:12+02:00
New Revision: aa933d82f867ab4d33eafc5ee2666dbbc61d293d

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

LOG: [lldb][NFC] Early-exit in DWARFASTParserClang::ParseSingleMember

This patch just early-exits after the 'if (num_attributes > 0)' check.

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 929001671af7..2d1db66e7fd9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2360,393 +2360,394 @@ void DWARFASTParserClang::ParseSingleMember(
 
   DWARFAttributes attributes;
   const size_t num_attributes = die.GetAttributes(attributes);
-  if (num_attributes > 0) {
-const char *name = nullptr;
-const char *prop_name = nullptr;
-const char *prop_getter_name = nullptr;
-const char *prop_setter_name = nullptr;
-uint32_t prop_attributes = 0;
-
-bool is_artificial = false;
-DWARFFormValue encoding_form;
-AccessType accessibility = eAccessNone;
-uint32_t member_byte_offset =
-(parent_die.Tag() == DW_TAG_union_type) ? 0 : UINT32_MAX;
-llvm::Optional byte_size;
-int64_t bit_offset = 0;
-uint64_t data_bit_offset = UINT64_MAX;
-size_t bit_size = 0;
-bool is_external =
-false; // On DW_TAG_members, this means the member is static
-uint32_t i;
-for (i = 0; i < num_attributes && !is_artificial; ++i) {
-  const dw_attr_t attr = attributes.AttributeAtIndex(i);
-  DWARFFormValue form_value;
-  if (attributes.ExtractFormValueAtIndex(i, form_value)) {
-// DW_AT_data_member_location indicates the byte offset of the
-// word from the base address of the structure.
-//
-// DW_AT_bit_offset indicates how many bits into the word
-// (according to the host endianness) the low-order bit of the
-// field starts.  AT_bit_offset can be negative.
-//
-// DW_AT_bit_size indicates the size of the field in bits.
-switch (attr) {
-case DW_AT_name:
-  name = form_value.AsCString();
-  break;
-case DW_AT_type:
-  encoding_form = form_value;
-  break;
-case DW_AT_bit_offset:
-  bit_offset = form_value.Signed();
-  break;
-case DW_AT_bit_size:
-  bit_size = form_value.Unsigned();
-  break;
-case DW_AT_byte_size:
-  byte_size = form_value.Unsigned();
-  break;
-case DW_AT_data_bit_offset:
-  data_bit_offset = form_value.Unsigned();
-  break;
-case DW_AT_data_member_location:
-  if (form_value.BlockData()) {
-Value initialValue(0);
-Value memberOffset(0);
-const DWARFDataExtractor &debug_info_data = die.GetData();
-uint32_t block_length = form_value.Unsigned();
-uint32_t block_offset =
-form_value.BlockData() - debug_info_data.GetDataStart();
-if (DWARFExpression::Evaluate(
-nullptr, // ExecutionContext *
-nullptr, // RegisterContext *
-module_sp,
-DataExtractor(debug_info_data, block_offset, block_length),
-die.GetCU(), eRegisterKindDWARF, &initialValue, nullptr,
-memberOffset, nullptr)) {
-  member_byte_offset = memberOffset.ResolveValue(nullptr).UInt();
-}
-  } else {
-// With DWARF 3 and later, if the value is an integer constant,
-// this form value is the offset in bytes from the beginning of
-// the containing entity.
-member_byte_offset = form_value.Unsigned();
+  if (num_attributes == 0)
+return;
+
+  const char *name = nullptr;
+  const char *prop_name = nullptr;
+  const char *prop_getter_name = nullptr;
+  const char *prop_setter_name = nullptr;
+  uint32_t prop_attributes = 0;
+
+  bool is_artificial = false;
+  DWARFFormValue encoding_form;
+  AccessType accessibility = eAccessNone;
+  uint32_t member_byte_offset =
+  (parent_die.Tag() == DW_TAG_union_type) ? 0 : UINT32_MAX;
+  llvm::Optional byte_size;
+  int64_t bit_offset = 0;
+  uint64_t data_bit_offset = UINT64_MAX;
+  size_t bit_size = 0;
+  bool is_external =
+  false; // On DW_TAG_members, this means the member is static
+  uint32_t i;
+  for (i = 0; i < num_attributes && !is_artificial; ++i) {
+const dw_attr_t attr = attributes.AttributeAtIndex(i);
+DWARFFormValue form_value;
+if (attrib

[Lldb-commits] [PATCH] D83541: Remove Linux sysroot dependencies of SVE PT macros

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

To use this code from Process/elf-core, we also need to move this file. 
`source/Plugins/Process/Utility` is the best place we got for this right now.




Comment at: lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h:9-10
 
 #ifndef lldb_LinuxPTraceDefines_arm64sve_h
 #define lldb_LinuxPTraceDefines_arm64sve_h
 

While, moving the file, please also adjust the header guard 
(`LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_FOO_H`) -- it looks like this has raced 
with the header guard unification we did in lldb some time ago...


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

https://reviews.llvm.org/D83541



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

[Lldb-commits] [lldb] f3b3689 - [lldb][NFC] Refactor instruction dumping out of DumpDataExtractor

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-07-13T15:03:40+02:00
New Revision: f3b3689c043f49ad42e9d3f5057bc8f1a9f56d09

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

LOG: [lldb][NFC] Refactor instruction dumping out of DumpDataExtractor

Added: 


Modified: 
lldb/source/Core/DumpDataExtractor.cpp

Removed: 




diff  --git a/lldb/source/Core/DumpDataExtractor.cpp 
b/lldb/source/Core/DumpDataExtractor.cpp
index 233a1b373550..33fc3a76d3d6 100644
--- a/lldb/source/Core/DumpDataExtractor.cpp
+++ b/lldb/source/Core/DumpDataExtractor.cpp
@@ -128,6 +128,53 @@ static lldb::offset_t DumpAPInt(Stream *s, const 
DataExtractor &data,
   return offset;
 }
 
+/// Dumps decoded instructions to a stream.
+static lldb::offset_t DumpInstructions(const DataExtractor &DE, Stream *s,
+   ExecutionContextScope *exe_scope,
+   offset_t start_offset,
+   uint64_t base_addr,
+   size_t number_of_instructions) {
+  offset_t offset = start_offset;
+
+  TargetSP target_sp;
+  if (exe_scope)
+target_sp = exe_scope->CalculateTarget();
+  if (target_sp) {
+DisassemblerSP disassembler_sp(
+Disassembler::FindPlugin(target_sp->GetArchitecture(),
+ target_sp->GetDisassemblyFlavor(), nullptr));
+if (disassembler_sp) {
+  lldb::addr_t addr = base_addr + start_offset;
+  lldb_private::Address so_addr;
+  bool data_from_file = true;
+  if (target_sp->GetSectionLoadList().ResolveLoadAddress(addr, so_addr)) {
+data_from_file = false;
+  } else {
+if (target_sp->GetSectionLoadList().IsEmpty() ||
+!target_sp->GetImages().ResolveFileAddress(addr, so_addr))
+  so_addr.SetRawAddress(addr);
+  }
+
+  size_t bytes_consumed = disassembler_sp->DecodeInstructions(
+  so_addr, DE, start_offset, number_of_instructions, false,
+  data_from_file);
+
+  if (bytes_consumed) {
+offset += bytes_consumed;
+const bool show_address = base_addr != LLDB_INVALID_ADDRESS;
+const bool show_bytes = true;
+ExecutionContext exe_ctx;
+exe_scope->CalculateExecutionContext(exe_ctx);
+disassembler_sp->GetInstructionList().Dump(s, show_address, show_bytes,
+   &exe_ctx);
+  }
+}
+  } else
+s->Printf("invalid target");
+
+  return offset;
+}
+
 lldb::offset_t lldb_private::DumpDataExtractor(
 const DataExtractor &DE, Stream *s, offset_t start_offset,
 lldb::Format item_format, size_t item_byte_size, size_t item_count,
@@ -147,44 +194,9 @@ lldb::offset_t lldb_private::DumpDataExtractor(
 
   offset_t offset = start_offset;
 
-  if (item_format == eFormatInstruction) {
-TargetSP target_sp;
-if (exe_scope)
-  target_sp = exe_scope->CalculateTarget();
-if (target_sp) {
-  DisassemblerSP disassembler_sp(Disassembler::FindPlugin(
-  target_sp->GetArchitecture(),
-  target_sp->GetDisassemblyFlavor(), nullptr));
-  if (disassembler_sp) {
-lldb::addr_t addr = base_addr + start_offset;
-lldb_private::Address so_addr;
-bool data_from_file = true;
-if (target_sp->GetSectionLoadList().ResolveLoadAddress(addr, so_addr)) 
{
-  data_from_file = false;
-} else {
-  if (target_sp->GetSectionLoadList().IsEmpty() ||
-  !target_sp->GetImages().ResolveFileAddress(addr, so_addr))
-so_addr.SetRawAddress(addr);
-}
-
-size_t bytes_consumed = disassembler_sp->DecodeInstructions(
-so_addr, DE, start_offset, item_count, false, data_from_file);
-
-if (bytes_consumed) {
-  offset += bytes_consumed;
-  const bool show_address = base_addr != LLDB_INVALID_ADDRESS;
-  const bool show_bytes = true;
-  ExecutionContext exe_ctx;
-  exe_scope->CalculateExecutionContext(exe_ctx);
-  disassembler_sp->GetInstructionList().Dump(s, show_address,
- show_bytes, &exe_ctx);
-}
-  }
-} else
-  s->Printf("invalid target");
-
-return offset;
-  }
+  if (item_format == eFormatInstruction)
+return DumpInstructions(DE, s, exe_scope, start_offset, base_addr,
+item_count);
 
   if ((item_format == eFormatOSType || item_format == eFormatAddressInfo) &&
   item_byte_size > 8)



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

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

Sorry this is taking so long, but I think beside one minor change this only 
needs some testing and then it's ready.

So the way I would like to see this tested would be with a pexpect test. 
Pexpect just launches a virtual terminal where LLDB runs inside and you can 
send input and read output. There is a good example test here that you can copy 
`lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py`.
 I think it's a good enough test if you just type any lldb command like `help 
frame` in the test, press enter and then check that if you type like `hel` you 
get the autosuggestion displayed in the terminal output. And then you press 
Ctrl+F in the terminal (you have to find the escape sequence for that when 
sending it to pexpect) and should see the complete autosuggestion.

Be aware that there is chance that the Pexpect terminal behaves differently 
than a real terminal and you see wrong results there. In that case we need to 
come up with another solution, but let's first try pexpect. Also you should 
know that pexpect tests don't know if LLDB is actually still producing output 
or idling, so if get timeouts while reading the output that just means we 
didn't see the right output.




Comment at: lldb/source/Host/common/Editline.cpp:1007
+  std::string to_add_autosuggestion = "";
+  m_suggestion_callback(line, to_add_autosuggestion,
+m_suggestion_callback_baton);

This will crash with disabled suggestions (m_suggestion_callback can be null if 
the feature is disabled).



Comment at: lldb/source/Host/common/Editline.cpp:1017
+  el_insertstr(m_editline, to_add.c_str());
+  return CC_REFRESH;
+}

gedatsu217 wrote:
> teemperor wrote:
> > If I understand the only effect this whole code has is to return CC_REFRESH 
> > instead of CC_REDISPLAY? If yes, then I think you can just replace the 
> > `break` below with `return CC_REFRESH;` which will be much simpler.
> > If yes, then I think you can just replace the break below with return 
> > CC_REFRESH; which will be much simpler.
> 
> Isn't it "return CC_REDISPLAY", not "CC_REFRESH"? I want to return CC_REFRESH 
> only when "to_add" is in "to_add_autosuggestion" (e.g. to_add = b, 
> to_add_autosuggestion = "breakpoint").  
> 
> That is because CC_REDISPLAY deletes the gray-colored autosuggested part, 
> while CC_REFRESH leaves it.
> 
> 
I see. What about just retuning always `CC_REFRESH` here? That should work as 
we only add text to the end with a normal completion (which is IIRC that's what 
`CC_REFRESH` is for, but 

```
lang=c++
case CompletionMode::Normal: {
  std::string to_add = completion.GetCompletion();
  to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
  std::string to_add_autosuggestion = "";
  to_add.push_back(' ');
  el_insertstr(m_editline, to_add.c_str());
  return CC_REFRESH;
}
```

That seems to work for me (and it avoids the crash I pointed out above).

Also my main point here is that this is quite a large change just to change the 
return value (and the other tab completions below aren't covered and would need 
similar changes if we do this change).


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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I didn't notice this before, but I see now that there's more register number 
overlap in `lldb-arm64-register-enums.h`. Having one overlapping enum is bad 
enough, but two seems really too much? Can we avoid the second enum somehow, at 
least? Perhaps by switching `RegisterContextCorePOSIX_arm64` to use the other 
enum definitions for its work?




Comment at: 
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp:256-257
+  uint32_t offset) {
+  // Register info mode denotes SVE vector length in context of AArch64.
+  // Register info mode once set to zero permanently selects default static
+  // AArch64 register info and cannot be changed to SVE. Also if an invalid

This comment looks outdated.



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp:280
+
+  m_sve_enabled = true;
+

IIUC, `m_sve_enabled` is only true if m_vector_reg_vq is not zero. Could we 
delete this variable and just make a function to make that comparison?



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp:288-291
+  if (m_per_vq_reg_infos.count(sve_vq)) {
+m_dynamic_register_infos = m_per_vq_reg_infos.at(sve_vq);
+m_register_info_p = &m_dynamic_register_infos[0];
+return m_vector_reg_vq;

It doesn't look like `m_dynamic_register_infos` is needed -- you could just 
make `m_register_info_p` point directly into the map.



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h:91
 
+  uint32_t ConfigureVectorRegisterInfos(uint32_t mode, uint32_t offset = 0);
+

The `offset` argument is not set anywhere.



Comment at: 
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h:17
 
+#ifndef SVE_PT_REGS_SVE
+#define INCLUDE_LINUX_PTRACE_DEFINITIONS_FOR_SVE_ARM64

I guess this does not make sense now that the header is standalone


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

https://reviews.llvm.org/D77047



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

[Lldb-commits] [PATCH] D83302: [lldb/DWARF] Don't treat class declarations with children as definitions

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D83302#2142155 , @aprantl wrote:

> I tried to do some radar archeology for more context, but I could neither 
> find a radar mentioning that commit, nor a mention of a radar in that commit.


Thanks for the effort.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83302



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

[Lldb-commits] [PATCH] D83545: [lldb/dotest] Remove the "xunit" result formatter

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa5803765d8e0: [lldb/dotest] Remove the "xunit" 
result formatter (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83545

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test_event/formatter/__init__.py
  lldb/packages/Python/lldbsuite/test_event/formatter/xunit.py

Index: lldb/packages/Python/lldbsuite/test_event/formatter/xunit.py
===
--- lldb/packages/Python/lldbsuite/test_event/formatter/xunit.py
+++ /dev/null
@@ -1,595 +0,0 @@
-"""
-Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-See https://llvm.org/LICENSE.txt for license information.
-SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-Provides an xUnit ResultsFormatter for integrating the LLDB
-test suite with the Jenkins xUnit aggregator and other xUnit-compliant
-test output processors.
-"""
-from __future__ import absolute_import
-from __future__ import print_function
-
-# System modules
-import re
-import sys
-import xml.sax.saxutils
-
-# Third-party modules
-import six
-
-# Local modules
-from ..event_builder import EventBuilder
-from ..build_exception import BuildError
-from .results_formatter import ResultsFormatter
-
-
-class XunitFormatter(ResultsFormatter):
-"""Provides xUnit-style formatted output.
-"""
-
-# Result mapping arguments
-RM_IGNORE = 'ignore'
-RM_SUCCESS = 'success'
-RM_FAILURE = 'failure'
-RM_PASSTHRU = 'passthru'
-
-@staticmethod
-def _build_illegal_xml_regex():
-"""Constructs a regex to match all illegal xml characters.
-
-Expects to be used against a unicode string."""
-# Construct the range pairs of invalid unicode characters.
-illegal_chars_u = [
-(0x00, 0x08), (0x0B, 0x0C), (0x0E, 0x1F), (0x7F, 0x84),
-(0x86, 0x9F), (0xFDD0, 0xFDDF), (0xFFFE, 0x)]
-
-# For wide builds, we have more.
-if sys.maxunicode >= 0x1:
-illegal_chars_u.extend(
-[(0x1FFFE, 0x1), (0x2FFFE, 0x2), (0x3FFFE, 0x3),
- (0x4FFFE, 0x4), (0x5FFFE, 0x5), (0x6FFFE, 0x6),
- (0x7FFFE, 0x7), (0x8FFFE, 0x8), (0x9FFFE, 0x9),
- (0xAFFFE, 0xA), (0xBFFFE, 0xB), (0xCFFFE, 0xC),
- (0xDFFFE, 0xD), (0xEFFFE, 0xE), (0xE, 0xF),
- (0x10FFFE, 0x10)])
-
-# Build up an array of range expressions.
-illegal_ranges = [
-"%s-%s" % (six.unichr(low), six.unichr(high))
-for (low, high) in illegal_chars_u]
-
-# Compile the regex
-return re.compile(six.u('[%s]') % six.u('').join(illegal_ranges))
-
-@staticmethod
-def _quote_attribute(text):
-"""Returns the given text in a manner safe for usage in an XML attribute.
-
-@param text the text that should appear within an XML attribute.
-@return the attribute-escaped version of the input text.
-"""
-return xml.sax.saxutils.quoteattr(text)
-
-def _replace_invalid_xml(self, str_or_unicode):
-"""Replaces invalid XML characters with a '?'.
-
-@param str_or_unicode a string to replace invalid XML
-characters within.  Can be unicode or not.  If not unicode,
-assumes it is a byte string in utf-8 encoding.
-
-@returns a utf-8-encoded byte string with invalid
-XML replaced with '?'.
-"""
-# Get the content into unicode
-if isinstance(str_or_unicode, str):
-# If we hit decoding errors due to data corruption, replace the
-# invalid characters with U+FFFD REPLACEMENT CHARACTER.
-unicode_content = str_or_unicode.decode('utf-8', 'replace')
-else:
-unicode_content = str_or_unicode
-return self.invalid_xml_re.sub(
-six.u('?'), unicode_content).encode('utf-8')
-
-@classmethod
-def arg_parser(cls):
-"""@return arg parser used to parse formatter-specific options."""
-parser = super(XunitFormatter, cls).arg_parser()
-
-# These are valid choices for results mapping.
-results_mapping_choices = [
-XunitFormatter.RM_IGNORE,
-XunitFormatter.RM_SUCCESS,
-XunitFormatter.RM_FAILURE,
-XunitFormatter.RM_PASSTHRU]
-parser.add_argument(
-"--assert-on-unknown-events",
-action="store_true",
-help=('cause unknown test events to generate '
-  'a python assert.  Default is to ignore.'))
-parser.add_argument(
-"--ignore-skip-name",
-   

[Lldb-commits] [lldb] a580376 - [lldb/dotest] Remove the "xunit" result formatter

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-13T16:33:38+02:00
New Revision: a5803765d8e0b62e0b48ea76bcad07a7c183618b

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

LOG: [lldb/dotest] Remove the "xunit" result formatter

Summary:
My understanding is that this was added to make dotest interact well
with the GreenDragon bots, back when dotest was the main test driver.
Now that everything goes through lit (which has its own xunit
formatter), it seems largely irrelevant.

There are more cleanups that can be done after removing this be done
here, but this should be enough to test the waters.

Reviewers: JDevlieghere

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/dotest_args.py
lldb/packages/Python/lldbsuite/test_event/formatter/__init__.py

Removed: 
lldb/packages/Python/lldbsuite/test_event/formatter/xunit.py



diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index ca2786446300..84de0130f990 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -122,10 +122,6 @@
 clang_module_cache_dir = None
 
 # Test results handling globals
-results_filename = None
-results_formatter_name = None
-results_formatter_object = None
-results_formatter_options = None
 test_result = None
 
 # Reproducers

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 8238168d0fb6..67f227cad715 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -408,19 +408,6 @@ def parseOptionsAndInitTestdirs():
 if do_help:
 usage(parser)
 
-if args.results_file:
-configuration.results_filename = args.results_file
-
-if args.results_formatter:
-configuration.results_formatter_name = args.results_formatter
-if args.results_formatter_options:
-configuration.results_formatter_options = 
args.results_formatter_options
-
-# Default to using the BasicResultsFormatter if no formatter is specified.
-if configuration.results_formatter_name is None:
-configuration.results_formatter_name = (
-
"lldbsuite.test_event.formatter.results_formatter.ResultsFormatter")
-
 # Reproducer arguments
 if args.capture_path and args.replay_path:
 logging.error('Cannot specify both a capture and a replay path.')
@@ -469,16 +456,10 @@ def parseOptionsAndInitTestdirs():
 
 def setupTestResults():
 """Sets up test results-related objects based on arg settings."""
-# Setup the results formatter configuration.
-formatter_config = formatter.FormatterConfig()
-formatter_config.filename = configuration.results_filename
-formatter_config.formatter_name = configuration.results_formatter_name
-formatter_config.formatter_options = (
-configuration.results_formatter_options)
 
 # Create the results formatter.
 formatter_spec = formatter.create_results_formatter(
-formatter_config)
+
"lldbsuite.test_event.formatter.results_formatter.ResultsFormatter")
 if formatter_spec is not None and formatter_spec.formatter is not None:
 configuration.results_formatter_object = formatter_spec.formatter
 

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest_args.py 
b/lldb/packages/Python/lldbsuite/test/dotest_args.py
index d6f59efdf28b..05dd523e744a 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -244,28 +244,6 @@ def create_parser():
 help='(Windows only) When LLDB crashes, display the Windows crash 
dialog.')
 group.set_defaults(disable_crash_dialog=True)
 
-# Test results support.
-group = parser.add_argument_group('Test results options')
-group.add_argument(
-'--results-file',
-action='store',
-help=('Specifies the file where test results will be written '
-  'according to the results-formatter class used'))
-group.add_argument(
-'--results-formatter',
-action='store',
-help=('Specifies the full package/module/class name used to translate '
-  'test events into some kind of meaningful report, written to '
-  'the designated output results file-like object'))
-group.add_argument(
-'--results-formatter-option',
-'-O',
-action='append',
-dest='results_formatter_options',
-help=('Specify an option to pass to the f

[Lldb-commits] [lldb] 1847f4d - [lldb/Utility] Rewrite Scalar::SetValueFromCString

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-13T16:44:42+02:00
New Revision: 1847f4dd7570f01f70646cd5067dd0c34257cd21

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

LOG: [lldb/Utility] Rewrite Scalar::SetValueFromCString

The function's reliance on host types meant that it was needlessly
complicated, and did not handle the newer (wider) types. Rewrite it in
terms of APInt/APFloat functions to save code and improve functionality.

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 87ac6c23892d..6c48bbde532f 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -24,6 +24,7 @@ using namespace lldb;
 using namespace lldb_private;
 
 using llvm::APFloat;
+using llvm::APInt;
 
 namespace {
 enum class Category { Void, Integral, Float };
@@ -1002,116 +1003,60 @@ Status Scalar::SetValueFromCString(const char 
*value_str, Encoding encoding,
 error.SetErrorString("Invalid encoding.");
 break;
 
-  case eEncodingUint:
-if (byte_size <= sizeof(uint64_t)) {
-  uint64_t uval64;
-  if (!llvm::to_integer(value_str, uval64))
-error.SetErrorStringWithFormat(
-"'%s' is not a valid unsigned integer string value", value_str);
-  else if (!UIntValueIsValidForSize(uval64, byte_size))
-error.SetErrorStringWithFormat(
-"value 0x%" PRIx64 " is too large to fit in a %" PRIu64
-" byte unsigned integer value",
-uval64, static_cast(byte_size));
-  else {
-m_type = Scalar::GetValueTypeForUnsignedIntegerWithByteSize(byte_size);
-switch (m_type) {
-case e_uint:
-  m_integer = llvm::APInt(sizeof(uint_t) * 8, uval64, false);
-  break;
-case e_ulong:
-  m_integer = llvm::APInt(sizeof(ulong_t) * 8, uval64, false);
-  break;
-case e_ulonglong:
-  m_integer = llvm::APInt(sizeof(ulonglong_t) * 8, uval64, false);
-  break;
-default:
-  error.SetErrorStringWithFormat(
-  "unsupported unsigned integer byte size: %" PRIu64 "",
-  static_cast(byte_size));
-  break;
-}
-  }
-} else {
-  error.SetErrorStringWithFormat(
-  "unsupported unsigned integer byte size: %" PRIu64 "",
-  static_cast(byte_size));
-  return error;
-}
-break;
-
   case eEncodingSint:
-if (byte_size <= sizeof(int64_t)) {
-  int64_t sval64;
-  if (!llvm::to_integer(value_str, sval64))
-error.SetErrorStringWithFormat(
-"'%s' is not a valid signed integer string value", value_str);
-  else if (!SIntValueIsValidForSize(sval64, byte_size))
-error.SetErrorStringWithFormat(
-"value 0x%" PRIx64 " is too large to fit in a %" PRIu64
-" byte signed integer value",
-sval64, static_cast(byte_size));
-  else {
-m_type = Scalar::GetValueTypeForSignedIntegerWithByteSize(byte_size);
-switch (m_type) {
-case e_sint:
-  m_integer = llvm::APInt(sizeof(sint_t) * 8, sval64, true);
-  break;
-case e_slong:
-  m_integer = llvm::APInt(sizeof(slong_t) * 8, sval64, true);
-  break;
-case e_slonglong:
-  m_integer = llvm::APInt(sizeof(slonglong_t) * 8, sval64, true);
-  break;
-default:
-  error.SetErrorStringWithFormat(
-  "unsupported signed integer byte size: %" PRIu64 "",
-  static_cast(byte_size));
-  break;
-}
-  }
-} else {
-  error.SetErrorStringWithFormat(
-  "unsupported signed integer byte size: %" PRIu64 "",
-  static_cast(byte_size));
-  return error;
+  case eEncodingUint: {
+llvm::StringRef str = value_str;
+bool is_signed = encoding == eEncodingSint;
+bool is_negative = is_signed && str.consume_front("-");
+APInt integer;
+if (str.getAsInteger(0, integer)) {
+  error.SetErrorStringWithFormatv(
+  "'{0}' is not a valid integer string value", value_str);
+  break;
+}
+bool fits;
+if (is_signed) {
+  integer = integer.zext(integer.getBitWidth() + 1);
+  if (is_negative)
+integer.negate();
+  fits = integer.isSignedIntN(byte_size * 8);
+} else
+  fits = integer.isIntN(byte_size * 8);
+if (!fits) {
+  error.SetErrorStringWithFormatv(
+  "value {0} is too large to fit in a {1} byte integer value",
+  value_str, byte_size);
+  break;
+}
+m_type = GetBestTypeForBitSize(8 * byte_size, is_signed);
+if (m_type == e_void) {
+  error.SetErrorStringWithFormatv("unsupported 

[Lldb-commits] [lldb] 340c376 - [lldb] Fix a CMake warning typo. NFC.

[Martin Storsjö via lldb-commits]

Author: Martin Storsjö
Date: 2020-07-13T22:48:17+03:00
New Revision: 340c376b87c72e7eb3670301e4920106615b6689

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

LOG: [lldb] Fix a CMake warning typo. NFC.

Added: 


Modified: 
lldb/tools/debugserver/source/CMakeLists.txt

Removed: 




diff  --git a/lldb/tools/debugserver/source/CMakeLists.txt 
b/lldb/tools/debugserver/source/CMakeLists.txt
index 9a7e2eb9a1a0..b29b3ddc3056 100644
--- a/lldb/tools/debugserver/source/CMakeLists.txt
+++ b/lldb/tools/debugserver/source/CMakeLists.txt
@@ -41,7 +41,7 @@ function(get_debugserver_codesign_identity result)
 return()
   endif()
 
-  message(WARNING "Development code sign identiy not found: 'lldb_codesign' 
${not_found_help}")
+  message(WARNING "Development code sign identity not found: 'lldb_codesign' 
${not_found_help}")
 
   # LLVM pendant: fallback if available
   if(LLVM_CODESIGNING_IDENTITY)



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

[Lldb-commits] [lldb] 341ec56 - Add a decorator to skip tests when running under Rosetta

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2020-07-13T13:09:53-07:00
New Revision: 341ec564182161861ec4415cdee1f4f3a0527e97

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

LOG: Add a decorator to skip tests when running under Rosetta

This allows skipping a test when running the testsuite on macOS under
the Rosetta translation layer.

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

Added: 


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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index ec084184cd65..be282f6db32c 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -552,6 +552,14 @@ def are_sb_headers_missing():
 return skipTestIfFn(are_sb_headers_missing)(func)
 
 
+def skipIfRosetta(func, bugnumber=None):
+"""Skip a test when running the testsuite on macOS under the Rosetta 
translation layer."""
+def is_running_rosetta(self):
+if not lldbplatformutil.getPlatform() in ['darwin', 'macosx']:
+return False
+return platform.uname()[5] == "arm" and self.getArchitecture() == 
"x86_64"
+return skipTestIfFn(is_running_rosetta, bugnumber)(func)
+
 def skipIfiOSSimulator(func):
 """Decorate the item to skip tests that should be skipped on the iOS 
Simulator."""
 def is_ios_simulator():



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

[Lldb-commits] [PATCH] D83600: Add a decorator to skip tests when running under Rosetta

[Adrian Prantl via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG341ec5641821: Add a decorator to skip tests when running 
under Rosetta (authored by aprantl).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83600

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


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -552,6 +552,14 @@
 return skipTestIfFn(are_sb_headers_missing)(func)
 
 
+def skipIfRosetta(func, bugnumber=None):
+"""Skip a test when running the testsuite on macOS under the Rosetta 
translation layer."""
+def is_running_rosetta(self):
+if not lldbplatformutil.getPlatform() in ['darwin', 'macosx']:
+return False
+return platform.uname()[5] == "arm" and self.getArchitecture() == 
"x86_64"
+return skipTestIfFn(is_running_rosetta, bugnumber)(func)
+
 def skipIfiOSSimulator(func):
 """Decorate the item to skip tests that should be skipped on the iOS 
Simulator."""
 def is_ios_simulator():


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -552,6 +552,14 @@
 return skipTestIfFn(are_sb_headers_missing)(func)
 
 
+def skipIfRosetta(func, bugnumber=None):
+"""Skip a test when running the testsuite on macOS under the Rosetta translation layer."""
+def is_running_rosetta(self):
+if not lldbplatformutil.getPlatform() in ['darwin', 'macosx']:
+return False
+return platform.uname()[5] == "arm" and self.getArchitecture() == "x86_64"
+return skipTestIfFn(is_running_rosetta, bugnumber)(func)
+
 def skipIfiOSSimulator(func):
 """Decorate the item to skip tests that should be skipped on the iOS Simulator."""
 def is_ios_simulator():
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Shu Anzai via Phabricator via lldb-commits]

gedatsu217 marked an inline comment as done.
gedatsu217 added inline comments.



Comment at: lldb/source/Host/common/Editline.cpp:1017
+  el_insertstr(m_editline, to_add.c_str());
+  return CC_REFRESH;
+}

teemperor wrote:
> gedatsu217 wrote:
> > teemperor wrote:
> > > If I understand the only effect this whole code has is to return 
> > > CC_REFRESH instead of CC_REDISPLAY? If yes, then I think you can just 
> > > replace the `break` below with `return CC_REFRESH;` which will be much 
> > > simpler.
> > > If yes, then I think you can just replace the break below with return 
> > > CC_REFRESH; which will be much simpler.
> > 
> > Isn't it "return CC_REDISPLAY", not "CC_REFRESH"? I want to return 
> > CC_REFRESH only when "to_add" is in "to_add_autosuggestion" (e.g. to_add = 
> > b, to_add_autosuggestion = "breakpoint").  
> > 
> > That is because CC_REDISPLAY deletes the gray-colored autosuggested part, 
> > while CC_REFRESH leaves it.
> > 
> > 
> I see. What about just retuning always `CC_REFRESH` here? That should work as 
> we only add text to the end with a normal completion (which is IIRC that's 
> what `CC_REFRESH` is for, but 
> 
> ```
> lang=c++
> case CompletionMode::Normal: {
>   std::string to_add = completion.GetCompletion();
>   to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
>   std::string to_add_autosuggestion = "";
>   to_add.push_back(' ');
>   el_insertstr(m_editline, to_add.c_str());
>   return CC_REFRESH;
> }
> ```
> 
> That seems to work for me (and it avoids the crash I pointed out above).
> 
> Also my main point here is that this is quite a large change just to change 
> the return value (and the other tab completions below aren't covered and 
> would need similar changes if we do this change).
Where is "to_add_autosuggestion" used in the above example?


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

https://reviews.llvm.org/D81001



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

[Lldb-commits] [lldb] 32d35fb - [lldb] Remove unused argument (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-07-13T13:44:51-07:00
New Revision: 32d35fb74b2672ddf3674188423b71837afea8c4

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

LOG: [lldb] Remove unused argument (NFC)

Nobody is writing to the stream so there's no point in passing it
around.

Added: 


Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/API/SBTarget.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Target/Platform.cpp
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index a3fb3447169e..bf9b64547ed5 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -737,7 +737,7 @@ class Process : public 
std::enable_shared_from_this,
   ///
   /// \return
   /// Returns an error object.
-  virtual Status ConnectRemote(Stream *strm, llvm::StringRef remote_url);
+  virtual Status ConnectRemote(llvm::StringRef remote_url);
 
   bool GetShouldDetach() const { return m_should_detach; }
 
@@ -925,7 +925,7 @@ class Process : public 
std::enable_shared_from_this,
   ///
   /// \return
   /// Returns an error object.
-  virtual Status DoConnectRemote(Stream *strm, llvm::StringRef remote_url) {
+  virtual Status DoConnectRemote(llvm::StringRef remote_url) {
 Status error;
 error.SetErrorString("remote connections are not supported");
 return error;

diff  --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index ca75e91bd906..b84e9f10fafe 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -566,7 +566,7 @@ lldb::SBProcess SBTarget::ConnectRemote(SBListener 
&listener, const char *url,
 
 if (process_sp) {
   sb_process.SetSP(process_sp);
-  error.SetError(process_sp->ConnectRemote(nullptr, url));
+  error.SetError(process_sp->ConnectRemote(url));
 } else {
   error.SetErrorString("unable to create lldb_private::Process");
 }

diff  --git 
a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp 
b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
index 18631a0c5315..21bf7f4ac46d 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -503,10 +503,10 @@ lldb::ProcessSP PlatformRemoteGDBServer::DebugProcess(
  "gdb-remote", nullptr);
 
   if (process_sp) {
-error = process_sp->ConnectRemote(nullptr, connect_url.c_str());
+error = process_sp->ConnectRemote(connect_url.c_str());
 // Retry the connect remote one time...
 if (error.Fail())
-  error = process_sp->ConnectRemote(nullptr, connect_url.c_str());
+  error = process_sp->ConnectRemote(connect_url.c_str());
 if (error.Success())
   error = process_sp->Launch(launch_info);
 else if (debugserver_pid != LLDB_INVALID_PROCESS_ID) {
@@ -589,7 +589,7 @@ lldb::ProcessSP PlatformRemoteGDBServer::Attach(
   
target->CreateProcess(attach_info.GetListenerForProcess(debugger),
 "gdb-remote", nullptr);
   if (process_sp) {
-error = process_sp->ConnectRemote(nullptr, connect_url.c_str());
+error = process_sp->ConnectRemote(connect_url.c_str());
 if (error.Success()) {
   ListenerSP listener_sp = attach_info.GetHijackListener();
   if (listener_sp)

diff  --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp 
b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
index 5b728a5f2960..2f4a8917a78a 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -217,7 +217,7 @@ bool ProcessKDP::GetHostArchitecture(ArchSpec &arch) {
   return false;
 }
 
-Status ProcessKDP::DoConnectRemote(Stream *strm, llvm::StringRef remote_url) {
+Status ProcessKDP::DoConnectRemote(llvm::StringRef remote_url) {
   Status error;
 
   // Don't let any JIT happen when doing KDP as we can't allocate memory and we

diff  --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h 
b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
index 67f8ac069820..52af56134404 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/Proces

[Lldb-commits] [lldb] 77c9aaf - Retry ""[lldb-vscode] Fix TestVSCode_module""

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-07-13T14:12:03-07:00
New Revision: 77c9aafc5d85a816c20d1f1fb176024bc0b8d0fe

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

LOG: Retry ""[lldb-vscode] Fix TestVSCode_module""

Original commit c60216db15132401ff60c08ccef899321f63b6b6.

The test can only run on Darwin because of how it was setup, so I'm
enforcing that.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Added: 
lldb/test/API/tools/lldb-vscode/module/Makefile
lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
lldb/test/API/tools/lldb-vscode/module/foo.cpp
lldb/test/API/tools/lldb-vscode/module/foo.h
lldb/test/API/tools/lldb-vscode/module/main.cpp

Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/JSONUtils.h
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 1ad168e794cf..6b1c1c961b54 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -113,6 +113,7 @@ def __init__(self, recv, send, init_commands):
 self.initialize_body = None
 self.thread_stop_reasons = {}
 self.breakpoint_events = []
+self.module_events = {}
 self.sequence = 1
 self.threads = None
 self.recv_thread.start()
@@ -133,6 +134,9 @@ def validate_response(cls, command, response):
 if command['seq'] != response['request_seq']:
 raise ValueError('seq mismatch in response')
 
+def get_active_modules(self):
+return self.module_events
+
 def get_output(self, category, timeout=0.0, clear=True):
 self.output_condition.acquire()
 output = None
@@ -218,6 +222,15 @@ def handle_recv_packet(self, packet):
 self.breakpoint_events.append(packet)
 # no need to add 'breakpoint' event packets to our packets list
 return keepGoing
+elif event == 'module':
+reason = body['reason']
+if (reason == 'new' or reason == 'changed'):
+self.module_events[body['module']['name']] = body['module']
+elif reason == 'removed':
+if body['module']['name'] in self.module_events:
+self.module_events.pop(body['module']['name'])
+return keepGoing
+
 elif packet_type == 'response':
 if packet['command'] == 'disconnect':
 keepGoing = False
@@ -747,6 +760,16 @@ def request_setFunctionBreakpoints(self, names, 
condition=None,
 }
 return self.send_recv(command_dict)
 
+def request_getCompileUnits(self, moduleId):
+args_dict = {'moduleId': moduleId}
+command_dict = {
+'command': 'getCompileUnits',
+'type': 'request',
+'arguments': args_dict
+}
+response = self.send_recv(command_dict)
+return response
+
 def request_completions(self, text):
 args_dict = {
 'text': text,

diff  --git a/lldb/test/API/tools/lldb-vscode/module/Makefile 
b/lldb/test/API/tools/lldb-vscode/module/Makefile
new file mode 100644
index ..1fb944b13893
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/module/Makefile
@@ -0,0 +1,13 @@
+DYLIB_NAME := foo
+DYLIB_CXX_SOURCES := foo.cpp
+CXX_SOURCES := main.cpp
+
+all: a.out.stripped
+
+include Makefile.rules
+
+a.out.stripped: a.out.dSYM
+   strip -o a.out.stripped a.out
+ifneq "$(CODESIGN)" ""
+   $(CODESIGN) -fs - a.out.stripped
+endif

diff  --git a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py 
b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
new file mode 100644
index ..461ac201a73f
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -0,0 +1,72 @@
+"""
+Test lldb-vscode setBreakpoints request
+"""
+
+from __future__ import print_function
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_module(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+
+@skipIfWindows
+@skipUnlessDarwin
+@skipIfRemote
+def test_modules_event(self):
+program_basename = "a.out.stripped"
+program= self.getBuildArtifact(program_basename)
+self.build_and_launch(program)
+ 

[Lldb-commits] [PATCH] D83728: [lldb] Make `process connect` behave correctly in synchronous mode.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jingham.
Herald added a subscriber: abidh.

In synchronous mode, the `process connect` command and its aliases should wait 
for the stop event before claiming the command is complete. Currently, the stop 
event is always handled asynchronously by the debugger. The implementation 
takes the same approach as `Process::ResumeSynchronous` which hijacks the event 
and handles it on the current thread. Similarly, after this patch, the stop 
event is part of the command return object, which is the property used by the 
test case.

Most of the discussion for this patch took place in D83446 



Repository:
  rLLDB LLDB

https://reviews.llvm.org/D83728

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Target/Platform.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -0,0 +1,89 @@
+import lldb
+import binascii
+import os
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+def hexlify(string):
+return binascii.hexlify(string.encode()).decode()
+
+
+class ProcesConnectResponder(MockGDBServerResponder):
+def __init__(self):
+MockGDBServerResponder.__init__(self)
+self.currentQsProc = 0
+self.all_users = False
+
+def qfProcessInfo(self, packet):
+if "all_users:1" in packet:
+self.all_users = True
+name = hexlify("/a/test_process")
+args = "-".join(
+map(hexlify,
+["/system/bin/sh", "-c", "/data/local/tmp/lldb-server"]))
+return "pid:10;ppid:1;uid:2;gid:3;euid:4;egid:5;name:" + name + ";args:" + args + ";"
+else:
+self.all_users = False
+return "E04"
+
+def qsProcessInfo(self):
+if self.all_users:
+if self.currentQsProc == 0:
+self.currentQsProc = 1
+name = hexlify("/b/another_test_process")
+# This intentionally has a badly encoded argument
+args = "X".join(map(hexlify, ["/system/bin/ls", "--help"]))
+return "pid:11;ppid:2;uid:3;gid:4;euid:5;egid:6;name:" + name + ";args:" + args + ";"
+elif self.currentQsProc == 1:
+self.currentQsProc = 0
+return "E04"
+else:
+return "E04"
+
+
+class TestProcesConnect(GDBRemoteTestBase):
+def test_gdb_remote_sync(self):
+"""Test the gdb-remote command in synchronous mode"""
+self.server.responder = ProcesConnectResponder()
+try:
+self.dbg.SetAsync(False)
+self.expect("gdb-remote %d" % self.server.port,
+substrs=['Process', 'stopped'])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_gdb_remote_async(self):
+"""Test the gdb-remote command in asynchronous mode"""
+self.server.responder = ProcesConnectResponder()
+try:
+self.dbg.SetAsync(True)
+self.expect("gdb-remote %d" % self.server.port,
+matching=False,
+substrs=['Process', 'stopped'])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_process_connect_sync(self):
+"""Test the gdb-remote command in synchronous mode"""
+self.server.responder = ProcesConnectResponder()
+try:
+self.dbg.SetAsync(False)
+self.expect("process connect connect://localhost:%d" %
+self.server.port,
+substrs=['Process', 'stopped'])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_process_connect_async(self):
+"""Test the gdb-remote command in asynchronous mode"""
+self.server.responder = ProcesConnectResponder()
+try:
+self.dbg.SetAsync(True)
+self.expect("process connect connect://localhost:%d" %
+self.server.port,
+matching=False,
+substrs=['Process', 'stopped'])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1774,9 +1774,23 @@
 
 lldb::ProcessSP Platform::ConnectProcess(llvm::StringRef connect_url,
  llvm::StringRef plugin_name,
- lldb_private::D

[Lldb-commits] [PATCH] D83728: [lldb] Make `process connect` blocking in synchronous mode.

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Couple of minor comments.




Comment at: lldb/include/lldb/Target/Platform.h:855
 protected:
+  lldb::ProcessSP DoConnectProcess(llvm::StringRef connect_url,
+   llvm::StringRef plugin_name,

Even though this is a private method it's still worth documenting the fact that 
you are switching off of "stream != nullptr" to determine sync vrs. async 
attach.  That's not obvious.



Comment at: lldb/source/Target/Platform.cpp:1834
   if (error.Fail())
 return nullptr;
 

If you fail here you leave the process hijacked.  That doesn't matter because 
if "ConnectRemote" fails, you aren't going to have much to listen to anyway.  
But it still looks odd.  I'm surprised we don't have some RAII-dingus for 
process hijacking, but anyway, it's good practice to undo this in the error 
branch.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D83728



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

[Lldb-commits] [PATCH] D83728: [lldb] Make `process connect` blocking in synchronous mode.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 277609.
JDevlieghere added a comment.

Address Jim's feedback


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

https://reviews.llvm.org/D83728

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Target/Platform.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -0,0 +1,89 @@
+import lldb
+import binascii
+import os
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+def hexlify(string):
+return binascii.hexlify(string.encode()).decode()
+
+
+class ProcesConnectResponder(MockGDBServerResponder):
+def __init__(self):
+MockGDBServerResponder.__init__(self)
+self.currentQsProc = 0
+self.all_users = False
+
+def qfProcessInfo(self, packet):
+if "all_users:1" in packet:
+self.all_users = True
+name = hexlify("/a/test_process")
+args = "-".join(
+map(hexlify,
+["/system/bin/sh", "-c", "/data/local/tmp/lldb-server"]))
+return "pid:10;ppid:1;uid:2;gid:3;euid:4;egid:5;name:" + name + ";args:" + args + ";"
+else:
+self.all_users = False
+return "E04"
+
+def qsProcessInfo(self):
+if self.all_users:
+if self.currentQsProc == 0:
+self.currentQsProc = 1
+name = hexlify("/b/another_test_process")
+# This intentionally has a badly encoded argument
+args = "X".join(map(hexlify, ["/system/bin/ls", "--help"]))
+return "pid:11;ppid:2;uid:3;gid:4;euid:5;egid:6;name:" + name + ";args:" + args + ";"
+elif self.currentQsProc == 1:
+self.currentQsProc = 0
+return "E04"
+else:
+return "E04"
+
+
+class TestProcesConnect(GDBRemoteTestBase):
+def test_gdb_remote_sync(self):
+"""Test the gdb-remote command in synchronous mode"""
+self.server.responder = ProcesConnectResponder()
+try:
+self.dbg.SetAsync(False)
+self.expect("gdb-remote %d" % self.server.port,
+substrs=['Process', 'stopped'])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_gdb_remote_async(self):
+"""Test the gdb-remote command in asynchronous mode"""
+self.server.responder = ProcesConnectResponder()
+try:
+self.dbg.SetAsync(True)
+self.expect("gdb-remote %d" % self.server.port,
+matching=False,
+substrs=['Process', 'stopped'])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_process_connect_sync(self):
+"""Test the gdb-remote command in synchronous mode"""
+self.server.responder = ProcesConnectResponder()
+try:
+self.dbg.SetAsync(False)
+self.expect("process connect connect://localhost:%d" %
+self.server.port,
+substrs=['Process', 'stopped'])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_process_connect_async(self):
+"""Test the gdb-remote command in asynchronous mode"""
+self.server.responder = ProcesConnectResponder()
+try:
+self.dbg.SetAsync(True)
+self.expect("process connect connect://localhost:%d" %
+self.server.port,
+matching=False,
+substrs=['Process', 'stopped'])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -12,9 +12,6 @@
 #include 
 #include 
 
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Path.h"
-
 #include "lldb/Breakpoint/BreakpointIDList.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Debugger.h"
@@ -40,8 +37,8 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StructuredData.h"
-
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 
 // Define these constants from POSIX mman.h rather than include the file so
 // that they will be correct even when compiled on Linux.
@@ -1774,9 +1771,23 @@
 
 lldb::ProcessSP Platform::ConnectProcess(llvm::StringRef connect_url,
  llvm::StringRef plugin_name,
-   

[Lldb-commits] [PATCH] D83728: [lldb] Make `process connect` blocking in synchronous mode.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 3 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Target/Platform.cpp:1834
   if (error.Fail())
 return nullptr;
 

jingham wrote:
> If you fail here you leave the process hijacked.  That doesn't matter because 
> if "ConnectRemote" fails, you aren't going to have much to listen to anyway.  
> But it still looks odd.  I'm surprised we don't have some RAII-dingus for 
> process hijacking, but anyway, it's good practice to undo this in the error 
> branch.
Yeah, that's exactly my reasoning. You can't use a RAII object here, because 
the order of destruction is undefined, so you might end up calling 
`RestoreProcessEvents` after the shared pointer has been destructed. Anyway, 
I've added the call just for consistency. 


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

https://reviews.llvm.org/D83728



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

[Lldb-commits] [PATCH] D83728: [lldb] Make `process connect` blocking in synchronous mode.

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D83728



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

[Lldb-commits] [PATCH] D83731: Add Debug Info Size to Symbol Status

[Yifan Shen via Phabricator via lldb-commits]

aelitashen created this revision.
aelitashen added reviewers: wallace, clayborg.
Herald added subscribers: lldb-commits, aprantl.
Herald added a project: LLDB.

Debug Info Size is displayed with Symbol Status Message.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83731

Files:
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h


Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -443,6 +443,8 @@
 
 llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit);
 
+uint64_t DebugInfoInSection(lldb::SBSection section);
+
 } // namespace lldb_vscode
 
 #endif
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -338,7 +338,32 @@
   std::string module_path(module_path_arr);
   object.try_emplace("path", module_path);
   if (module.GetNumCompileUnits() > 0) {
-object.try_emplace("symbolStatus", "Symbols loaded.");
+std::string symbol_str = "Symbols loaded.";
+uint64_t debug_info = 0;
+size_t num_sections = module.GetNumSections();
+if (num_sections > 0) {
+  for (int i = 0; i < (int)num_sections; i++) {
+lldb::SBSection section = module.GetSectionAtIndex(i);
+debug_info += DebugInfoInSection(section);
+  }
+}
+if (debug_info > 0) {
+  char debug_info_size[10];
+  if (debug_info < 1024) {
+sprintf(debug_info_size, " (%lluKB)", debug_info);
+  } else if (debug_info < 1024*1024) {
+debug_info = double(debug_info*10/1024);
+sprintf(debug_info_size, " (%.1fKB)", double(debug_info/10));
+  } else if (debug_info < 1024*1024*1024) {
+debug_info = debug_info*10/(1024*1024);
+sprintf(debug_info_size, " (%.1fMB)", double(debug_info/10));
+  } else {
+debug_info = debug_info*10/(1024*1024*1024);
+sprintf(debug_info_size, " (%.1fGB)", double(debug_info/10));
+  }
+  symbol_str = symbol_str + debug_info_size;
+}
+object.try_emplace("symbolStatus", symbol_str);
 char symbol_path_arr[PATH_MAX];
 module.GetSymbolFileSpec().GetPath(symbol_path_arr, 
sizeof(symbol_path_arr));
 std::string symbol_path(symbol_path_arr);
@@ -362,6 +387,16 @@
   return llvm::json::Value(std::move(object));
 }
 
+uint64_t DebugInfoInSection(lldb::SBSection section) {
+  uint64_t section_size = 0;
+  size_t num_sub_sections = section.GetNumSubSections();
+  for (int i = 0; i < (int)num_sub_sections; i++) {
+lldb::SBSection sub_section = section.GetSubSectionAtIndex(i);
+section_size += sub_section.GetFileByteSize();
+  }
+  return section_size;
+}
+
 void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints,
   llvm::Optional request_path,
   llvm::Optional request_line) {


Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -443,6 +443,8 @@
 
 llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit);
 
+uint64_t DebugInfoInSection(lldb::SBSection section);
+
 } // namespace lldb_vscode
 
 #endif
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -338,7 +338,32 @@
   std::string module_path(module_path_arr);
   object.try_emplace("path", module_path);
   if (module.GetNumCompileUnits() > 0) {
-object.try_emplace("symbolStatus", "Symbols loaded.");
+std::string symbol_str = "Symbols loaded.";
+uint64_t debug_info = 0;
+size_t num_sections = module.GetNumSections();
+if (num_sections > 0) {
+  for (int i = 0; i < (int)num_sections; i++) {
+lldb::SBSection section = module.GetSectionAtIndex(i);
+debug_info += DebugInfoInSection(section);
+  }
+}
+if (debug_info > 0) {
+  char debug_info_size[10];
+  if (debug_info < 1024) {
+sprintf(debug_info_size, " (%lluKB)", debug_info);
+  } else if (debug_info < 1024*1024) {
+debug_info = double(debug_info*10/1024);
+sprintf(debug_info_size, " (%.1fKB)", double(debug_info/10));
+  } else if (debug_info < 1024*1024*1024) {
+debug_info = debug_info*10/(1024*1024);
+sprintf(debug_info_size, " (%.1fMB)", double(debug_info/10));
+  } else {
+debug_info = debug_info*10/(1024*1024*1024);
+sprintf(debug_info_size, " (%.1fGB)", double(debug_info/10));
+  }
+  symbol_str = symbol_str + debug_info_size;
+}
+object.try_emplace("symbolStatus", symbol_str);
 char symbol_path_arr[PATH_MAX];
 module.GetSymbolFileSpec().GetPath(symbol_path_arr, sizeof(symbo

[Lldb-commits] [PATCH] D83731: Add Debug Info Size to Symbol Status

[walter erquinigo via Phabricator via lldb-commits]

wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.

Please include a test case




Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:351-362
+  char debug_info_size[10];
+  if (debug_info < 1024) {
+sprintf(debug_info_size, " (%lluKB)", debug_info);
+  } else if (debug_info < 1024*1024) {
+debug_info = double(debug_info*10/1024);
+sprintf(debug_info_size, " (%.1fKB)", double(debug_info/10));
+  } else if (debug_info < 1024*1024*1024) {

Move this logic to another function, so that this function is simpler



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:356
+debug_info = double(debug_info*10/1024);
+sprintf(debug_info_size, " (%.1fKB)", double(debug_info/10));
+  } else if (debug_info < 1024*1024*1024) {

debug_info is int, thus, if you do `debug_info/10`, then the result with be an 
int rounded down. If you cast it to double, you'll have a double without 
decimals. The correct way to do is to do `double(debug_info) / 10`. 



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:390
 
+uint64_t DebugInfoInSection(lldb::SBSection section) {
+  uint64_t section_size = 0;

This is a method, therefore it should start with a verb


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731



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

[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

[Benson Li via Phabricator via lldb-commits]

bbli added a comment.

Hi Pavel,

When I change the `stderr` argument to point to `STDOUT` inside the `Popen` 
function, which is inside the `lldbtest.system` function, the output actually 
gets mixed up. Whereas currently,, after running a couple times, the contents 
of stderr always gets printed out first and then stdout. In any case, should I 
just make two headers then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83425



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

[Lldb-commits] [lldb] 869d05f - [lldb-vscode] Fix TestVSCode_module

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-07-13T18:02:37-07:00
New Revision: 869d05fb3e449ec7ec835b8a61687f8df41b8651

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

LOG: [lldb-vscode] Fix TestVSCode_module
This test was added in https://reviews.llvm.org/D82477 and needs to wait a 
little bit before fetching some information.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index 676e08d5a38c..c1b33c220b4b 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -2,6 +2,7 @@
 from lldbsuite.test.lldbtest import *
 import os
 import vscode
+import time
 
 
 class VSCodeTestCaseBase(TestBase):
@@ -52,6 +53,12 @@ def set_function_breakpoints(self, functions, condition=None,
 breakpoint_ids.append('%i' % (breakpoint['id']))
 return breakpoint_ids
 
+def waitUntil(self, condition):
+while True:
+if condition():
+break
+time.sleep(0.5)
+
 def verify_breakpoint_hit(self, breakpoint_ids):
 '''Wait for the process we are debugging to stop, and verify we hit
any breakpoint location in the "breakpoint_ids" array.

diff  --git a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py 
b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
index 461ac201a73f..40c4145b38e3 100644
--- a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
+++ b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -11,7 +11,6 @@
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
 
-
 class TestVSCode_module(lldbvscode_testcase.VSCodeTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
@@ -40,6 +39,13 @@ def test_modules_event(self):
 self.assertEqual('Symbols not found.', program_module['symbolStatus'])
 symbol_path = self.getBuildArtifact("a.out")
 self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" 
"%s"' % (program, symbol_path)))
+
+def checkSymbolsLoaded():
+active_modules = self.vscode.get_active_modules()
+program_module = active_modules[program_basename]
+return 'Symbols loaded.' == program_module['symbolStatus']
+self.waitUntil(checkSymbolsLoaded)
+
 active_modules = self.vscode.get_active_modules()
 program_module = active_modules[program_basename]
 self.assertEqual(program_basename, program_module['name'])
@@ -63,7 +69,6 @@ def test_compile_units(self):
 self.continue_to_breakpoints(breakpoint_ids)
 moduleId = self.vscode.get_active_modules()['a.out']['id']
 response = self.vscode.request_getCompileUnits(moduleId)
-print(response['body'])
 self.assertTrue(response['body'])
 self.assertTrue(len(response['body']['compileUnits']) == 1,
 'Only one source file should exist')



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

[Lldb-commits] [lldb] 9a9ae01 - [lldb-vscode] Fix TestVSCode_setBreakpoints

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-07-13T18:27:53-07:00
New Revision: 9a9ae01f994a92900bc703cf8a512082f49ce45d

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

LOG: [lldb-vscode] Fix TestVSCode_setBreakpoints

It was failing because some module events had empty UUID, and that was not 
handled correctly.
The diff that added that logic is https://reviews.llvm.org/D82477

Added: 


Modified: 
lldb/tools/lldb-vscode/JSONUtils.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/JSONUtils.cpp 
b/lldb/tools/lldb-vscode/JSONUtils.cpp
index 86c29fb23811..1ebaa5c37712 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -331,7 +331,8 @@ llvm::json::Value CreateModule(lldb::SBModule &module) {
   llvm::json::Object object;
   if (!module.IsValid())
 return llvm::json::Value(std::move(object));
-  object.try_emplace("id", std::string(module.GetUUIDString()));
+  const char *uuid = module.GetUUIDString();
+  object.try_emplace("id", uuid ? std::string(uuid) : std::string(""));
   object.try_emplace("name", std::string(module.GetFileSpec().GetFilename()));
   char module_path_arr[PATH_MAX];
   module.GetFileSpec().GetPath(module_path_arr, sizeof(module_path_arr));



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

[Lldb-commits] [PATCH] D83512: [lldb/Module] Allow for the creation of memory-only modules

[Frederic Riss via Phabricator via lldb-commits]

friss updated this revision to Diff 277657.
friss added a comment.
Herald added a subscriber: mgorny.

I went ahead and updated all unittests to use only in-memory modules.
This required tweaking both ObjectFileELF and ObjectFilePECOFF a little.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83512

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Core/ModuleSpec.h
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/Utility/DataBuffer.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Symbol/ObjectFile.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/MangledTest.cpp
  lldb/unittests/Core/ModuleSpecTest.cpp
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
  lldb/unittests/ObjectFile/PECOFF/TestPECallFrameInfo.cpp
  lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp
  lldb/unittests/Symbol/TestLineEntry.cpp
  lldb/unittests/TestingSupport/TestUtilities.cpp
  lldb/unittests/TestingSupport/TestUtilities.h

Index: lldb/unittests/TestingSupport/TestUtilities.h
===
--- lldb/unittests/TestingSupport/TestUtilities.h
+++ lldb/unittests/TestingSupport/TestUtilities.h
@@ -9,6 +9,8 @@
 #ifndef LLDB_UNITTESTS_TESTINGSUPPORT_TESTUTILITIES_H
 #define LLDB_UNITTESTS_TESTINGSUPPORT_TESTUTILITIES_H
 
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Utility/DataBuffer.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Error.h"
@@ -34,22 +36,28 @@
   static llvm::Expected fromYaml(llvm::StringRef Yaml);
   static llvm::Expected fromYamlFile(const llvm::Twine &Name);
 
-  TestFile(TestFile &&RHS) : Name(std::move(RHS.Name)) {
-RHS.Name = llvm::None;
+  TestFile(TestFile &&RHS) : Buffer(std::move(RHS.Buffer)) {
+RHS.Buffer = llvm::None;
   }
 
-  ~TestFile();
+  ~TestFile() = default;
 
-  llvm::StringRef name() { return *Name; }
+  ModuleSpec moduleSpec() {
+return ModuleSpec(FileSpec(), UUID(), dataBuffer());
+  }
 
 private:
-  TestFile(llvm::StringRef Name, llvm::FileRemover &&Remover)
-  : Name(std::string(Name)) {
-Remover.releaseFile();
-  }
+  TestFile(std::string &&Buffer) : Buffer(Buffer) {}
+
   void operator=(const TestFile &) = delete;
 
-  llvm::Optional Name;
+  lldb::DataBufferSP dataBuffer() {
+auto *buffer = reinterpret_cast(Buffer->data());
+return std::make_shared(const_cast(buffer),
+   Buffer->size());
+  }
+
+  llvm::Optional Buffer;
 };
 }
 
Index: lldb/unittests/TestingSupport/TestUtilities.cpp
===
--- lldb/unittests/TestingSupport/TestUtilities.cpp
+++ lldb/unittests/TestingSupport/TestUtilities.cpp
@@ -29,21 +29,14 @@
 llvm::Expected TestFile::fromYaml(llvm::StringRef Yaml) {
   const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
   assert(Info);
-  llvm::SmallString<128> Name;
-  int FD;
-  if (std::error_code EC = llvm::sys::fs::createTemporaryFile(
-  llvm::Twine(Info->test_case_name()) + "-" + Info->name(), "test", FD,
-  Name))
-return llvm::errorCodeToError(EC);
-  llvm::FileRemover Remover(Name);
-  {
-llvm::raw_fd_ostream OS(FD, /*shouldClose*/ true);
-llvm::yaml::Input YIn(Yaml);
-if (!llvm::yaml::convertYAML(YIn, OS, [](const llvm::Twine &Msg) {}))
-  return llvm::createStringError(llvm::inconvertibleErrorCode(),
- "convertYAML() failed");
-  }
-  return TestFile(Name, std::move(Remover));
+
+  std::string Buffer;
+  llvm::raw_string_ostream OS(Buffer);
+  llvm::yaml::Input YIn(Yaml);
+  if (!llvm::yaml::convertYAML(YIn, OS, [](const llvm::Twine &Msg) {}))
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "convertYAML() failed");
+  return TestFile(std::move(Buffer));
 }
 
 llvm::Expected TestFile::fromYamlFile(const llvm::Twine &Name) {
@@ -55,11 +48,3 @@
   return fromYaml(BufferOrError.get()->getBuffer());
 }
 
-TestFile::~TestFile() {
-  if (!Name)
-return;
-  if (std::error_code EC =
-  llvm::sys::fs::remove(*Name, /*IgnoreNonExisting*/ false))
-GTEST_LOG_(WARNING) << "Failed to delete `" << Name->c_str()
-<< "`: " << EC.message();
-}
Index: lldb/unittests/Symbol/TestLineEntry.cpp
===
--- lldb/unittests/Symbol/TestLineEntry.cpp
+++ lldb/unittests/Symbol/TestLineEntry.cpp
@@ -49,7 +49,7 @@
   auto ExpectedFile = TestFile::fromYamlFile("inlined-functions.yaml");
   ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
   m_file.emplace(std::move(*ExpectedFile));
-  m_module_sp = std::make_shared(ModuleSpec(FileSpec(m_file->name(;
+  m_module_sp = std::make_shared(m_file->

[Lldb-commits] [PATCH] D83512: [lldb/Module] Allow for the creation of memory-only modules

[Frederic Riss via Phabricator via lldb-commits]

friss marked 2 inline comments as done.
friss added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:543-544
 
+  if (m_data.ValidOffsetForDataOfSize(offset, size))
+return DataExtractor(m_data, offset, size);
+

This seems to work in the test, but I have to admit that I'm not 100% sure it's 
correct given the comment below about wanting to write this buffer.



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:565
 DataExtractor ObjectFilePECOFF::ReadImageDataByRVA(uint32_t rva, size_t size) {
-  if (m_file) {
-Address addr = GetAddress(rva);

I'm not sure why `m_file` was tested here, but this doesn't work with a pure 
in-memory file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83512



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

[Lldb-commits] [PATCH] D83731: Add Debug Info Size to Symbol Status

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.

See inlined comments. You will also need to fix the test for this since it will 
now fail as the "symbolStatus" now contains the size.




Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:342-349
+uint64_t debug_info = 0;
+size_t num_sections = module.GetNumSections();
+if (num_sections > 0) {
+  for (int i = 0; i < (int)num_sections; i++) {
+lldb::SBSection section = module.GetSectionAtIndex(i);
+debug_info += DebugInfoInSection(section);
+  }

Move these lines to a static function above this function:

```
static uint64_t GetDebugInfoSize(lldb::SBModule module) {
  uint64_t debug_info = 0;
  size_t num_sections = module.GetNumSections();
  for (int i = 0; i < (int)num_sections; i++) {
lldb::SBSection section = module.GetSectionAtIndex(i);
debug_info += DebugInfoInSection(section);
  }
  return debug_info;
}
```



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:351
+if (debug_info > 0) {
+  char debug_info_size[10];
+  if (debug_info < 1024) {

wallace wrote:
> Move this logic to another function, so that this function is simpler
increase size to 32 just in case we get really big debug info later..



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:353
+  if (debug_info < 1024) {
+sprintf(debug_info_size, " (%lluKB)", debug_info);
+  } else if (debug_info < 1024*1024) {

Use the PRIu64 macro here to make sure this works on all platforms and the 
units are wrong here, should just be "B" instead of "KB". Also use snprintf for 
safety:
```
snprintf(debug_info_size, sizeof(debug_info_size), " (%"  PRIu64 "B)", 
debug_info);
```
PRIu64 is a macro that will expand to a string that will always match a 
uint64_t. The three strings (" (%", PRIu64, and "B)") will get combined by the 
compiler into a single format string. 



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:355
+  } else if (debug_info < 1024*1024) {
+debug_info = double(debug_info*10/1024);
+sprintf(debug_info_size, " (%.1fKB)", double(debug_info/10));

no need to multiply the debug_info by 10 here. You will want to make a local 
double so we don't convert back to an integer:

```
double kb = (double)debug_info/1024.0;
```



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:356
+debug_info = double(debug_info*10/1024);
+sprintf(debug_info_size, " (%.1fKB)", double(debug_info/10));
+  } else if (debug_info < 1024*1024*1024) {

wallace wrote:
> debug_info is int, thus, if you do `debug_info/10`, then the result with be 
> an int rounded down. If you cast it to double, you'll have a double without 
> decimals. The correct way to do is to do `double(debug_info) / 10`. 
Just use the local double above "kb" and use snprintf:
```
snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fKB)", kb);
```




Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:358
+  } else if (debug_info < 1024*1024*1024) {
+debug_info = debug_info*10/(1024*1024);
+sprintf(debug_info_size, " (%.1fMB)", double(debug_info/10));

Don't multiply by 10 and use a local double:
```
double mb = (double)debug_info/(1024.0*1024.0);
```



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:359
+debug_info = debug_info*10/(1024*1024);
+sprintf(debug_info_size, " (%.1fMB)", double(debug_info/10));
+  } else {

Use local double and use snprintf:
```
snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fMB)", mb);
```




Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:361
+  } else {
+debug_info = debug_info*10/(1024*1024*1024);
+sprintf(debug_info_size, " (%.1fGB)", double(debug_info/10));

```
double gb = (double)debug_info/(1024.0*1024.0*1024.0);
```




Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:362
+debug_info = debug_info*10/(1024*1024*1024);
+sprintf(debug_info_size, " (%.1fGB)", double(debug_info/10));
+  }

Use local and snprintf:
```
snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fGB)", gb);
```



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:390
 
+uint64_t DebugInfoInSection(lldb::SBSection section) {
+  uint64_t section_size = 0;

wallace wrote:
> This is a method, therefore it should start with a verb
Rename this as Walter commented and make this function static. You will need to 
move it above the function above otherwise the compiler won't see it:

```
static uint64_t GetDebugInfoSize(lldb::SBSection section) {
```



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:391
+uint64_t DebugInfoInSection(lldb::SBSection section) {
+  uint64_t section_size = 0;
+  size