[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D86667#2243781 , @mib wrote:

> In D86667#2242566 , @jingham wrote:
>
>> Do people really call command-line shells interpreters?  I would have 
>> thought --shell would be a better name.  lldb already has its own command 
>> interpreter which is orthogonal to the shells, so it seems confusing to use 
>> the same term for both.
>
> I think `platform shell --shell` sounds/looks repetitive so I opted for 
> `-i|--interpreter` instead, as in Command-line **Interpreter**.

Yes, it is repetitive. OTOH:

- the unix commands that take a shell as an argument (e.g., `su`, `sudo`), do 
so via a --shell/-s argument
- we have a `memory load --load` command

Overall, I can understand where you're coming from, but I think I'd go with 
--shell nonetheless...




Comment at: lldb/include/lldb/Target/Platform.h:643
+  *command_output, // Pass nullptr if you don't want the command output
+  const Timeout &timeout, bool run_in_shell = true);
 

The run_in_shell argument makes the inferface weird (why do I need to specify a 
shell, if I don't want to use it). And still suffers from the virtual default 
argument problem described above. Let's not propagate this Host weirdness into 
the platform class.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2820
 lldb_private::Status GDBRemoteCommunicationClient::RunShellCommand(
-const char *command, // Shouldn't be NULL
+llvm::StringRef command, // Shouldn't be NULL
 const FileSpec &

These `Shouldn't be NULL` are out of place now, and probably not needed anymore.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2831
   stream.PutCString("qPlatform_shell:");
-  stream.PutBytesAsRawHex8(command, strlen(command));
+  stream.PutBytesAsRawHex8(command.str().c_str(), command.size());
   stream.PutChar(',');

`command.data()` will do just fine



Comment at: lldb/test/API/commands/platform/basic/TestPlatformPython.py:95
+executable = self.getBuildArtifact('myshell')
+d = {'C_SOURCES': 'myshell.c', 'EXE': executable}
+self.build(dictionary=d)

We normally set these things in the makefile. The environment tricks are 
reserved for cases where you need to do something funny (e.g. build multiple 
things from a single makefile, or similar...).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86667

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

[Lldb-commits] [PATCH] D86417: [lldb] do not propagate eTrapHandlerFrame repeatedly

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Hi, thanks so much for showing all the unwind plans, sorry for dropping off the 
thread for a day.

I'm seeing something I very much don't expect, and it may be important.  The 
unwind information for __restore_rt is marked as a trap handler in both the 
eh_frame unwind information and from the PlatformLinux, and the unwind rules 
look like a trap handler.

However, I see that both raise and abort have eh_frame UnwindPlans saying that 
they are not trap handlers and eh_frame augmented UnwindPlan saying they *are*. 
 I think we have a little buggo where the augmented UnwindPlan is created and 
we have an uninitialized ivar or something.  I'll look at this code tomorrow, I 
bet it's an obvious bug.

I need to re-read the original bug description, but this will have some minor 
misbehaviors especially after a no-return function like abort() where the next 
instruction may be an entirely wrong symbol context if we treat abort as a trap 
handler.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D86417

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

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D86667#2243878 , @labath wrote:

> In D86667#2243781 , @mib wrote:
>
>> In D86667#2242566 , @jingham wrote:
>>
>>> Do people really call command-line shells interpreters?  I would have 
>>> thought --shell would be a better name.  lldb already has its own command 
>>> interpreter which is orthogonal to the shells, so it seems confusing to use 
>>> the same term for both.
>>
>> I think `platform shell --shell` sounds/looks repetitive so I opted for 
>> `-i|--interpreter` instead, as in Command-line **Interpreter**.
>
> Yes, it is repetitive. OTOH:
>
> - the unix commands that take a shell as an argument (e.g., `su`, `sudo`), do 
> so via a --shell/-s argument
> - we have a `memory load --load` command
>
> Overall, I can understand where you're coming from, but I think I'd go with 
> --shell nonetheless...

I'd agree, process launch already takes a -c|--shell argument so it would be 
consistent to do that.  (I assume -c was used because it's kind of like sh(1)'s 
-c option but not really.  maybe more likely that -s was already used ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86667

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

[Lldb-commits] [PATCH] D86745: [lldb/test] Use @skipIfWindows for PExpectTest

[Pavel Labath via Phabricator via lldb-commits]

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

This looks like it could work, though only barely. `@skipIfWindows` checks for 
target system, but we would really need this to check the host OS (for remote 
testing, although I don't know if anyone does that on windows these days). 
However, in conjuction with `@skipIfRemote` it should do the right thing. In 
any case, this is small enough a change to try it out. Just be sure to check 
the windows bot afterwards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86745

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

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759
+  QualType FromTy = ArrayFrom->getElementType();
+  QualType ToTy = ArrayTo->getElementType();
+
+  FromRecordDecl = FromTy->getAsRecordDecl();
+  ToRecordDecl = ToTy->getAsRecordDecl();

shafik wrote:
> martong wrote:
> > labath wrote:
> > > What about 2- or n-dimensional arrays?
> > @labath, this is a very good question! And made me realize that D71378 is 
> > fundamentally flawed (@shafik, please take no offense). Let me explain.
> > 
> > So, with D71378, we added the `if (ImportedOrErr) { ... }` block to import 
> > definitions specifically of fields' Record types. But we forget to handle 
> > arrays. Now we may forget to handle multidimensional arrays ... and we may 
> > forget to handle other language constructs. So, we would finally end up in 
> > re-implementing the logic of `ASTNodeImporter::VisitFieldDecl`.
> > 
> > So all this should have been handled properly by the preceding import call 
> > of the FieldDecl! Here
> > ```
> > 1735: ExpectedDecl ImportedOrErr = import(From);
> > ```
> > I have a suspicion that real reason why this import call fails in case of 
> > the public ASTImporter::ImportDefinition() is that we fail to drive through 
> > the import kind (`IDK_Everything`) during the import process.
> > Below we set IDK_Everything and start a complete import process.
> > ```
> >   8784   if (auto *ToRecord = dyn_cast(To)) {
> >   8785 if (!ToRecord->getDefinition()) {
> >   8786   return Importer.ImportDefinition(   // 
> > ASTNodeImporter::ImportDefinition !
> >   8787   cast(FromDC), ToRecord,
> >   8788   ASTNodeImporter::IDK_Everything);
> >   8789 }
> >   8790   }
> > ```
> > However, there may be many places where we fail to channel through that we 
> > want to do a complete import. E.g here
> > ```
> > 1957   
> > ImportDefinitionIfNeeded(Base1.getType()->getAsCXXRecordDecl()))
> > ```
> > we do another definition import and IDK_Everything is not set. So we may 
> > have a wrong kind of import since the "minimal" flag is set.
> > 
> > The thing is, it is really confusing and error-prone to have both the 
> > `ASTImporter::Minimal` flag and the `ImportDefinitionKind`. They seem to be 
> > in contradiction to each other some times.
> > I think we should get rid of the Minimal flag. And Kind should be either a 
> > full completion (IDK_Everythink) or just a minimal (IDK_Basic). So, I'd 
> > scrap the IDK_Default too. Alternatively we could have a Kind member in 
> > AST**//Node//**Importer.
> > I think we should go into this direction to avoid similar problems during 
> > CodeGen in the future. WDYT?
> No offense, you definitely understand the Importer better than I, so I value 
> your input here. I don't believe that should have other fields where we could 
> have a record that effects the layout but the concern is still valid and yes 
> I did miss multi-dimensional arrays.
> 
> Your theory on `IDK_Everything` not be pushed through everywhere, I did a 
> quick look and it seems pretty reasonable. 
> 
> I agree that the `ASTImporter::Minimal` flag and the `ImportDefinitionKind` 
> seem to inconsistent or perhaps a work-around. That seems like a bigger 
> change with a lot more impact beyond this fix but worth looking into longer 
> term. 
> 
> If pushing through `IDK_Everything` everywhere fixes the underlying issue I 
> am very happy to take that approach. If not we can discuss alternatives. 
I've been looking at this code, but I'm still not very familiar with it, so 
what I am asking may be obvious, but... What is the expected behavior for 
non-minimal imports for code like this?
```
struct A { ...};
struct B { A* a; }; // Note the pointer
```
Should importing B also import the definition of the A struct ? As I think that 
should not happen in the minimal import... If we get rid of the minimal flag, 
and rely solely on argument passing, we will need to be careful, as it 
shouldn't be passed _everywhere_ (it should stop at pointers for instance). But 
then it may not be possible to reproduce the current non-minimal import, as it 
(I think) expects that A will be fully imported too...

> I don't believe that should have other fields where we could have a record 
> that effects the layout
This isn't exactly layout related, but there is the question of covariant 
methods. If a method is covariant, then its return type must be complete. 
Currently we handle the completion of these in LLDB, but that solution is a bit 
hacky. It might be interesting if that could be handled by the ast importer as 
well.



Comment at: 
lldb/test/API/commands/expression/codegen-crash-import-def-arraytype-element/main.cpp:22-27
+  union {
+struct {
+  unsigned char *_s;
+} t;
+char *tt[1];
+  } U;

shafik wrote:
> labath wrote:
> > What's the significance of this union?
> Not sur

[Lldb-commits] [PATCH] D86652: [lldb] Fix ProcessEventData::DoOnRemoval to support multiple listeners

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This is going to derail this patch somewhat (sorry), but since this is an 
important long-standing open issue, I think we should discuss it in more detail.

The part that's bothering me about this for quite some time is... Why do we 
even have this DoOnRemoval stuff in the first place?

I mean instead of doing this work in `DoOnRemoval`, why don't we do this work 
/before/ enqueueing the relevant event. Then the event would be of purely 
informative character and it would not matter if its received by one, two, ten, 
or zero listeners. Of course, if a user wanted to program lldb in a correct and 
race-free manner, it would /have to/ listen to these events (as otherwise it 
would not know when is it safe to run some actions), but that would be 
something that's entirely up to him.

Back when I was debugging some races in tests with asynchronous listeners, I 
remember it was pretty hard to reason about things and prove they are correct, 
since the dequeueing of the event (and the resulting flurry of actions within 
lldb) could come at any moment. If those actions were done internally to lldb, 
they would be more sequenced more deterministically and be easier to reason 
about than if they are executed whenever the user chooses to dequeue an event. 
(I remember things got particularly messy when a test finished before dequeuing 
an event, at which point the processing of the event was executing concurrently 
with the `process.Kill()` statement we execute at the end of each test.)

In D86652#2242770 , @ilya-nozhkin 
wrote:

> One of the possible solutions is to send a specifically marked event to a 
> primary listener first. Then, make this event to invoke a handshake in its 
> destructor (which would mean that primary listener doesn't need the event 
> anymore). And handshake can be implemented as just a rebroadcasting a new 
> instance of the event to other listeners (or via unlocking some additional 
> mutex / notifying some conditional variable). However, destructor may be 
> invoked not immediately after event processing is finished. For example, if a 
> primary listener's thread defines an EventSP before the listening cycle and 
> doesn't reset it until GetEvent returns a new one (which may not happen until 
> the next stop, for example).

This primary listener technique could be implemented purely in your own code, 
could it not? You could have one listener, which listens to lldb events, and 
then rebroadcasts it (using arbitrary techniques, not necessarily SBListener) 
to any number of interested threads that want to act on it. That might make it 
easier to ensure there are no races in your code, and would avoid stepping on 
some of the internal lldb races that I am sure we have... Otherwise I (as Jim) 
expect that you will still need to have some sort of communication between the 
various listeners listening to process events, as they'd need to coordinate

> Anyway, the concept of primary and secondary listeners requires to forbid 
> secondary listeners to do "step" or "continue" at all. So, why can't we do 
> the same without any additional code? I mean, I think that the most common 
> case of multiple listeners is a Python extension adding a listener to 
> read/write some memory/registers on each stop taking the RunLock.

Yes, you could take the run lock, which would guarantee that the other 
listeners don't get to resume the process. But how do you guarantee that you 
_can_ take the run lock  -- some other thread (the primary listener) could beat 
you to it resume the process, so you'd miss the chance to read/write those 
registers. Seems very fragile. A stop hook does look like a better way to 
achieve these things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86652

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

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D85705#2243536 , @wallace wrote:

> - I'm still using StructuredData for the parsing. There's a chance that once 
> we release this feature users will want support for other formats besides 
> JSON, so for now I prefer to ask for JSON input but parse in a 
> format-agnostic way. If eventually no one needs any other format, we can 
> switch to JSON-only parsing.

That argument goes both ways. We could say that if anyone wants to add a 
non-json format, he can change the code to accept his preferred format. 
Overall, I am very sceptical of the StructuredData's ability to abstract 
diverse formats in a meaningful way. I doubt that a natural XML representation 
of this data would map to the same abstract format as this json and that it 
could be parsed by the same code -- simply because xml conventions are 
different, and xml has features that json doesn't (attributes, for one).

And since this file is just a descriptor, and actual plugin-specific data is 
contained in other files, I don't see why it's format couldn't be fixed.




Comment at: lldb/test/API/commands/trace/intelpt-trace/trace.json:11
+  },
+  "triple": "x86_64-*-linux",
+  "processes": [

What if one of the processes is 64-bit and the other 32? It sounds like this 
should be a property of a particular process.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705

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

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think it would be appropriate to discuss the design of this feature on 
lldb-dev before going over the individual patches. One of the fundamental 
aspects of this patchset that I think should not be overlooked is that it 
essentially adds a new level of structure ( a "Target Group") lldb. One of the 
questions I'd have is whether this concept shouldn't be formalized somewhere 
instead of it existing just virtually as the group of threads that happen to 
share the same trace object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86670

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

[Lldb-commits] [lldb] 1f9595e - [lldb] Reduce intentation in SymbolFileDWARF::ParseVariableDIE

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-08-28T11:44:03+02:00
New Revision: 1f9595ede48d694b8506d22fd71f7ce13e00dafa

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

LOG: [lldb] Reduce intentation in SymbolFileDWARF::ParseVariableDIE

using early exits. NFC.

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 271821b24517..9f4556f791ae 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3091,363 +3091,350 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const 
SymbolContext &sc,
   if (die.GetDWARF() != this)
 return die.GetDWARF()->ParseVariableDIE(sc, die, func_low_pc);
 
-  VariableSP var_sp;
   if (!die)
-return var_sp;
+return nullptr;
 
-  var_sp = GetDIEToVariable()[die.GetDIE()];
-  if (var_sp)
+  if (VariableSP var_sp = GetDIEToVariable()[die.GetDIE()])
 return var_sp; // Already been parsed!
 
   const dw_tag_t tag = die.Tag();
   ModuleSP module = GetObjectFile()->GetModule();
 
-  if ((tag == DW_TAG_variable) || (tag == DW_TAG_constant) ||
-  (tag == DW_TAG_formal_parameter && sc.function)) {
-DWARFAttributes attributes;
-const size_t num_attributes = die.GetAttributes(attributes);
-DWARFDIE spec_die;
-if (num_attributes > 0) {
-  const char *name = nullptr;
-  const char *mangled = nullptr;
-  Declaration decl;
-  DWARFFormValue type_die_form;
-  DWARFExpression location;
-  bool is_external = false;
-  bool is_artificial = false;
-  DWARFFormValue const_value_form, location_form;
-  Variable::RangeList scope_ranges;
-  // AccessType accessibility = eAccessNone;
-
-  for (size_t i = 0; i < num_attributes; ++i) {
-dw_attr_t attr = attributes.AttributeAtIndex(i);
-DWARFFormValue form_value;
-
-if (attributes.ExtractFormValueAtIndex(i, form_value)) {
-  switch (attr) {
-  case DW_AT_decl_file:
-decl.SetFile(sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-form_value.Unsigned()));
-break;
-  case DW_AT_decl_line:
-decl.SetLine(form_value.Unsigned());
-break;
-  case DW_AT_decl_column:
-decl.SetColumn(form_value.Unsigned());
-break;
-  case DW_AT_name:
-name = form_value.AsCString();
-break;
-  case DW_AT_linkage_name:
-  case DW_AT_MIPS_linkage_name:
-mangled = form_value.AsCString();
-break;
-  case DW_AT_type:
-type_die_form = form_value;
-break;
-  case DW_AT_external:
-is_external = form_value.Boolean();
-break;
-  case DW_AT_const_value:
-const_value_form = form_value;
-break;
-  case DW_AT_location:
-location_form = form_value;
-break;
-  case DW_AT_specification:
-spec_die = form_value.Reference();
-break;
-  case DW_AT_start_scope:
-// TODO: Implement this.
-break;
-  case DW_AT_artificial:
-is_artificial = form_value.Boolean();
-break;
-  case DW_AT_accessibility:
-break; // accessibility =
-   // DW_ACCESS_to_AccessType(form_value.Unsigned()); break;
-  case DW_AT_declaration:
-  case DW_AT_description:
-  case DW_AT_endianity:
-  case DW_AT_segment:
-  case DW_AT_visibility:
-  default:
-  case DW_AT_abstract_origin:
-  case DW_AT_sibling:
-break;
-  }
-}
-  }
+  if (tag != DW_TAG_variable && tag != DW_TAG_constant &&
+  (tag != DW_TAG_formal_parameter || !sc.function))
+return nullptr;
 
-  // Prefer DW_AT_location over DW_AT_const_value. Both can be emitted e.g.
-  // for static constexpr member variables -- DW_AT_const_value will be
-  // present in the class declaration and DW_AT_location in the DIE 
defining
-  // the member.
-  bool location_is_const_value_data = false;
-  bool has_explicit_location = false;
-  bool use_type_size_for_value = false;
-  if (location_form.IsValid()) {
-has_explicit_location = true;
-if (DWARFFormValue::IsBlockForm(location_form.Form())) {
-  const DWARFDataExtractor &data = die.GetData();
-
-  uint32_t block_offset =
-  location_form.BlockData() - data.GetDataStart();
-  uint32_t block_length = location_form.Unsigned();
-  locatio

[Lldb-commits] [lldb] 9b50546 - [lldb/Utility] Polish the Scalar class

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-08-28T11:51:25+02:00
New Revision: 9b50546b0b4087ec2ee8479d68e70b89c7956aaa

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

LOG: [lldb/Utility] Polish the Scalar class

This patch is mostly about removing the "Category" enum, which was
very useful when the Type enum contained a large number of types, but
now the two are completely identical.

It also removes some other artifacts like unused typedefs and macros.

Added: 


Modified: 
lldb/include/lldb/Utility/Scalar.h
lldb/source/Utility/Scalar.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Scalar.h 
b/lldb/include/lldb/Utility/Scalar.h
index 332bb00489d0..7403fd1d9717 100644
--- a/lldb/include/lldb/Utility/Scalar.h
+++ b/lldb/include/lldb/Utility/Scalar.h
@@ -20,18 +20,12 @@
 #include 
 
 namespace lldb_private {
+
 class DataExtractor;
 class Stream;
-} // namespace lldb_private
 
 #define NUM_OF_WORDS_INT128 2
 #define BITWIDTH_INT128 128
-#define NUM_OF_WORDS_INT256 4
-#define BITWIDTH_INT256 256
-#define NUM_OF_WORDS_INT512 8
-#define BITWIDTH_INT512 512
-
-namespace lldb_private {
 
 // A class designed to hold onto values and their corresponding types.
 // Operators are defined and Scalar objects will correctly promote their types
@@ -48,7 +42,6 @@ class Scalar {
   }
 
 public:
-  // FIXME: These are host types which seems to be an odd choice.
   enum Type {
 e_void = 0,
 e_int,
@@ -219,21 +212,6 @@ class Scalar {
   }
 
 protected:
-  typedef char schar_t;
-  typedef unsigned char uchar_t;
-  typedef short sshort_t;
-  typedef unsigned short ushort_t;
-  typedef int sint_t;
-  typedef unsigned int uint_t;
-  typedef long slong_t;
-  typedef unsigned long ulong_t;
-  typedef long long slonglong_t;
-  typedef unsigned long long ulonglong_t;
-  typedef float float_t;
-  typedef double double_t;
-  typedef long double long_double_t;
-
-  // Classes that inherit from Scalar can see and modify these
   Scalar::Type m_type;
   llvm::APSInt m_integer;
   llvm::APFloat m_float;
@@ -242,10 +220,7 @@ class Scalar {
 
   static Type PromoteToMaxType(Scalar &lhs, Scalar &rhs);
 
-  enum class Category { Void, Integral, Float };
-  static Category GetCategory(Scalar::Type type);
-
-  using PromotionKey = std::tuple;
+  using PromotionKey = std::tuple;
   PromotionKey GetPromoKey() const;
 
   static PromotionKey GetFloatPromoKey(const llvm::fltSemantics &semantics);

diff  --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 3dd2813b5eb0..9bf633d0c4e0 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -27,26 +27,13 @@ using llvm::APFloat;
 using llvm::APInt;
 using llvm::APSInt;
 
-Scalar::Category Scalar::GetCategory(Scalar::Type type) {
-  switch (type) {
-  case Scalar::e_void:
-return Category::Void;
-  case Scalar::e_float:
-return Category::Float;
-  case Scalar::e_int:
-return Category::Integral;
-  }
-  llvm_unreachable("Unhandled type!");
-}
-
 Scalar::PromotionKey Scalar::GetPromoKey() const {
-  Category cat = GetCategory(m_type);
-  switch (cat) {
-  case Category::Void:
-return PromotionKey{cat, 0, false};
-  case Category::Integral:
-return PromotionKey{cat, m_integer.getBitWidth(), m_integer.isUnsigned()};
-  case Category::Float:
+  switch (m_type) {
+  case e_void:
+return PromotionKey{e_void, 0, false};
+  case e_int:
+return PromotionKey{e_int, m_integer.getBitWidth(), 
m_integer.isUnsigned()};
+  case e_float:
 return GetFloatPromoKey(m_float.getSemantics());
   }
   llvm_unreachable("Unhandled category!");
@@ -58,7 +45,7 @@ Scalar::PromotionKey Scalar::GetFloatPromoKey(const 
llvm::fltSemantics &sem) {
   &APFloat::x87DoubleExtended()};
   for (const auto &entry : llvm::enumerate(order)) {
 if (entry.value() == &sem)
-  return PromotionKey{Category::Float, entry.index(), false};
+  return PromotionKey{e_float, entry.index(), false};
   }
   llvm_unreachable("Unsupported semantics!");
 }
@@ -67,13 +54,13 @@ Scalar::PromotionKey Scalar::GetFloatPromoKey(const 
llvm::fltSemantics &sem) {
 // expressions.
 Scalar::Type Scalar::PromoteToMaxType(Scalar &lhs, Scalar &rhs) {
   const auto &Promote = [](Scalar &a, const Scalar &b) {
-switch (GetCategory(b.GetType())) {
-case Category::Void:
+switch (b.GetType()) {
+case e_void:
   break;
-case Category::Integral:
+case e_int:
   a.IntegralPromote(b.m_integer.getBitWidth(), b.m_integer.isSigned());
   break;
-case Category::Float:
+case e_float:
   a.FloatPromote(b.m_float.getSemantics());
 }
   };
@@ -129,13 +116,13 @@ void Scalar::GetBytes(llvm::MutableArrayRef 
storage) const {
   const auto &store = [&](const llvm::APInt &val) {
 StoreIntToMemory(val, s

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

[Gabor Marton via Phabricator via lldb-commits]

martong added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:1755-1759
+  QualType FromTy = ArrayFrom->getElementType();
+  QualType ToTy = ArrayTo->getElementType();
+
+  FromRecordDecl = FromTy->getAsRecordDecl();
+  ToRecordDecl = ToTy->getAsRecordDecl();

labath wrote:
> shafik wrote:
> > martong wrote:
> > > labath wrote:
> > > > What about 2- or n-dimensional arrays?
> > > @labath, this is a very good question! And made me realize that D71378 is 
> > > fundamentally flawed (@shafik, please take no offense). Let me explain.
> > > 
> > > So, with D71378, we added the `if (ImportedOrErr) { ... }` block to 
> > > import definitions specifically of fields' Record types. But we forget to 
> > > handle arrays. Now we may forget to handle multidimensional arrays ... 
> > > and we may forget to handle other language constructs. So, we would 
> > > finally end up in re-implementing the logic of 
> > > `ASTNodeImporter::VisitFieldDecl`.
> > > 
> > > So all this should have been handled properly by the preceding import 
> > > call of the FieldDecl! Here
> > > ```
> > > 1735: ExpectedDecl ImportedOrErr = import(From);
> > > ```
> > > I have a suspicion that real reason why this import call fails in case of 
> > > the public ASTImporter::ImportDefinition() is that we fail to drive 
> > > through the import kind (`IDK_Everything`) during the import process.
> > > Below we set IDK_Everything and start a complete import process.
> > > ```
> > >   8784   if (auto *ToRecord = dyn_cast(To)) {
> > >   8785 if (!ToRecord->getDefinition()) {
> > >   8786   return Importer.ImportDefinition(   // 
> > > ASTNodeImporter::ImportDefinition !
> > >   8787   cast(FromDC), ToRecord,
> > >   8788   ASTNodeImporter::IDK_Everything);
> > >   8789 }
> > >   8790   }
> > > ```
> > > However, there may be many places where we fail to channel through that 
> > > we want to do a complete import. E.g here
> > > ```
> > > 1957   
> > > ImportDefinitionIfNeeded(Base1.getType()->getAsCXXRecordDecl()))
> > > ```
> > > we do another definition import and IDK_Everything is not set. So we may 
> > > have a wrong kind of import since the "minimal" flag is set.
> > > 
> > > The thing is, it is really confusing and error-prone to have both the 
> > > `ASTImporter::Minimal` flag and the `ImportDefinitionKind`. They seem to 
> > > be in contradiction to each other some times.
> > > I think we should get rid of the Minimal flag. And Kind should be either 
> > > a full completion (IDK_Everythink) or just a minimal (IDK_Basic). So, I'd 
> > > scrap the IDK_Default too. Alternatively we could have a Kind member in 
> > > AST**//Node//**Importer.
> > > I think we should go into this direction to avoid similar problems during 
> > > CodeGen in the future. WDYT?
> > No offense, you definitely understand the Importer better than I, so I 
> > value your input here. I don't believe that should have other fields where 
> > we could have a record that effects the layout but the concern is still 
> > valid and yes I did miss multi-dimensional arrays.
> > 
> > Your theory on `IDK_Everything` not be pushed through everywhere, I did a 
> > quick look and it seems pretty reasonable. 
> > 
> > I agree that the `ASTImporter::Minimal` flag and the `ImportDefinitionKind` 
> > seem to inconsistent or perhaps a work-around. That seems like a bigger 
> > change with a lot more impact beyond this fix but worth looking into longer 
> > term. 
> > 
> > If pushing through `IDK_Everything` everywhere fixes the underlying issue I 
> > am very happy to take that approach. If not we can discuss alternatives. 
> I've been looking at this code, but I'm still not very familiar with it, so 
> what I am asking may be obvious, but... What is the expected behavior for 
> non-minimal imports for code like this?
> ```
> struct A { ...};
> struct B { A* a; }; // Note the pointer
> ```
> Should importing B also import the definition of the A struct ? As I think 
> that should not happen in the minimal import... If we get rid of the minimal 
> flag, and rely solely on argument passing, we will need to be careful, as it 
> shouldn't be passed _everywhere_ (it should stop at pointers for instance). 
> But then it may not be possible to reproduce the current non-minimal import, 
> as it (I think) expects that A will be fully imported too...
> 
> > I don't believe that should have other fields where we could have a record 
> > that effects the layout
> This isn't exactly layout related, but there is the question of covariant 
> methods. If a method is covariant, then its return type must be complete. 
> Currently we handle the completion of these in LLDB, but that solution is a 
> bit hacky. It might be interesting if that could be handled by the ast 
> importer as well.
> What is the expected behavior for non-minimal imports for code like this?
In this case we import the definition of

[Lldb-commits] [PATCH] D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm going to respond to the rest of your (very insightful) comment later. So 
far, I'm just responding to this:

>>   This isn't exactly layout related, but there is the question of covariant 
>> methods. If a method is covariant, then its return type must be complete.
>
> We already import all the base classes of a derived class (at least in full 
> import). But, we do not import all the derived classes during the import of 
> the base. Is this what you do in LLDB to support covariant return types?

This situation is a bit tricky to explain as there are two independent class 
hierarchies that need to be considered. Let me start with an example:

  struct B1;
  struct B2;
  struct A1 { B1 *f(); };
  struct A2 { B2 *f(); };

This is a perfectly valid C++ snippet. In fact it could be extended to also 
implement `A1::f` and `A2::f` and call them and it would still not require the 
definitions for structs `B1` and `B2`. And I believe the ast importer will 
would not import their definitions even if they were available. It certainly 
wouldn't force them to be defined (`getExternalSource()->CompleteType(B1)`).

This changes if the methods become virtual. In this case, this code becomes 
valid only if `B2*` is convertible to `B1*` (i.e., `B2` is derived from `B1`). 
To know that, both `B1` and `B2` have to be complete. Regular clang parser will 
enforce that, and codegen will depend on it. However, the AST importer will not 
automatically import these classes. Lldb's code to do that is here 
https://github.com/llvm/llvm-project/blob/fdc6aea3fd822b639baaa5b666fdf7598d08c8de/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp#L994.

I don't know whether something like this would be in scope for the default ast 
importer, but it seemed useful to mention in the context of the present 
discussion.


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

https://reviews.llvm.org/D86660

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

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

[Med Ismail Bennani via Phabricator via lldb-commits]

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



Comment at: lldb/include/lldb/Target/Platform.h:643
+  *command_output, // Pass nullptr if you don't want the command output
+  const Timeout &timeout, bool run_in_shell = true);
 

labath wrote:
> The run_in_shell argument makes the inferface weird (why do I need to specify 
> a shell, if I don't want to use it). And still suffers from the virtual 
> default argument problem described above. Let's not propagate this Host 
> weirdness into the platform class.
I started by removing this argument, then I noticed that some code calls 
`Host::RunShellCommand` with the `run_in_shell` argument set to false (i.e. 
`PlatformDarwin::GetXcodeSelectPath`). This means that `Host::RunShellCommand` 
will run the command without prepending the "sh -c" part.

That's why I kept it, and renamed it to `run_in_shell` instead of 
`run_in_default_shell`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86667

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

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 288593.
mib edited the summary of this revision.
mib added a comment.

- Changed CommandObject option from `-i|--interpreter` to `-s|--shell`.
- Updated test program build settings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86667

Files:
  lldb/bindings/interface/SBPlatform.i
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/Host/Host.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Target/RemoteAwarePlatform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/Options.td
  lldb/source/Host/common/Host.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Target/Platform.cpp
  lldb/source/Target/RemoteAwarePlatform.cpp
  lldb/test/API/commands/platform/basic/Makefile
  lldb/test/API/commands/platform/basic/TestPlatformCommand.py
  lldb/test/API/commands/platform/basic/TestPlatformPython.py
  lldb/test/API/commands/platform/basic/myshell.c

Index: lldb/test/API/commands/platform/basic/myshell.c
===
--- /dev/null
+++ lldb/test/API/commands/platform/basic/myshell.c
@@ -0,0 +1,24 @@
+#include 
+#include 
+#include 
+
+int main(int argc, char *argv[]) {
+  if (argc < 3) {
+fprintf(stderr, "ERROR: Too few arguments (count: %d).\n", argc - 1);
+exit(1);
+  }
+
+#ifdef WIN32
+  char *cmd_opt = "/C";
+#else
+  char *cmd_opt = "-c";
+#endif
+
+  if (strncmp(argv[1], cmd_opt, 2)) {
+fprintf(stderr, "ERROR: Missing shell command option ('%s').\n", cmd_opt);
+exit(1);
+  }
+
+  printf("SUCCESS: %s\n", argv[0]);
+  return 0;
+}
Index: lldb/test/API/commands/platform/basic/TestPlatformPython.py
===
--- lldb/test/API/commands/platform/basic/TestPlatformPython.py
+++ lldb/test/API/commands/platform/basic/TestPlatformPython.py
@@ -13,6 +13,7 @@
 class PlatformPythonTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
 
 @add_test_categories(['pyapi'])
 @no_debug_info_test
@@ -79,3 +80,20 @@
 self.assertEqual(
 desc_data.GetType(), lldb.eStructuredDataTypeString,
 'Platform description is a string')
+
+@add_test_categories(['pyapi'])
+@no_debug_info_test
+def test_shell_interpreter(self):
+""" Test a shell with a custom interpreter """
+platform = self.dbg.GetSelectedPlatform()
+self.assertTrue(platform.IsValid())
+
+sh_cmd = lldb.SBPlatformShellCommand('/bin/zsh', 'echo $0')
+self.assertIn('/bin/zsh', sh_cmd.GetInterpreter())
+self.assertIn('echo $0', sh_cmd.GetCommand())
+
+self.build()
+sh_cmd.SetInterpreter(self.getBuildArtifact('a.out'))
+err = platform.Run(sh_cmd)
+self.assertTrue(err.Success())
+self.assertIn("SUCCESS", sh_cmd.GetOutput())
Index: lldb/test/API/commands/platform/basic/TestPlatformCommand.py
===
--- lldb/test/API/commands/platform/basic/TestPlatformCommand.py
+++ lldb/test/API/commands/platform/basic/TestPlatformCommand.py
@@ -13,6 +13,7 @@
 class PlatformCommandTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
 
 @no_debug_info_test
 def test_help_platform(self):
@@ -92,3 +93,11 @@
 "error: timed out waiting for shell command to complete"])
 self.expect("shell -t 1 --  sleep 3", error=True, substrs=[
 "error: timed out waiting for shell command to complete"])
+
+@no_debug_info_test
+def test_host_shell_interpreter(self):
+""" Test the host platform shell with a different interpreter """
+self.build()
+exe = self.getBuildArtifact('a.out')
+self.expect("platform shell -h -s " + exe + " -- 'echo $0'",
+substrs=['SUCCESS', 'a.out'])
Index: lldb/test/API/commands/platform/basic/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/platform/basic/Makefile
@@ -0,0 +1,5 @@
+C_SOURCES := myshell.c
+CFLAGS_EXTRAS := -g0 # No debug info.
+MAKE_DSYM := NO
+
+include Makefile.rules
Index: lldb/source/Target/RemoteAwarePlatform.cpp
===
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -170,16 +170,31 @@
   return error;
 }
 
-Status RemoteAwarePlatform::RunShellCommand(
-const char *command, cons

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Target/Platform.h:643
+  *command_output, // Pass nullptr if you don't want the command output
+  const Timeout &timeout, bool run_in_shell = true);
 

mib wrote:
> labath wrote:
> > The run_in_shell argument makes the inferface weird (why do I need to 
> > specify a shell, if I don't want to use it). And still suffers from the 
> > virtual default argument problem described above. Let's not propagate this 
> > Host weirdness into the platform class.
> I started by removing this argument, then I noticed that some code calls 
> `Host::RunShellCommand` with the `run_in_shell` argument set to false (i.e. 
> `PlatformDarwin::GetXcodeSelectPath`). This means that 
> `Host::RunShellCommand` will run the command without prepending the "sh -c" 
> part.
> 
> That's why I kept it, and renamed it to `run_in_shell` instead of 
> `run_in_default_shell`.
Right, but that code only calls the Host version. I'm only saying we should 
delete it (or, not add it, to be precise) to the Platform version. (I'd like to 
remove it from the Host version too, but that's a different story.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86667

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

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 288597.
mib edited the summary of this revision.
mib added a comment.

Removed `bool run_in_shell` argument from `Platform` classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86667

Files:
  lldb/bindings/interface/SBPlatform.i
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/Host/Host.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Target/RemoteAwarePlatform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/Options.td
  lldb/source/Host/common/Host.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Target/Platform.cpp
  lldb/source/Target/RemoteAwarePlatform.cpp
  lldb/test/API/commands/platform/basic/Makefile
  lldb/test/API/commands/platform/basic/TestPlatformCommand.py
  lldb/test/API/commands/platform/basic/TestPlatformPython.py
  lldb/test/API/commands/platform/basic/myshell.c

Index: lldb/test/API/commands/platform/basic/myshell.c
===
--- /dev/null
+++ lldb/test/API/commands/platform/basic/myshell.c
@@ -0,0 +1,24 @@
+#include 
+#include 
+#include 
+
+int main(int argc, char *argv[]) {
+  if (argc < 3) {
+fprintf(stderr, "ERROR: Too few arguments (count: %d).\n", argc - 1);
+exit(1);
+  }
+
+#ifdef WIN32
+  char *cmd_opt = "/C";
+#else
+  char *cmd_opt = "-c";
+#endif
+
+  if (strncmp(argv[1], cmd_opt, 2)) {
+fprintf(stderr, "ERROR: Missing shell command option ('%s').\n", cmd_opt);
+exit(1);
+  }
+
+  printf("SUCCESS: %s\n", argv[0]);
+  return 0;
+}
Index: lldb/test/API/commands/platform/basic/TestPlatformPython.py
===
--- lldb/test/API/commands/platform/basic/TestPlatformPython.py
+++ lldb/test/API/commands/platform/basic/TestPlatformPython.py
@@ -13,6 +13,7 @@
 class PlatformPythonTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
 
 @add_test_categories(['pyapi'])
 @no_debug_info_test
@@ -79,3 +80,20 @@
 self.assertEqual(
 desc_data.GetType(), lldb.eStructuredDataTypeString,
 'Platform description is a string')
+
+@add_test_categories(['pyapi'])
+@no_debug_info_test
+def test_shell_interpreter(self):
+""" Test a shell with a custom interpreter """
+platform = self.dbg.GetSelectedPlatform()
+self.assertTrue(platform.IsValid())
+
+sh_cmd = lldb.SBPlatformShellCommand('/bin/zsh', 'echo $0')
+self.assertIn('/bin/zsh', sh_cmd.GetInterpreter())
+self.assertIn('echo $0', sh_cmd.GetCommand())
+
+self.build()
+sh_cmd.SetInterpreter(self.getBuildArtifact('a.out'))
+err = platform.Run(sh_cmd)
+self.assertTrue(err.Success())
+self.assertIn("SUCCESS", sh_cmd.GetOutput())
Index: lldb/test/API/commands/platform/basic/TestPlatformCommand.py
===
--- lldb/test/API/commands/platform/basic/TestPlatformCommand.py
+++ lldb/test/API/commands/platform/basic/TestPlatformCommand.py
@@ -13,6 +13,7 @@
 class PlatformCommandTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
 
 @no_debug_info_test
 def test_help_platform(self):
@@ -92,3 +93,11 @@
 "error: timed out waiting for shell command to complete"])
 self.expect("shell -t 1 --  sleep 3", error=True, substrs=[
 "error: timed out waiting for shell command to complete"])
+
+@no_debug_info_test
+def test_host_shell_interpreter(self):
+""" Test the host platform shell with a different interpreter """
+self.build()
+exe = self.getBuildArtifact('a.out')
+self.expect("platform shell -h -s " + exe + " -- 'echo $0'",
+substrs=['SUCCESS', 'a.out'])
Index: lldb/test/API/commands/platform/basic/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/platform/basic/Makefile
@@ -0,0 +1,5 @@
+C_SOURCES := myshell.c
+CFLAGS_EXTRAS := -g0 # No debug info.
+MAKE_DSYM := NO
+
+include Makefile.rules
Index: lldb/source/Target/RemoteAwarePlatform.cpp
===
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -171,15 +171,24 @@
 }
 
 Status RemoteAwarePlatform::RunShellCommand(
-const char *command, const FileSpec &working_dir, int *status_ptr,
+llvm::StringRef com

[Lldb-commits] [PATCH] D86792: [lldb] Improve test failure reporting for expect()

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
DavidSpickett requested review of this revision.
Herald added a subscriber: JDevlieghere.

This updates the errors reported by expect()
to something like:
'''
Ran command:
"help"

Got output:
Debugger commands:
<...>

Expecting start string: "Debugger commands:" (was found)
Expecting end string: "foo" (was not found)
'''
(see added tests for more examples)

This shows the user exactly what was run,
what checks passed and which failed. Along with
whether that check was supposed to pass.
(including what regex patterns matched)

These lines are also output to the test
trace file, whether the test passes or not.

Note that expect() will still fail at the first failed
check, in line with previous behaviour.

Also I have flipped the wording of the assert
message functions (.*_MSG) to describe failures
not successes. This makes more sense as they are
only shown on assert failures.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86792

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/assert_messages_test/TestAssertMessages.py

Index: lldb/test/API/assert_messages_test/TestAssertMessages.py
===
--- /dev/null
+++ lldb/test/API/assert_messages_test/TestAssertMessages.py
@@ -0,0 +1,113 @@
+"""
+Test the format of API test suite assert failure messages
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from textwrap import dedent
+
+
+class AssertMessagesTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def catch_assert_fail(self, *args, **kwargs):
+try:
+self.expect(*args, **kwargs)
+except AssertionError as e:
+return e
+else:
+self.fail("Expect should have raised AssertionError!")
+
+def expect_assert_msg(self, exception, substrs):
+self.expect(str(exception), exe=False, substrs=substrs)
+
+def test_expect(self):
+"""Test format of messages produced by expect(...)"""
+
+# When an expect passes the messages are sent to the trace
+# file which we can't access here. So really, these only
+# check what failures look like, but it *should* be the same
+# content for the trace log too.
+
+# Will stop at startstr fail
+e = self.catch_assert_fail("help", startstr="dog", endstr="cat")
+self.expect_assert_msg(e, [dedent("""\
+Ran command:
+"help"
+
+Got output:
+Debugger commands:"""),
+"""Expecting start string: "dog" (was not found)"""])
+
+# startstr passes, endstr fails
+# We see both reported
+e = self.catch_assert_fail("help",
+startstr="Debugger commands:", endstr="foo")
+self.expect_assert_msg(e, [dedent("""\
+Ran command:
+"help"
+
+Got output:
+Debugger commands:"""),
+"""Expecting start string: "Debugger commands:" (was found)""",
+"""Expecting end string: "foo" (was not found)"""])
+
+# Same thing for substrs, regex patterns ignored because of substr failure
+# Any substr after the first missing is also ignored
+e = self.catch_assert_fail("abcdefg", substrs=["abc", "ijk", "xyz"],
+patterns=["foo", "bar"], exe=False)
+self.expect_assert_msg(e, [dedent("""\
+Checking string:
+"abcdefg"
+
+Expecting sub string: "abc" (was found)
+Expecting sub string: "ijk" (was not found)""")])
+
+# Regex patterns also stop at first failure, subsequent patterns ignored
+# They are last in the chain so no other check gets skipped
+# Including the rest of the conditions here to prove they are run and shown
+e = self.catch_assert_fail("0123456789",
+startstr="012", endstr="789", substrs=["345", "678"],
+patterns=["[0-9]+", "[a-f]+", "a|b|c"], exe=False)
+self.expect_assert_msg(e, [dedent("""\
+Checking string:
+"0123456789"
+
+Expecting start string: "012" (was found)
+Expecting end string: "789" (was found)
+Expecting sub string: "345" (was found)
+Expecting sub string: "678" (was found)
+Expecting regex pattern: "[0-9]+" (was found, matched "0123456789")
+Expecting regex pattern: "[a-f]+" (was not found)""")])
+
+# This time we dont' want matches but we do get them
+e = self.catch_assert_fail("the quick brown fox",
+startstr="cat", endstr="rabbit", substrs=["abc", "def"],
+# Note that the second patte

[Lldb-commits] [PATCH] D86792: [lldb] Improve test failure reporting for expect()

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added reviewers: teemperor, labath.
DavidSpickett added a comment.

I realise the test is a bit meta so if there might be a better place to put it. 
It does at least run successfully from the API test dir.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86792

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

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

You changed my mind. I'll move to Support/JSON.h




Comment at: lldb/test/API/commands/trace/intelpt-trace/trace.json:11
+  },
+  "triple": "x86_64-*-linux",
+  "processes": [

labath wrote:
> What if one of the processes is 64-bit and the other 32? It sounds like this 
> should be a property of a particular process.
That's a very good point. Will do so


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705

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

[Lldb-commits] [PATCH] D86792: [lldb] Improve test failure reporting for expect()

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:2460
+matched = output.startswith(startstr)
+log_lines.append("{} start string: \"{}\" ({})".format(
+expecting_str, startstr, found_str(matched)))

I've added speech marks around things that are probably on the shorter side. 
I've found it helps narrow down issues with empty strings or leading/trailing 
whitespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86792

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

[Lldb-commits] [PATCH] D86667: [lldb/Target] Add custom interpreter option to `platform shell`

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 288636.
mib added a comment.

Replaced all the `interpreter` occurrences with `shell` in the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86667

Files:
  lldb/bindings/interface/SBPlatform.i
  lldb/include/lldb/API/SBPlatform.h
  lldb/include/lldb/Host/Host.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Target/RemoteAwarePlatform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/Options.td
  lldb/source/Host/common/Host.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Target/Platform.cpp
  lldb/source/Target/RemoteAwarePlatform.cpp
  lldb/test/API/commands/platform/basic/Makefile
  lldb/test/API/commands/platform/basic/TestPlatformCommand.py
  lldb/test/API/commands/platform/basic/TestPlatformPython.py
  lldb/test/API/commands/platform/basic/myshell.c

Index: lldb/test/API/commands/platform/basic/myshell.c
===
--- /dev/null
+++ lldb/test/API/commands/platform/basic/myshell.c
@@ -0,0 +1,24 @@
+#include 
+#include 
+#include 
+
+int main(int argc, char *argv[]) {
+  if (argc < 3) {
+fprintf(stderr, "ERROR: Too few arguments (count: %d).\n", argc - 1);
+exit(1);
+  }
+
+#ifdef WIN32
+  char *cmd_opt = "/C";
+#else
+  char *cmd_opt = "-c";
+#endif
+
+  if (strncmp(argv[1], cmd_opt, 2)) {
+fprintf(stderr, "ERROR: Missing shell command option ('%s').\n", cmd_opt);
+exit(1);
+  }
+
+  printf("SUCCESS: %s\n", argv[0]);
+  return 0;
+}
Index: lldb/test/API/commands/platform/basic/TestPlatformPython.py
===
--- lldb/test/API/commands/platform/basic/TestPlatformPython.py
+++ lldb/test/API/commands/platform/basic/TestPlatformPython.py
@@ -13,6 +13,7 @@
 class PlatformPythonTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
 
 @add_test_categories(['pyapi'])
 @no_debug_info_test
@@ -79,3 +80,20 @@
 self.assertEqual(
 desc_data.GetType(), lldb.eStructuredDataTypeString,
 'Platform description is a string')
+
+@add_test_categories(['pyapi'])
+@no_debug_info_test
+def test_shell_interpreter(self):
+""" Test a shell with a custom interpreter """
+platform = self.dbg.GetSelectedPlatform()
+self.assertTrue(platform.IsValid())
+
+sh_cmd = lldb.SBPlatformShellCommand('/bin/zsh', 'echo $0')
+self.assertIn('/bin/zsh', sh_cmd.GetShell())
+self.assertIn('echo $0', sh_cmd.GetCommand())
+
+self.build()
+sh_cmd.SetShell(self.getBuildArtifact('a.out'))
+err = platform.Run(sh_cmd)
+self.assertTrue(err.Success())
+self.assertIn("SUCCESS", sh_cmd.GetOutput())
Index: lldb/test/API/commands/platform/basic/TestPlatformCommand.py
===
--- lldb/test/API/commands/platform/basic/TestPlatformCommand.py
+++ lldb/test/API/commands/platform/basic/TestPlatformCommand.py
@@ -13,6 +13,7 @@
 class PlatformCommandTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
 
 @no_debug_info_test
 def test_help_platform(self):
@@ -92,3 +93,11 @@
 "error: timed out waiting for shell command to complete"])
 self.expect("shell -t 1 --  sleep 3", error=True, substrs=[
 "error: timed out waiting for shell command to complete"])
+
+@no_debug_info_test
+def test_host_shell_interpreter(self):
+""" Test the host platform shell with a different interpreter """
+self.build()
+exe = self.getBuildArtifact('a.out')
+self.expect("platform shell -h -s " + exe + " -- 'echo $0'",
+substrs=['SUCCESS', 'a.out'])
Index: lldb/test/API/commands/platform/basic/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/platform/basic/Makefile
@@ -0,0 +1,5 @@
+C_SOURCES := myshell.c
+CFLAGS_EXTRAS := -g0 # No debug info.
+MAKE_DSYM := NO
+
+include Makefile.rules
Index: lldb/source/Target/RemoteAwarePlatform.cpp
===
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -171,15 +171,24 @@
 }
 
 Status RemoteAwarePlatform::RunShellCommand(
-const char *command, const FileSpec &working_dir, int *status_ptr,
+llvm::StringRef command, const FileSpec &working_dir, int *status

[Lldb-commits] [PATCH] D86652: [lldb] Fix ProcessEventData::DoOnRemoval to support multiple listeners

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D86652#2244043 , @labath wrote:

> This is going to derail this patch somewhat (sorry), but since this is an 
> important long-standing open issue, I think we should discuss it in more 
> detail.
>
> The part that's bothering me about this for quite some time is... Why do we 
> even have this DoOnRemoval stuff in the first place?
>
> I mean instead of doing this work in `DoOnRemoval`, why don't we do this work 
> /before/ enqueueing the relevant event. Then the event would be of purely 
> informative character and it would not matter if its received by one, two, 
> ten, or zero listeners. Of course, if a user wanted to program lldb in a 
> correct and race-free manner, it would /have to/ listen to these events (as 
> otherwise it would not know when is it safe to run some actions), but that 
> would be something that's entirely up to him.
>
> Back when I was debugging some races in tests with asynchronous listeners, I 
> remember it was pretty hard to reason about things and prove they are 
> correct, since the dequeueing of the event (and the resulting flurry of 
> actions within lldb) could come at any moment. If those actions were done 
> internally to lldb, they would be more sequenced more deterministically and 
> be easier to reason about than if they are executed whenever the user chooses 
> to dequeue an event. (I remember things got particularly messy when a test 
> finished before dequeuing an event, at which point the processing of the 
> event was executing concurrently with the `process.Kill()` statement we 
> execute at the end of each test.)

It would certainly be worth rethinking this, these were decisions made very 
early in the development of lldb...

I did it the way I did for several reasons:

First of all was to get the work that was done by the DoOnRemoval - which is 
basically anything lldb can do since it's where the breakpoint callbacks are 
triggered - off the private state thread.  The private state thread needs to be 
alive and getting private events and running the thread plans to do the work 
required by breakpoint callbacks, and while I do have a system to spin up 
another private state threads if you need to use one while doing some piece of 
work on the private state thread, that seemed fragile to me and I didn't want 
to lean on it too heavily.  It is important to make the transition from lldb 
doing internal work with the incoming event stream to giving over control to 
the user.  Internally, we have to let ourselves do things based off the private 
state, but external users are gated by the public state instead,  Having it 
happen when the event gets handed to the user was a convenient place to put 
this barrier.

The other reason I did it this way was that I didn't want the process state and 
the event stream to get out of sync.  I we do all the work on an event 
including setting the public state to stopped before posting the event, then 
you will easily get into the situation where the public state said "stopped" 
when you haven't yet fetched the stopped event off of the event queue.  This 
inconsistencies seemed likely to result in confusing errors in use of the API's.

If we can come up with another way to achieve these goals but do the 
user-facing work of processing the event somewhere else, I don't have any 
objections.

> In D86652#2242770 , @ilya-nozhkin 
> wrote:
>
>> One of the possible solutions is to send a specifically marked event to a 
>> primary listener first. Then, make this event to invoke a handshake in its 
>> destructor (which would mean that primary listener doesn't need the event 
>> anymore). And handshake can be implemented as just a rebroadcasting a new 
>> instance of the event to other listeners (or via unlocking some additional 
>> mutex / notifying some conditional variable). However, destructor may be 
>> invoked not immediately after event processing is finished. For example, if 
>> a primary listener's thread defines an EventSP before the listening cycle 
>> and doesn't reset it until GetEvent returns a new one (which may not happen 
>> until the next stop, for example).
>
> This primary listener technique could be implemented purely in your own code, 
> could it not? You could have one listener, which listens to lldb events, and 
> then rebroadcasts it (using arbitrary techniques, not necessarily SBListener) 
> to any number of interested threads that want to act on it. That might make 
> it easier to ensure there are no races in your code, and would avoid stepping 
> on some of the internal lldb races that I am sure we have... Otherwise I (as 
> Jim) expect that you will still need to have some sort of communication 
> between the various listeners listening to process events, as they'd need to 
> coordinate
>
>> Anyway, the concept of primary and secondary listeners requires to forbid 
>> secondary listeners to do "step" or

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

[Galina via Phabricator via lldb-commits]

gkistanova added a comment.

Hi Mateusz,
Yes, this looks like a separate patch on top of this revert.

By the way, this hasn't been merged yet. Are we waiting for anything? Harmen, 
do you need a help committing this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86521

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

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

[Galina via Phabricator via lldb-commits]

gkistanova added a comment.

Fair enough. Reverted.
https://github.com/llvm/llvm-project/commit/cdcb9ab10e53ff08293915af3cd897c42112bcc5

Thanks, everyone!

Petr, please feel free to send me a patch you want me to check for you. Make 
sure I could apply it on top of the trunk.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86521

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

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

[Petr Hosek via Phabricator via lldb-commits]

phosek added a comment.

I have suggested an alternative in https://reviews.llvm.org/D85820#2237357 to 
try and fix this forward and I was waiting for @haampie's response before 
landing this. I can implement that today if you're OK fixing this forward? I 
think it'd be easier to do that than reverting and then relanding all these 
changes again. @gkistanova how did you check the patch with 
http://lab.llvm.org:8011/builders/lld-perf-testsuite bot? Is there a way to do 
a presubmit check on that bot?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86521

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

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

[Harmen Stoppels via Phabricator via lldb-commits]

haampie added a comment.

@phosek it would be great to get that patch in, but the truth is it is 
difficult testing across multiple platforms for me; I can only test on Linux 
and macOS, not Windows (GNU). Since that patch would touch predefined, 
platform-dependent cmake variables, I would want to test it properly before 
submitting a patch, which might take a bit more time. So for me it would be 
best to temporarily accept this reverting patch, and submit a properly tested 
patch afterwards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86521

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

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

[Galina via Phabricator via lldb-commits]

gkistanova added a comment.

Hi Petr,

I don't have a strong opinion on the exact way of fixing the problem. But we 
should do this as soon as possible, as keeping the bots red for that long is 
not Ok.

Please send me your patch and I will check it on lld-perf-testsuite bot. If you 
do not have the patch ready yet, I'd say we shall commit this revert to buy 
more time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86521

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

[Lldb-commits] [PATCH] D86521: Revert "Use find_library for ncurses"

[Galina via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcdcb9ab10e53: Revert "Use find_library for 
ncurses" (authored by haampie, committed by gkistanova).

Changed prior to commit:
  https://reviews.llvm.org/D86521?vs=287618&id=288497#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86521

Files:
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/xray/tests/CMakeLists.txt
  lldb/source/Core/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Unix/Process.inc
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -286,9 +286,9 @@
   }
 
   if (llvm_enable_terminfo) {
-values += [ "LLVM_ENABLE_TERMINFO=1" ]
+values += [ "HAVE_TERMINFO=1" ]
   } else {
-values += [ "LLVM_ENABLE_TERMINFO=" ]
+values += [ "HAVE_TERMINFO=" ]
   }
 
   if (llvm_enable_dia_sdk) {
Index: llvm/lib/Support/Unix/Process.inc
===
--- llvm/lib/Support/Unix/Process.inc
+++ llvm/lib/Support/Unix/Process.inc
@@ -313,7 +313,7 @@
   return getColumns();
 }
 
-#ifdef LLVM_ENABLE_TERMINFO
+#ifdef HAVE_TERMINFO
 // We manually declare these extern functions because finding the correct
 // headers from various terminfo, curses, or other sources is harder than
 // writing their specs down.
@@ -323,12 +323,12 @@
 extern "C" int tigetnum(char *capname);
 #endif
 
-#ifdef LLVM_ENABLE_TERMINFO
+#ifdef HAVE_TERMINFO
 static ManagedStatic TermColorMutex;
 #endif
 
 static bool terminalHasColors(int fd) {
-#ifdef LLVM_ENABLE_TERMINFO
+#ifdef HAVE_TERMINFO
   // First, acquire a global lock because these C routines are thread hostile.
   std::lock_guard G(*TermColorMutex);
 
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -2,19 +2,6 @@
   set(imported_libs ZLIB::ZLIB)
 endif()
 
-function(get_system_libname libpath libname)
-  get_filename_component(libpath ${libpath} NAME)
-  if( CMAKE_FIND_LIBRARY_PREFIXES )
-string(REPLACE ";" "|" PREFIXES "${CMAKE_FIND_LIBRARY_PREFIXES}")
-string(REGEX REPLACE "^(${PREFIXES})" "" libpath ${libpath})
-  endif()
-  if( CMAKE_FIND_LIBRARY_SUFFIXES )
-string(REPLACE ";" "|" SUFFIXES "${CMAKE_FIND_LIBRARY_SUFFIXES}")
-string(REGEX REPLACE "(${SUFFIXES})$" "" libpath ${libpath})
-  endif()
-  set(${libname} "${libpath}" PARENT_SCOPE)
-endfunction()
-
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -34,8 +21,10 @@
 STRING(REGEX REPLACE "^lib" "" Backtrace_LIBFILE ${Backtrace_LIBFILE})
 set(system_libs ${system_libs} ${Backtrace_LIBFILE})
   endif()
-  if( LLVM_ENABLE_TERMINFO )
-set(imported_libs ${imported_libs} "${TERMINFO_LIB}")
+  if(LLVM_ENABLE_TERMINFO)
+if(HAVE_TERMINFO)
+  set(system_libs ${system_libs} ${TERMINFO_LIBS})
+endif()
   endif()
   if( LLVM_ENABLE_THREADS AND (HAVE_LIBATOMIC OR HAVE_CXX_LIBATOMICS64) )
 set(system_libs ${system_libs} atomic)
@@ -248,15 +237,20 @@
   if(NOT zlib_library)
 get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
   endif()
-  get_system_libname(${zlib_library} zlib_library)
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND CMAKE_STATIC_LIBRARY_SUFFIX AND
+  zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND CMAKE_SHARED_LIBRARY_SUFFIX AND
+  zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*${CMAKE_SHARED_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+STRING(REGEX REPLACE "${CMAKE_SHARED_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
   set(llvm_system_libs ${llvm_system_libs} "${zlib_library}")
 endif()
 
-if(LLVM_ENABLE_TERMINFO)
-  get_system_libname(${TERMINFO_LIB} terminfo_library)
-  set(llvm_system_libs ${llvm_system_libs} "${terminfo_library}")
-endif()
-
 set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${llvm_system_libs}")
 
 if(LLVM_WITH_Z3)
Index: llvm/include/llvm/Config/config.h.cmake
==

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D86670#2244092 , @labath wrote:

> I think it would be appropriate to discuss the design of this feature on 
> lldb-dev before going over the individual patches. One of the fundamental 
> aspects of this patchset that I think should not be overlooked is that it 
> essentially adds a new level of structure ( a "Target Group") lldb. One of 
> the questions I'd have is whether this concept shouldn't be formalized 
> somewhere instead of it existing just virtually as the group of threads that 
> happen to share the same trace object.

The idea was to iterate on the design of the trace feature using these patches 
and have the discussion here. The idea is one trace file can create N 
individual targets. Each target can be independent and the threads contained in 
each belong to each target.  "trace dump" when no args are supplied it could 
maybe only dump the currently selected target to start with? I am not sure what 
the right semantics are, but that shouldn't stop us with proceeding with these 
patches IMHO. It will be easy to modify as we evolve if we do want to have 
target groups, but I don't think it is required for these patches. The target 
group is a much higher level design that can affect many things and could be 
used in this case, but isn't required. If a trace plug-in needs to iterate over 
all of the targets this can be achieved by using the debugger to grab all 
targets and iterate over them, so no real group is required.

So it would simplify things right now if we say that "trace dump" dumps the 
trace data for the currently selected target right now. That will map well with 
the stepping commands that will soon be added to allow forward and reverse 
traversal of the trace data.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86670

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

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

[walter erquinigo via Phabricator via lldb-commits]

wallace marked an inline comment as not done.
wallace added a comment.

I agree with that Greg said.

> So it would simplify things right now if we say that "trace dump" dumps the 
> trace data for the currently selected target right now. That will map well 
> with the stepping commands that will soon be added to allow forward and 
> reverse traversal of the trace data.

I'll go ahead with this in order to simplify the code. We can revisit that later


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86670

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

[Lldb-commits] [lldb] e5e05ec - [lldb/test] Use @skipIfWindows for PExpectTest

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2020-08-28T11:41:07-07:00
New Revision: e5e05ecf65aba836802154279efbc8cbce6fe2ea

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

LOG: [lldb/test] Use @skipIfWindows for PExpectTest

Annotating `PExpectTest` with `@skipIfWindows` instead of marking it as an 
empty class will make the test runner recognize it as a test class, which 
should allow me to reland adb5c23f8c0d60eeec41dcbe21d1b26184e1c97d.

I don't have a windows machine to verify this works, but I did some tests using 
`@skipIfLinux` and they all worked as expected. In case the `pexpect` import is 
not at all available on windows, I moved it to within the method where it's 
used.

Reviewed By: labath

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

Added: 


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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py 
b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
index 67de73bf8970..12ac4d381e2c 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -13,65 +13,60 @@
 from . import lldbutil
 from lldbsuite.test.decorators import *
 
-if sys.platform.startswith('win32'):
-# llvm.org/pr22274: need a pexpect replacement for windows
-class PExpectTest(object):
-pass
-else:
-import pexpect
+@skipIfRemote
+@skipIfWindows  # llvm.org/pr22274: need a pexpect replacement for windows
+class PExpectTest(TestBase):
 
-@skipIfRemote
-class PExpectTest(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+PROMPT = "(lldb) "
 
-NO_DEBUG_INFO_TESTCASE = True
-PROMPT = "(lldb) "
+def expect_prompt(self):
+self.child.expect_exact(self.PROMPT)
 
-def expect_prompt(self):
-self.child.expect_exact(self.PROMPT)
+def launch(self, executable=None, extra_args=None, timeout=30, 
dimensions=None):
+logfile = getattr(sys.stdout, 'buffer',
+sys.stdout) if self.TraceOn() else None
 
-def launch(self, executable=None, extra_args=None, timeout=30, 
dimensions=None):
-logfile = getattr(sys.stdout, 'buffer',
-  sys.stdout) if self.TraceOn() else None
+args = ['--no-lldbinit', '--no-use-colors']
+for cmd in self.setUpCommands():
+args += ['-O', cmd]
+if executable is not None:
+args += ['--file', executable]
+if extra_args is not None:
+args.extend(extra_args)
 
-args = ['--no-lldbinit', '--no-use-colors']
-for cmd in self.setUpCommands():
-args += ['-O', cmd]
-if executable is not None:
-args += ['--file', executable]
-if extra_args is not None:
-args.extend(extra_args)
+env = dict(os.environ)
+env["TERM"]="vt100"
 
-env = dict(os.environ)
-env["TERM"]="vt100"
-
-self.child = pexpect.spawn(
-lldbtest_config.lldbExec, args=args, logfile=logfile,
-timeout=timeout, dimensions=dimensions, env=env)
+import pexpect
+self.child = pexpect.spawn(
+lldbtest_config.lldbExec, args=args, logfile=logfile,
+timeout=timeout, dimensions=dimensions, env=env)
+self.expect_prompt()
+for cmd in self.setUpCommands():
+self.child.expect_exact(cmd)
 self.expect_prompt()
-for cmd in self.setUpCommands():
-self.child.expect_exact(cmd)
-self.expect_prompt()
-if executable is not None:
-self.child.expect_exact("target create")
-self.child.expect_exact("Current executable set to")
-self.expect_prompt()
-
-def expect(self, cmd, substrs=None):
-self.assertNotIn('\n', cmd)
-self.child.sendline(cmd)
-if substrs is not None:
-for s in substrs:
-self.child.expect_exact(s)
+if executable is not None:
+self.child.expect_exact("target create")
+self.child.expect_exact("Current executable set to")
 self.expect_prompt()
 
-def quit(self, gracefully=True):
-self.child.sendeof()
-self.child.close(force=not gracefully)
-self.child = None
+def expect(self, cmd, substrs=None):
+self.assertNotIn('\n', cmd)
+self.child.sendline(cmd)
+if substrs is not None:
+for s in substrs:
+self.child.expect_exact(s)
+self.expect_prompt()
+
+def quit(self, gracefully=True):
+self.c

[Lldb-commits] [PATCH] D86745: [lldb/test] Use @skipIfWindows for PExpectTest

[Jordan Rupprecht via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe5e05ecf65ab: [lldb/test] Use @skipIfWindows for PExpectTest 
(authored by rupprecht).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86745

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

Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -13,65 +13,60 @@
 from . import lldbutil
 from lldbsuite.test.decorators import *
 
-if sys.platform.startswith('win32'):
-# llvm.org/pr22274: need a pexpect replacement for windows
-class PExpectTest(object):
-pass
-else:
-import pexpect
+@skipIfRemote
+@skipIfWindows  # llvm.org/pr22274: need a pexpect replacement for windows
+class PExpectTest(TestBase):
 
-@skipIfRemote
-class PExpectTest(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+PROMPT = "(lldb) "
 
-NO_DEBUG_INFO_TESTCASE = True
-PROMPT = "(lldb) "
+def expect_prompt(self):
+self.child.expect_exact(self.PROMPT)
 
-def expect_prompt(self):
-self.child.expect_exact(self.PROMPT)
+def launch(self, executable=None, extra_args=None, timeout=30, dimensions=None):
+logfile = getattr(sys.stdout, 'buffer',
+sys.stdout) if self.TraceOn() else None
 
-def launch(self, executable=None, extra_args=None, timeout=30, dimensions=None):
-logfile = getattr(sys.stdout, 'buffer',
-  sys.stdout) if self.TraceOn() else None
+args = ['--no-lldbinit', '--no-use-colors']
+for cmd in self.setUpCommands():
+args += ['-O', cmd]
+if executable is not None:
+args += ['--file', executable]
+if extra_args is not None:
+args.extend(extra_args)
 
-args = ['--no-lldbinit', '--no-use-colors']
-for cmd in self.setUpCommands():
-args += ['-O', cmd]
-if executable is not None:
-args += ['--file', executable]
-if extra_args is not None:
-args.extend(extra_args)
+env = dict(os.environ)
+env["TERM"]="vt100"
 
-env = dict(os.environ)
-env["TERM"]="vt100"
-
-self.child = pexpect.spawn(
-lldbtest_config.lldbExec, args=args, logfile=logfile,
-timeout=timeout, dimensions=dimensions, env=env)
+import pexpect
+self.child = pexpect.spawn(
+lldbtest_config.lldbExec, args=args, logfile=logfile,
+timeout=timeout, dimensions=dimensions, env=env)
+self.expect_prompt()
+for cmd in self.setUpCommands():
+self.child.expect_exact(cmd)
 self.expect_prompt()
-for cmd in self.setUpCommands():
-self.child.expect_exact(cmd)
-self.expect_prompt()
-if executable is not None:
-self.child.expect_exact("target create")
-self.child.expect_exact("Current executable set to")
-self.expect_prompt()
-
-def expect(self, cmd, substrs=None):
-self.assertNotIn('\n', cmd)
-self.child.sendline(cmd)
-if substrs is not None:
-for s in substrs:
-self.child.expect_exact(s)
+if executable is not None:
+self.child.expect_exact("target create")
+self.child.expect_exact("Current executable set to")
 self.expect_prompt()
 
-def quit(self, gracefully=True):
-self.child.sendeof()
-self.child.close(force=not gracefully)
-self.child = None
+def expect(self, cmd, substrs=None):
+self.assertNotIn('\n', cmd)
+self.child.sendline(cmd)
+if substrs is not None:
+for s in substrs:
+self.child.expect_exact(s)
+self.expect_prompt()
+
+def quit(self, gracefully=True):
+self.child.sendeof()
+self.child.close(force=not gracefully)
+self.child = None
 
-def cursor_forward_escape_seq(self, chars_to_move):
-"""
-Returns the escape sequence to move the cursor forward/right
-by a certain amount of characters.
-"""
-return b"\x1b\[" + str(chars_to_move).encode("utf-8") + b"C"
+def cursor_forward_escape_seq(self, chars_to_move):
+"""
+Returns the escape sequence to move the cursor forward/right
+by a certain amount of characters.
+"""
+return b"\x1b\[" + str(chars_to_move).encode("utf-8") + b"C"
___
lldb-commits mailing list
lldb-commits@

[Lldb-commits] [lldb] cdc1816 - [lldb] Fix typo in disassemble_options_line description

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-08-28T11:41:59-07:00
New Revision: cdc18163cd1449c3a1c20e65a4d95a35ba3f6c23

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

LOG: [lldb] Fix typo in disassemble_options_line description

Added: 


Modified: 
lldb/source/Commands/Options.td

Removed: 




diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 6384179f0ef1..fbb64957f48d 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -202,8 +202,8 @@ let Command = "breakpoint set" in {
 Desc<"Move breakpoints to nearest code. If not set the "
 "target.move-to-nearest-codesetting is used.">;
   def breakpoint_set_file_colon_line : Option<"joint-specifier", "y">, 
Group<12>, Arg<"FileLineColumn">,
-Required, Completion<"SourceFile">, 
-Desc<"A specifier in the form filename:line[:column] for setting file & 
line breakpoints.">;  
+Required, Completion<"SourceFile">,
+Desc<"A specifier in the form filename:line[:column] for setting file & 
line breakpoints.">;
   /* Don't add this option till it actually does something useful...
   def breakpoint_set_exception_typename : Option<"exception-typename", "O">,
 Arg<"TypeName">, Desc<"The breakpoint will only stop if an "
@@ -324,7 +324,7 @@ let Command = "disassemble" in {
   def disassemble_options_pc : Option<"pc", "p">, Group<5>,
 Desc<"Disassemble around the current pc.">;
   def disassemble_options_line : Option<"line", "l">, Group<6>,
-Desc<"Disassemble the current frame's current source line instructions if"
+Desc<"Disassemble the current frame's current source line instructions if "
 "there is debug line table information, else disassemble around the pc.">;
   def disassemble_options_address : Option<"address", "a">, Group<7>,
 Arg<"AddressOrExpression">,
@@ -751,7 +751,7 @@ let Command = "source list" in {
   def source_list_reverse : Option<"reverse", "r">, Group<4>, Desc<"Reverse 
the"
 " listing to look backwards from the last displayed block of source.">;
   def source_list_file_colon_line : Option<"joint-specifier", "y">, Group<5>,
-Arg<"FileLineColumn">, Completion<"SourceFile">, 
+Arg<"FileLineColumn">, Completion<"SourceFile">,
 Desc<"A specifier in the form filename:line[:column] from which to display"
  " source.">;
 }



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

[Lldb-commits] [PATCH] D86752: [lldb/test] Use shorter test case names in TestStandardUnwind

[Jordan Rupprecht via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8bd895cac0cd: [lldb/test] Use shorter test case names in 
TestStandardUnwind (authored by rupprecht).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86752

Files:
  lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py


Index: lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py
===
--- lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py
+++ lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py
@@ -164,7 +164,7 @@
 self.skipTest("Inferior not supported")
 self.standard_unwind_tests()
 
-test_name = "test_unwind_" + str(f)
+test_name = "test_unwind_" + str(os.path.basename(f))
 for c in ".=()/\\":
 test_name = test_name.replace(c, '_')
 


Index: lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py
===
--- lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py
+++ lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py
@@ -164,7 +164,7 @@
 self.skipTest("Inferior not supported")
 self.standard_unwind_tests()
 
-test_name = "test_unwind_" + str(f)
+test_name = "test_unwind_" + str(os.path.basename(f))
 for c in ".=()/\\":
 test_name = test_name.replace(c, '_')
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 8bd895c - [lldb/test] Use shorter test case names in TestStandardUnwind

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2020-08-28T11:49:50-07:00
New Revision: 8bd895cac0cd4eaf76b9bb296a995e5ee485205b

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

LOG: [lldb/test] Use shorter test case names in TestStandardUnwind

TestStandardUnwind uses the full absolute path to a set of C/C++ files as the 
test case name, which in turn is used in the name of a log file. When the 
source file is long, and the directory where log files are stored is also long, 
this causes an OSError because the log filename is too long.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py 
b/lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py
index 032b9e1aa618..fdf488978cbd 100644
--- a/lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py
+++ b/lldb/test/API/functionalities/unwind/standard/TestStandardUnwind.py
@@ -164,7 +164,7 @@ def test_function_dwarf(self, f=f):
 self.skipTest("Inferior not supported")
 self.standard_unwind_tests()
 
-test_name = "test_unwind_" + str(f)
+test_name = "test_unwind_" + str(os.path.basename(f))
 for c in ".=()/\\":
 test_name = test_name.replace(c, '_')
 



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

[Lldb-commits] [PATCH] D86817: [lldb] Get rid of LLDB_LIB_DIR and LLDB_IMPLIB_DIR in dotest

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: rupprecht, labath, mgorny.
Herald added a subscriber: danielkiss.
JDevlieghere requested review of this revision.

This patch removes the very confusing LLDB_LIB_DIR and LLDB_IMPLIB_DIR 
environment variables. They are confusing because LLDB_LIB_DIR would point to 
the `bin` subdirectory in the build root while LLDB_IMPLIB_DIR would point to 
the `lib` subdirectory. The reason far that was LLDB.framework, which gets 
build under `bin`. This patch replaces their uses with 
`configuration.lldb_framework_path` and `configuration.lldb_libs_dir` 
respectively.


https://reviews.llvm.org/D86817

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
  lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py

Index: lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py
===
--- lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py
+++ lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py
@@ -30,9 +30,7 @@
 """Test 'mpx-table show' command"""
 self.build()
 
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
+plugin_file = os.path.join(configuration.lldb_libs_dir, "liblldbIntelFeatures.so")
 if not os.path.isfile(plugin_file):
 self.skipTest("features plugin missing.")
 plugin_command = " "
@@ -122,9 +120,7 @@
 """Test 'mpx-table set' command"""
 self.build()
 
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
+plugin_file = os.path.join(configuration.lldb_libs_dir, "liblldbIntelFeatures.so")
 if not os.path.isfile(plugin_file):
 self.skipTest("features plugin missing.")
 plugin_command = " "
Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -21,7 +21,7 @@
 if 'intel-pt' not in configuration.enabled_plugins:
 self.skipTest("The intel-pt test plugin is not enabled")
 
-plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], "liblldbIntelFeatures.so")
+plugin_path = os.path.join(configuration.lldb_libs_dir, "liblldbIntelFeatures.so")
 self.runCmd("plugin load " + plugin_path)
 
 @skipIf(oslist=no_match(['linux']))
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -916,28 +916,18 @@
 self.setPlatformWorkingDir()
 self.enableLogChannelsForCurrentTest()
 
-lib_dir = os.environ["LLDB_LIB_DIR"]
-self.dsym = None
+self.lib_lldb = None
 self.framework_dir = None
-self.darwinWithFramework = self.platformIsDarwin()
-if sys.platform.startswith("darwin"):
-# Handle the framework environment variable if it is set
-if hasattr(lldbtest_config, 'lldb_framework_path'):
-framework_path = lldbtest_config.lldb_framework_path
-# Framework dir should be the directory containing the framework
-self.framework_dir = framework_path[:framework_path.rfind('LLDB.framework')]
-# If a framework dir was not specified assume the Xcode build
-# directory layout where the framework is in LLDB_LIB_DIR.
-else:
-self.framework_dir = lib_dir
-self.dsym = os.path.join(self.framework_dir, 'LLDB.framework', 'LLDB')
-# If the framework binary doesn't exist, assume we didn't actually
-# build a framework, and fallback to standard *nix behavior by
-# setting framework_dir and dsym to None.
-if not os.path.exists(self.dsym):
-self.framework_dir = None
-self.dsym = None
-self.darwinWithFramework = False
+self.darwinWithFramework = False
+
+if sys.platform.startswith("darwin") and configuration.lldb_framework_path:
+framework = configuration.lldb_framework_path
+lib = os.path.join(framework, 'LLDB')
+if os.path.exists(lib):
+self.framework_dir = os.path.dirname(framework)
+self.lib_lldb = lib
+

[Lldb-commits] [lldb] 031554e - Reland "[test] Exit with an error if no tests are run."

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2020-08-28T14:27:37-07:00
New Revision: 031554ed46c8540e8efdf5dde4c10ddd156ac1fa

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

LOG: Reland "[test] Exit with an error if no tests are run."

This reverts commit a06c28df3e8c85ceb665d3d9a1ebc2853dfd87a9 (reland 
adb5c23f8c0d60eeec41dcbe21d1b26184e1c97d).

The issue with PExpect tests on Windows should be fixed with 
e5e05ecf65aba836802154279efbc8cbce6fe2ea.

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 870b85ef4c4b..c96840024b8a 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -1019,6 +1019,10 @@ def run_suite():
 (configuration.suite.countTestCases(),
  configuration.suite.countTestCases() != 1 and "s" or ""))
 
+if configuration.suite.countTestCases() == 0:
+logging.error("did not discover any matching tests")
+exitTestSuite(1)
+
 # Invoke the test runner.
 if configuration.count == 1:
 result = unittest2.TextTestRunner(



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

[Lldb-commits] [PATCH] D86817: [lldb] Get rid of LLDB_LIB_DIR and LLDB_IMPLIB_DIR in dotest

[Jonas Devlieghere via Phabricator via lldb-commits]

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

- Fix LLDB.framework path
- Don't try to guess the LLDB.framework location


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

https://reviews.llvm.org/D86817

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
  lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py

Index: lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py
===
--- lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py
+++ lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py
@@ -30,9 +30,7 @@
 """Test 'mpx-table show' command"""
 self.build()
 
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
+plugin_file = os.path.join(configuration.lldb_libs_dir, "liblldbIntelFeatures.so")
 if not os.path.isfile(plugin_file):
 self.skipTest("features plugin missing.")
 plugin_command = " "
@@ -122,9 +120,7 @@
 """Test 'mpx-table set' command"""
 self.build()
 
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
+plugin_file = os.path.join(configuration.lldb_libs_dir, "liblldbIntelFeatures.so")
 if not os.path.isfile(plugin_file):
 self.skipTest("features plugin missing.")
 plugin_command = " "
Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -21,7 +21,7 @@
 if 'intel-pt' not in configuration.enabled_plugins:
 self.skipTest("The intel-pt test plugin is not enabled")
 
-plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], "liblldbIntelFeatures.so")
+plugin_path = os.path.join(configuration.lldb_libs_dir, "liblldbIntelFeatures.so")
 self.runCmd("plugin load " + plugin_path)
 
 @skipIf(oslist=no_match(['linux']))
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -916,28 +916,18 @@
 self.setPlatformWorkingDir()
 self.enableLogChannelsForCurrentTest()
 
-lib_dir = os.environ["LLDB_LIB_DIR"]
-self.dsym = None
+self.lib_lldb = None
 self.framework_dir = None
-self.darwinWithFramework = self.platformIsDarwin()
-if sys.platform.startswith("darwin"):
-# Handle the framework environment variable if it is set
-if hasattr(lldbtest_config, 'lldb_framework_path'):
-framework_path = lldbtest_config.lldb_framework_path
-# Framework dir should be the directory containing the framework
-self.framework_dir = framework_path[:framework_path.rfind('LLDB.framework')]
-# If a framework dir was not specified assume the Xcode build
-# directory layout where the framework is in LLDB_LIB_DIR.
-else:
-self.framework_dir = lib_dir
-self.dsym = os.path.join(self.framework_dir, 'LLDB.framework', 'LLDB')
-# If the framework binary doesn't exist, assume we didn't actually
-# build a framework, and fallback to standard *nix behavior by
-# setting framework_dir and dsym to None.
-if not os.path.exists(self.dsym):
-self.framework_dir = None
-self.dsym = None
-self.darwinWithFramework = False
+self.darwinWithFramework = False
+
+if sys.platform.startswith("darwin") and configuration.lldb_framework_path:
+framework = configuration.lldb_framework_path
+lib = os.path.join(framework, 'LLDB')
+if os.path.exists(lib):
+self.framework_dir = os.path.dirname(framework)
+self.lib_lldb = lib
+self.darwinWithFramework = self.platformIsDarwin()
+
 self.makeBuildDir()
 
 def setAsync(self, value):
@@ -1499,7 +1489,7 @@
  'EXE': exe_name,
  'CFLAGS_EXTRAS': "%s %s" % (stdflag, stdlibflag),
  'FRAMEWORK_INCLUDES': "-F%s" % self.framework_dir,
- 'LD_EXTRAS': "%s -Wl,-rpath,%s" % (self.dsym, self.framework_

[Lldb-commits] [PATCH] D86818: [lldb/Host] Add missing proc states

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht created this revision.
Herald added subscribers: lldb-commits, danielkiss.
Herald added a project: LLDB.
rupprecht requested review of this revision.
Herald added a subscriber: JDevlieghere.

The /proc//status parsing is missing a few cases:

- Idle
- Parked
- Dead

If we encounter an unknown proc state, this leads to an msan warning. In 
reality, we only check that the state != Zombie, so it doesn't really matter 
that we handle all cases, but handle them anyway (current list: [1]). Also 
explicitly set it to unknown if we encounter an unknown state. There will still 
be an msan warning if the proc entry has no `State:` line, but that should not 
happen.

Use a StringSwitch to make the handling of proc states a little more compact.

[1] https://github.com/torvalds/linux/blob/master/fs/proc/array.c


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86818

Files:
  lldb/source/Host/linux/Host.cpp


Index: lldb/source/Host/linux/Host.cpp
===
--- lldb/source/Host/linux/Host.cpp
+++ lldb/source/Host/linux/Host.cpp
@@ -16,6 +16,7 @@
 #include 
 #include 
 
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/Object/ELF.h"
 #include "llvm/Support/ScopedPrinter.h"
 
@@ -35,8 +36,11 @@
 namespace {
 enum class ProcessState {
   Unknown,
+  Dead,
   DiskSleep,
+  Idle,
   Paging,
+  Parked,
   Running,
   Sleeping,
   TracedOrStopped,
@@ -50,12 +54,14 @@
 
 static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo &ProcessInfo,
   ProcessState &State, ::pid_t &TracerPid) {
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+
   auto BufferOrError = getProcFile(Pid, "status");
   if (!BufferOrError)
 return false;
 
   llvm::StringRef Rest = BufferOrError.get()->getBuffer();
-  while(!Rest.empty()) {
+  while (!Rest.empty()) {
 llvm::StringRef Line;
 std::tie(Line, Rest) = Rest.split('\n');
 
@@ -84,26 +90,19 @@
   Line.ltrim().consumeInteger(10, PPid);
   ProcessInfo.SetParentProcessID(PPid);
 } else if (Line.consume_front("State:")) {
-  char S = Line.ltrim().front();
-  switch (S) {
-  case 'R':
-State = ProcessState::Running;
-break;
-  case 'S':
-State = ProcessState::Sleeping;
-break;
-  case 'D':
-State = ProcessState::DiskSleep;
-break;
-  case 'Z':
-State = ProcessState::Zombie;
-break;
-  case 'T':
-State = ProcessState::TracedOrStopped;
-break;
-  case 'W':
-State = ProcessState::Paging;
-break;
+  State = llvm::StringSwitch(Line.ltrim().take_front(1))
+  .Case("D", ProcessState::DiskSleep)
+  .Case("I", ProcessState::Idle)
+  .Case("R", ProcessState::Running)
+  .Case("S", ProcessState::Sleeping)
+  .CaseLower("T", ProcessState::TracedOrStopped)
+  .Case("W", ProcessState::Paging)
+  .Case("P", ProcessState::Parked)
+  .Case("X", ProcessState::Dead)
+  .Case("Z", ProcessState::Zombie)
+  .Default(ProcessState::Unknown);
+  if (State == ProcessState::Unknown) {
+LLDB_LOG(log, "Unknown process state {0}", Line);
   }
 } else if (Line.consume_front("TracerPid:")) {
   Line = Line.ltrim();


Index: lldb/source/Host/linux/Host.cpp
===
--- lldb/source/Host/linux/Host.cpp
+++ lldb/source/Host/linux/Host.cpp
@@ -16,6 +16,7 @@
 #include 
 #include 
 
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/Object/ELF.h"
 #include "llvm/Support/ScopedPrinter.h"
 
@@ -35,8 +36,11 @@
 namespace {
 enum class ProcessState {
   Unknown,
+  Dead,
   DiskSleep,
+  Idle,
   Paging,
+  Parked,
   Running,
   Sleeping,
   TracedOrStopped,
@@ -50,12 +54,14 @@
 
 static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo &ProcessInfo,
   ProcessState &State, ::pid_t &TracerPid) {
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
+
   auto BufferOrError = getProcFile(Pid, "status");
   if (!BufferOrError)
 return false;
 
   llvm::StringRef Rest = BufferOrError.get()->getBuffer();
-  while(!Rest.empty()) {
+  while (!Rest.empty()) {
 llvm::StringRef Line;
 std::tie(Line, Rest) = Rest.split('\n');
 
@@ -84,26 +90,19 @@
   Line.ltrim().consumeInteger(10, PPid);
   ProcessInfo.SetParentProcessID(PPid);
 } else if (Line.consume_front("State:")) {
-  char S = Line.ltrim().front();
-  switch (S) {
-  case 'R':
-State = ProcessState::Running;
-break;
-  case 'S':
-State = ProcessState::Sleeping;
-break;
-  case 'D':
-State = ProcessState::DiskSleep;
-break;
-  case 'Z':
-State = ProcessState::Zombie;
-break;
-  case 'T':
-State = ProcessState::TracedOrStopped;
-

[Lldb-commits] [PATCH] D86821: [lldb] Make the majority of the lit configuration values optional for the API tests

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: rupprecht, labath.
Herald added subscribers: danielkiss, teemperor.
JDevlieghere requested review of this revision.

LIT uses a model where the test suite is configurable trough a `lit.site.cfg` 
file. Most of the time we use the `lit.site.cfg` which has a bunch of default 
values, generated by CMake, that match the current build configuration. 
However, nothing prevents you from running the test suite with a different 
configuration, either by overriding some of these values from the command line, 
or by passing a different  `lit.site.cfg`.

Many of those configurationvalues are optional but they still need to be set 
because `lit.cfg.py` is accessing them directly. This patch changes the code to 
use `getattr` to return the attribute if it exists. This makes it possible to 
specify a minimal `lit.site.cfg` with only the mandatory/desired configuration 
values.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D86821

Files:
  lldb/test/API/lit.cfg.py

Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -23,14 +23,14 @@
 
 
 def mkdir_p(path):
-import errno
-try:
-os.makedirs(path)
-except OSError as e:
-if e.errno != errno.EEXIST:
-raise
-if not os.path.isdir(path):
-raise OSError(errno.ENOTDIR, "%s is not a directory"%path)
+  import errno
+  try:
+os.makedirs(path)
+  except OSError as e:
+if e.errno != errno.EEXIST:
+  raise
+  if not os.path.isdir(path):
+raise OSError(errno.ENOTDIR, "%s is not a directory"%path)
 
 
 def find_sanitizer_runtime(name):
@@ -85,22 +85,39 @@
   return copied_python
 
 
-if 'Address' in config.llvm_use_sanitizer:
-  config.environment['ASAN_OPTIONS'] = 'detect_stack_use_after_return=1'
-  if 'Darwin' in config.host_os and 'x86' in config.host_triple:
-config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
-'libclang_rt.asan_osx_dynamic.dylib')
+def is_configured(attr):
+  """Return the configuration attribute if it exists and None otherwise.
+
+  This allows us to check if the attribute exists before trying to access it."""
+  return getattr(config, attr, None)
+
+
+def delete_module_cache(path):
+  """Clean the module caches in the test build directory.
+
+  This is necessary in an incremental build whenever clang changes underneath,
+  so doing it once per lit.py invocation is close enough. """
+  if os.path.isdir(path):
+print("Deleting module cache at %s." % path)
+shutil.rmtree(path)
+
+if is_configured('llvm_use_sanitizer'):
+  if 'Address' in config.llvm_use_sanitizer:
+config.environment['ASAN_OPTIONS'] = 'detect_stack_use_after_return=1'
+if 'Darwin' in config.host_os and 'x86' in config.host_triple:
+  config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
+  'libclang_rt.asan_osx_dynamic.dylib')
 
-if 'Thread' in config.llvm_use_sanitizer:
-  if 'Darwin' in config.host_os and 'x86' in config.host_triple:
-config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
-'libclang_rt.tsan_osx_dynamic.dylib')
+  if 'Thread' in config.llvm_use_sanitizer:
+if 'Darwin' in config.host_os and 'x86' in config.host_triple:
+  config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
+  'libclang_rt.tsan_osx_dynamic.dylib')
 
 if 'DYLD_INSERT_LIBRARIES' in config.environment and platform.system() == 'Darwin':
   config.python_executable = find_python_interpreter()
 
 # Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
-if config.shared_libs:
+if is_configured('shared_libs'):
   for shlibpath_var in find_shlibpath_var():
 # In stand-alone build llvm_shlib_dir specifies LLDB's lib directory while
 # llvm_libs_dir specifies LLVM's lib directory.
@@ -129,14 +146,6 @@
   elif lldb_repro_mode == 'replay':
 config.available_features.add('lldb-repro-replay')
 
-# Clean the module caches in the test build directory. This is necessary in an
-# incremental build whenever clang changes underneath, so doing it once per
-# lit.py invocation is close enough.
-for cachedir in [config.clang_module_cache, config.lldb_module_cache]:
-  if os.path.isdir(cachedir):
-print("Deleting module cache at %s." % cachedir)
-shutil.rmtree(cachedir)
-
 # Set a default per-test timeout of 10 minutes. Setting a timeout per test
 # requires that killProcessAndChildren() is supported on the platform and
 # lit complains if the value is set but it is not supported.
@@ -148,11 +157,12 @@
 
 # Build dotest command.
 dotest_cmd = [config.dotest_path]
-dotest_cmd += ['--arch', config.test_arch]
-dotest_cmd.extend(config.dotest_args_str.split(';'))
+
+if is_configured('dotest_args_str'):
+  dotest_cmd.extend(config.dotest_args_str.split(';'))
 
 # Library path may be needed to locate just-built clang.
-if config.llvm

[Lldb-commits] [lldb] 141c847 - [lldb] Get rid of LLDB_LIB_DIR and LLDB_IMPLIB_DIR in dotest

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-08-28T15:45:54-07:00
New Revision: 141c8475b693e245388cf7a4ac9ec17303988700

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

LOG: [lldb] Get rid of LLDB_LIB_DIR and LLDB_IMPLIB_DIR in dotest

This patch removes the rather confusing LLDB_LIB_DIR and LLDB_IMPLIB_DIR
environment variables. They are confusing because LLDB_LIB_DIR would
point to the bin subdirectory in the build root while LLDB_IMPLIB_DIR
would point to the lib subdirectory. The reason far this was
LLDB.framework, which gets build under bin.

This patch replaces their uses with configuration.lldb_framework_path
and configuration.lldb_libs_dir respectively.

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index 85f346d73283..4e47165cdb1f 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -510,10 +510,9 @@ def are_sb_headers_missing():
 if lldb.remote_platform:
 return "skip because SBHeaders tests make no sense remotely"
 
-if lldbplatformutil.getHostPlatform() == 'darwin':
+if lldbplatformutil.getHostPlatform() == 'darwin' and 
configuration.lldb_framework_path:
 header = os.path.join(
-os.environ["LLDB_LIB_DIR"],
-'LLDB.framework',
+configuration.lldb_framework_path,
 'Versions',
 'Current',
 'Headers',

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index c96840024b8a..30d6afc231fd 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -519,13 +519,6 @@ def setupSysPath():
 print("The 'lldb' executable cannot be located.  Some of the tests may 
not be run as a result.")
 sys.exit(-1)
 
-# confusingly, this is the "bin" directory
-lldbLibDir = os.path.dirname(lldbtest_config.lldbExec)
-os.environ["LLDB_LIB_DIR"] = lldbLibDir
-lldbImpLibDir = configuration.lldb_libs_dir
-os.environ["LLDB_IMPLIB_DIR"] = lldbImpLibDir
-print("LLDB library dir:", os.environ["LLDB_LIB_DIR"])
-print("LLDB import library dir:", os.environ["LLDB_IMPLIB_DIR"])
 os.system('%s -v' % lldbtest_config.lldbExec)
 
 lldbDir = os.path.dirname(lldbtest_config.lldbExec)
@@ -540,8 +533,6 @@ def setupSysPath():
 configuration.skip_categories.append("lldb-vscode")
 
 lldbPythonDir = None  # The directory that contains 'lldb/__init__.py'
-if not configuration.lldb_framework_path and 
os.path.exists(os.path.join(lldbLibDir, "LLDB.framework")):
-configuration.lldb_framework_path = os.path.join(lldbLibDir, 
"LLDB.framework")
 if configuration.lldb_framework_path:
 lldbtest_config.lldb_framework_path = configuration.lldb_framework_path
 candidatePath = os.path.join(

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 4180ba271613..112b19abab39 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -916,28 +916,18 @@ def setUp(self):
 self.setPlatformWorkingDir()
 self.enableLogChannelsForCurrentTest()
 
-lib_dir = os.environ["LLDB_LIB_DIR"]
-self.dsym = None
+self.lib_lldb = None
 self.framework_dir = None
-self.darwinWithFramework = self.platformIsDarwin()
-if sys.platform.startswith("darwin"):
-# Handle the framework environment variable if it is set
-if hasattr(lldbtest_config, 'lldb_framework_path'):
-framework_path = lldbtest_config.lldb_framework_path
-# Framework dir should be the directory containing the 
framework
-self.framework_dir = 
framework_path[:framework_path.rfind('LLDB.framework')]
-# If a framework dir was not specified assume the Xcode build
-# directory layout where the framework is in LLDB_LIB_DIR.
-else:
-self.framework_dir = lib_dir
-self.dsym = os.path.join(self.framework_dir, 'LLDB.framework', 
'LLDB')
-# If the framework binary doesn't exist, assume we didn't actually
-# b

[Lldb-commits] [PATCH] D86817: [lldb] Get rid of LLDB_LIB_DIR and LLDB_IMPLIB_DIR in dotest

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG141c8475b693: [lldb] Get rid of LLDB_LIB_DIR and 
LLDB_IMPLIB_DIR in dotest (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86817

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
  lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py

Index: lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py
===
--- lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py
+++ lldb/tools/intel-features/intel-mpx/test/TestMPXTable.py
@@ -30,9 +30,7 @@
 """Test 'mpx-table show' command"""
 self.build()
 
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
+plugin_file = os.path.join(configuration.lldb_libs_dir, "liblldbIntelFeatures.so")
 if not os.path.isfile(plugin_file):
 self.skipTest("features plugin missing.")
 plugin_command = " "
@@ -122,9 +120,7 @@
 """Test 'mpx-table set' command"""
 self.build()
 
-lldb_exec_dir = os.environ["LLDB_IMPLIB_DIR"]
-lldb_lib_dir = os.path.join(lldb_exec_dir, os.pardir, "lib")
-plugin_file = os.path.join(lldb_lib_dir, "liblldbIntelFeatures.so")
+plugin_file = os.path.join(configuration.lldb_libs_dir, "liblldbIntelFeatures.so")
 if not os.path.isfile(plugin_file):
 self.skipTest("features plugin missing.")
 plugin_command = " "
Index: lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
===
--- lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
+++ lldb/test/API/tools/intel-features/intel-pt/test/TestIntelPTSimpleBinary.py
@@ -21,7 +21,7 @@
 if 'intel-pt' not in configuration.enabled_plugins:
 self.skipTest("The intel-pt test plugin is not enabled")
 
-plugin_path = os.path.join(os.environ["LLDB_IMPLIB_DIR"], "liblldbIntelFeatures.so")
+plugin_path = os.path.join(configuration.lldb_libs_dir, "liblldbIntelFeatures.so")
 self.runCmd("plugin load " + plugin_path)
 
 @skipIf(oslist=no_match(['linux']))
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -916,28 +916,18 @@
 self.setPlatformWorkingDir()
 self.enableLogChannelsForCurrentTest()
 
-lib_dir = os.environ["LLDB_LIB_DIR"]
-self.dsym = None
+self.lib_lldb = None
 self.framework_dir = None
-self.darwinWithFramework = self.platformIsDarwin()
-if sys.platform.startswith("darwin"):
-# Handle the framework environment variable if it is set
-if hasattr(lldbtest_config, 'lldb_framework_path'):
-framework_path = lldbtest_config.lldb_framework_path
-# Framework dir should be the directory containing the framework
-self.framework_dir = framework_path[:framework_path.rfind('LLDB.framework')]
-# If a framework dir was not specified assume the Xcode build
-# directory layout where the framework is in LLDB_LIB_DIR.
-else:
-self.framework_dir = lib_dir
-self.dsym = os.path.join(self.framework_dir, 'LLDB.framework', 'LLDB')
-# If the framework binary doesn't exist, assume we didn't actually
-# build a framework, and fallback to standard *nix behavior by
-# setting framework_dir and dsym to None.
-if not os.path.exists(self.dsym):
-self.framework_dir = None
-self.dsym = None
-self.darwinWithFramework = False
+self.darwinWithFramework = False
+
+if sys.platform.startswith("darwin") and configuration.lldb_framework_path:
+framework = configuration.lldb_framework_path
+lib = os.path.join(framework, 'LLDB')
+if os.path.exists(lib):
+self.framework_dir = os.path.dirname(framework)
+self.lib_lldb = lib
+self.darwinWithFramework = self.platformIsDarwin()
+
 self.makeBuildDir()
 
 def setAsync(self, value):
@@ -1499,7 +1489,7 @@
  'EXE': exe_name,
  'CFLAGS_EXTRAS': "%s %s" % (stdflag, stdlibflag),
  'FRAMEWORK_INCLUDES': 

[Lldb-commits] [lldb] 55e7d91 - [lldb] Dervice dotest.py path from config.lldb_src_root (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-08-28T15:45:54-07:00
New Revision: 55e7d91072e865d36953e91a7b2c8bfc219464d6

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

LOG: [lldb] Dervice dotest.py path from config.lldb_src_root (NFC)

Added: 


Modified: 
lldb/test/API/lit.cfg.py
lldb/test/API/lit.site.cfg.py.in

Removed: 




diff  --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index e083e2fd9beb..238df53b69a5 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -147,7 +147,7 @@ def find_python_interpreter():
   lit_config.warning("Could not set a default per-test timeout. " + errormsg)
 
 # Build dotest command.
-dotest_cmd = [config.dotest_path]
+dotest_cmd = [os.path.join(config.lldb_src_root, 'test', 'API', 'dotest.py')]
 dotest_cmd += ['--arch', config.test_arch]
 dotest_cmd.extend(config.dotest_args_str.split(';'))
 

diff  --git a/lldb/test/API/lit.site.cfg.py.in 
b/lldb/test/API/lit.site.cfg.py.in
index 3a108ae5c85a..b746b22a84e1 100644
--- a/lldb/test/API/lit.site.cfg.py.in
+++ b/lldb/test/API/lit.site.cfg.py.in
@@ -20,7 +20,6 @@ config.target_triple = "@TARGET_TRIPLE@"
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
 config.lldb_reproducer_directory = os.path.join("@LLDB_TEST_BUILD_DIRECTORY@", 
"reproducers")
 config.python_executable = "@Python3_EXECUTABLE@"
-config.dotest_path = "@LLDB_SOURCE_DIR@/test/API/dotest.py"
 config.dotest_args_str = "@LLDB_DOTEST_ARGS@"
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.dotest_lit_args_str = None



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

[Lldb-commits] [PATCH] D86825: [lldb/Gui] zero-initialize children_stop_id

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht created this revision.
Herald added subscribers: lldb-commits, danielkiss.
Herald added a project: LLDB.
rupprecht requested review of this revision.
Herald added a subscriber: JDevlieghere.

This is currently causing msan warnings in the API tests when run under msan, 
e.g. `commands/gui/basic/TestGuiBasic.py`.

I'm not sure if 0 is the right initial value, but the tests pass with it (and 
msan doesn't complain anymore).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86825

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1507,8 +1507,9 @@
   std::vector children;
 
   Row(const ValueObjectSP &v, Row *p)
-  : value(v, lldb::eDynamicDontRunTarget, true), parent(p), row_idx(0),
-x(1), y(1), might_have_children(v ? v->MightHaveChildren() : false),
+  : value(v, lldb::eDynamicDontRunTarget, true), parent(p),
+children_stop_id(0), row_idx(0), x(1), y(1),
+might_have_children(v ? v->MightHaveChildren() : false),
 expanded(false), calculated_children(false), children() {}
 
   size_t GetDepth() const {


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1507,8 +1507,9 @@
   std::vector children;
 
   Row(const ValueObjectSP &v, Row *p)
-  : value(v, lldb::eDynamicDontRunTarget, true), parent(p), row_idx(0),
-x(1), y(1), might_have_children(v ? v->MightHaveChildren() : false),
+  : value(v, lldb::eDynamicDontRunTarget, true), parent(p),
+children_stop_id(0), row_idx(0), x(1), y(1),
+might_have_children(v ? v->MightHaveChildren() : false),
 expanded(false), calculated_children(false), children() {}
 
   size_t GetDepth() const {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 288739.
wallace added a comment.
Herald added subscribers: llvm-commits, danielkiss, hiraditya.
Herald added a project: LLVM.

Addressed all comments.

- Wwitched to using llvm::json instead of StructuredData. Fortunately, the 
logic is pretty much the same.
- Moved the "triple" field to the process level


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Target/TraceSettingsParser.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/CommandObjectTrace.h
  lldb/source/Commands/Options.td
  lldb/source/Core/PluginManager.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/Trace/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceSettingsParser.cpp
  lldb/source/Utility/StructuredData.cpp
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/TestTraceSchema.py
  lldb/test/API/commands/trace/intelpt-trace/3842849.trace
  lldb/test/API/commands/trace/intelpt-trace/a.out
  lldb/test/API/commands/trace/intelpt-trace/main.cpp
  lldb/test/API/commands/trace/intelpt-trace/trace.json
  lldb/test/API/commands/trace/intelpt-trace/trace_bad.json
  lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
  lldb/test/API/commands/trace/intelpt-trace/trace_bad3.json
  llvm/include/llvm/Support/JSON.h
  llvm/lib/Support/JSON.cpp

Index: llvm/lib/Support/JSON.cpp
===
--- llvm/lib/Support/JSON.cpp
+++ llvm/lib/Support/JSON.cpp
@@ -77,6 +77,62 @@
 return V->getAsArray();
   return nullptr;
 }
+
+llvm::Error Object::createMissingKeyError(llvm::StringRef K) const {
+  std::string str;
+  llvm::raw_string_ostream OS(str);
+  json::Object obj = *this;
+  OS << llvm::formatv(
+  "JSON object is missing the \"{0}\" field.\nValue::\n{1:2}", K,
+  json::Value(std::move(obj)));
+  OS.flush();
+
+  return llvm::createStringError(std::errc::invalid_argument, OS.str().c_str());
+}
+
+llvm::Expected Object::getOrError(StringRef K) const {
+  if (const json::Value *value = get(K))
+return value;
+  else
+return createMissingKeyError(K);
+}
+
+llvm::Expected Object::getIntegerOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsIntegerOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected Object::getStringOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsStringOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected Object::getArrayOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsArrayOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected
+Object::getObjectOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsObjectOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected>
+Object::getOptionalStringOrError(StringRef K) const {
+  if (const json::Value *V = get(K))
+return V->getAsStringOrError();
+  return llvm::None;
+}
+
 bool operator==(const Object &LHS, const Object &RHS) {
   if (LHS.size() != RHS.size())
 return false;
@@ -150,6 +206,42 @@
   }
 }
 
+llvm::Error Value::createWrongTypeError(llvm::StringRef type) const {
+  std::string str;
+  llvm::raw_string_ostream OS(str);
+  OS << llvm::formatv("JSON value is expected to be \"{0}\".\nValue:\n{1:2}",
+  type, *this);
+  OS.flush();
+
+  return llvm::createStringError(std::errc::invalid_argument, OS.str().c_str());
+}
+
+llvm::Expected Value::getAsIntegerOrError() const {
+  llvm::Optional value = getAsInteger();
+  if (value.hasValue())
+return *value;
+  return createWrongTypeError("integer");
+}
+
+llvm::Expected Value::getAsStringOrError() const {
+  llvm::Optional value = getAsString();
+  if (value.hasValue())
+return *value;
+  return createWrongTypeError("string");
+}
+
+llvm::Expected Value::getAsArrayOrError() const {
+  if (const json::Array *value = getAsArray())
+return value;
+  return createWrongTypeError("array");
+}
+
+llvm::Expected Value::getAsObjectOrError() const {
+  if (const json::Object *value = getAsObject())
+return value;
+  return createWrongTypeError("object");
+}
+
 void Value::destroy() {
   switch (Type) {
   case T_Null:

[Lldb-commits] [PATCH] D86821: [lldb] Make the majority of the lit configuration values optional for the API tests

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3f2fb0132f7b: [lldb] Make the lit configuration values 
optional for the API tests (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D86821?vs=288713&id=288741#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86821

Files:
  lldb/test/API/lit.cfg.py
  lldb/test/API/lldbtest.py

Index: lldb/test/API/lldbtest.py
===
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -38,7 +38,7 @@
 if litConfig.noExecute:
 return lit.Test.PASS, ''
 
-if not test.config.lldb_enable_python:
+if not getattr(test.config, 'lldb_enable_python', False):
 return (lit.Test.UNSUPPORTED, 'Python module disabled')
 
 if test.config.unsupported:
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -23,14 +23,14 @@
 
 
 def mkdir_p(path):
-import errno
-try:
-os.makedirs(path)
-except OSError as e:
-if e.errno != errno.EEXIST:
-raise
-if not os.path.isdir(path):
-raise OSError(errno.ENOTDIR, "%s is not a directory"%path)
+  import errno
+  try:
+os.makedirs(path)
+  except OSError as e:
+if e.errno != errno.EEXIST:
+  raise
+  if not os.path.isdir(path):
+raise OSError(errno.ENOTDIR, "%s is not a directory"%path)
 
 
 def find_sanitizer_runtime(name):
@@ -85,22 +85,39 @@
   return copied_python
 
 
-if 'Address' in config.llvm_use_sanitizer:
-  config.environment['ASAN_OPTIONS'] = 'detect_stack_use_after_return=1'
-  if 'Darwin' in config.host_os and 'x86' in config.host_triple:
-config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
-'libclang_rt.asan_osx_dynamic.dylib')
+def is_configured(attr):
+  """Return the configuration attribute if it exists and None otherwise.
+
+  This allows us to check if the attribute exists before trying to access it."""
+  return getattr(config, attr, None)
+
+
+def delete_module_cache(path):
+  """Clean the module caches in the test build directory.
+
+  This is necessary in an incremental build whenever clang changes underneath,
+  so doing it once per lit.py invocation is close enough. """
+  if os.path.isdir(path):
+print("Deleting module cache at %s." % path)
+shutil.rmtree(path)
 
-if 'Thread' in config.llvm_use_sanitizer:
-  if 'Darwin' in config.host_os and 'x86' in config.host_triple:
-config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
-'libclang_rt.tsan_osx_dynamic.dylib')
+if is_configured('llvm_use_sanitizer'):
+  if 'Address' in config.llvm_use_sanitizer:
+config.environment['ASAN_OPTIONS'] = 'detect_stack_use_after_return=1'
+if 'Darwin' in config.host_os and 'x86' in config.host_triple:
+  config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
+  'libclang_rt.asan_osx_dynamic.dylib')
+
+  if 'Thread' in config.llvm_use_sanitizer:
+if 'Darwin' in config.host_os and 'x86' in config.host_triple:
+  config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
+  'libclang_rt.tsan_osx_dynamic.dylib')
 
 if 'DYLD_INSERT_LIBRARIES' in config.environment and platform.system() == 'Darwin':
   config.python_executable = find_python_interpreter()
 
 # Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
-if config.shared_libs:
+if is_configured('shared_libs'):
   for shlibpath_var in find_shlibpath_var():
 # In stand-alone build llvm_shlib_dir specifies LLDB's lib directory while
 # llvm_libs_dir specifies LLVM's lib directory.
@@ -129,14 +146,6 @@
   elif lldb_repro_mode == 'replay':
 config.available_features.add('lldb-repro-replay')
 
-# Clean the module caches in the test build directory. This is necessary in an
-# incremental build whenever clang changes underneath, so doing it once per
-# lit.py invocation is close enough.
-for cachedir in [config.clang_module_cache, config.lldb_module_cache]:
-  if os.path.isdir(cachedir):
-print("Deleting module cache at %s." % cachedir)
-shutil.rmtree(cachedir)
-
 # Set a default per-test timeout of 10 minutes. Setting a timeout per test
 # requires that killProcessAndChildren() is supported on the platform and
 # lit complains if the value is set but it is not supported.
@@ -148,11 +157,12 @@
 
 # Build dotest command.
 dotest_cmd = [os.path.join(config.lldb_src_root, 'test', 'API', 'dotest.py')]
-dotest_cmd += ['--arch', config.test_arch]
-dotest_cmd.extend(config.dotest_args_str.split(';'))
+
+if is_configured('dotest_args_str'):
+  dotest_cmd.extend(config.dotest_args_str.split(';'))
 
 # Library path may be needed to locate just-built clang.
-if config.ll

[Lldb-commits] [lldb] 3f2fb01 - [lldb] Make the lit configuration values optional for the API tests

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-08-28T18:08:22-07:00
New Revision: 3f2fb0132f7b09e1309e8f7e0b5ba8ea471b17e7

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

LOG: [lldb] Make the lit configuration values optional for the API tests

LIT uses a model where the test suite is configurable trough a
lit.site.cfg file. Most of the time we use the lit.site.cfg with values
that match the current build configuration, generated by CMake.

Nothing prevents you from running the test suite with a different
configuration, either by overriding some of these values from the
command line, or by passing a different lit.site.cfg.

The latter is currently tedious. Many configuration values are optional
but they still need to be set because lit.cfg.py is accessing them
directly. This patch changes the code to use getattr to return the
attribute if it exists. This makes it possible to specify a minimal
lit.site.cfg with only the mandatory/desired configuration values.

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

Added: 


Modified: 
lldb/test/API/lit.cfg.py
lldb/test/API/lldbtest.py

Removed: 




diff  --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index 238df53b69a5..5da12eea4504 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -23,14 +23,14 @@
 
 
 def mkdir_p(path):
-import errno
-try:
-os.makedirs(path)
-except OSError as e:
-if e.errno != errno.EEXIST:
-raise
-if not os.path.isdir(path):
-raise OSError(errno.ENOTDIR, "%s is not a directory"%path)
+  import errno
+  try:
+os.makedirs(path)
+  except OSError as e:
+if e.errno != errno.EEXIST:
+  raise
+  if not os.path.isdir(path):
+raise OSError(errno.ENOTDIR, "%s is not a directory"%path)
 
 
 def find_sanitizer_runtime(name):
@@ -85,22 +85,39 @@ def find_python_interpreter():
   return copied_python
 
 
-if 'Address' in config.llvm_use_sanitizer:
-  config.environment['ASAN_OPTIONS'] = 'detect_stack_use_after_return=1'
-  if 'Darwin' in config.host_os and 'x86' in config.host_triple:
-config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
-'libclang_rt.asan_osx_dynamic.dylib')
+def is_configured(attr):
+  """Return the configuration attribute if it exists and None otherwise.
+
+  This allows us to check if the attribute exists before trying to access 
it."""
+  return getattr(config, attr, None)
+
+
+def delete_module_cache(path):
+  """Clean the module caches in the test build directory.
+
+  This is necessary in an incremental build whenever clang changes underneath,
+  so doing it once per lit.py invocation is close enough. """
+  if os.path.isdir(path):
+print("Deleting module cache at %s." % path)
+shutil.rmtree(path)
 
-if 'Thread' in config.llvm_use_sanitizer:
-  if 'Darwin' in config.host_os and 'x86' in config.host_triple:
-config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
-'libclang_rt.tsan_osx_dynamic.dylib')
+if is_configured('llvm_use_sanitizer'):
+  if 'Address' in config.llvm_use_sanitizer:
+config.environment['ASAN_OPTIONS'] = 'detect_stack_use_after_return=1'
+if 'Darwin' in config.host_os and 'x86' in config.host_triple:
+  config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
+  'libclang_rt.asan_osx_dynamic.dylib')
+
+  if 'Thread' in config.llvm_use_sanitizer:
+if 'Darwin' in config.host_os and 'x86' in config.host_triple:
+  config.environment['DYLD_INSERT_LIBRARIES'] = find_sanitizer_runtime(
+  'libclang_rt.tsan_osx_dynamic.dylib')
 
 if 'DYLD_INSERT_LIBRARIES' in config.environment and platform.system() == 
'Darwin':
   config.python_executable = find_python_interpreter()
 
 # Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
-if config.shared_libs:
+if is_configured('shared_libs'):
   for shlibpath_var in find_shlibpath_var():
 # In stand-alone build llvm_shlib_dir specifies LLDB's lib directory while
 # llvm_libs_dir specifies LLVM's lib directory.
@@ -129,14 +146,6 @@ def find_python_interpreter():
   elif lldb_repro_mode == 'replay':
 config.available_features.add('lldb-repro-replay')
 
-# Clean the module caches in the test build directory. This is necessary in an
-# incremental build whenever clang changes underneath, so doing it once per
-# lit.py invocation is close enough.
-for cachedir in [config.clang_module_cache, config.lldb_module_cache]:
-  if os.path.isdir(cachedir):
-print("Deleting module cache at %s." % cachedir)
-shutil.rmtree(cachedir)
-
 # Set a default per-test timeout of 10 minutes. Setting a timeout per test
 # requires that killProcessAndChildren() is supported on the platform and
 # lit complains if the value is

[Lldb-commits] [lldb] 2965e9b - [lldb] Hoist --framework argument out of LLDB_TEST_COMMON_ARGS (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-08-28T18:15:33-07:00
New Revision: 2965e9bd5edb079746a668794865be37f6f4d3d8

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

LOG: [lldb] Hoist --framework argument out of LLDB_TEST_COMMON_ARGS (NFC)

Give the framework argument its own variable (LLDB_FRAMEWORK_DIR) so
that we can configure it in lit.site.cfg.py if we so desire.

Added: 


Modified: 
lldb/test/API/CMakeLists.txt
lldb/test/API/lit.cfg.py
lldb/test/API/lit.site.cfg.py.in
lldb/utils/lldb-dotest/lldb-dotest.in

Removed: 




diff  --git a/lldb/test/API/CMakeLists.txt b/lldb/test/API/CMakeLists.txt
index 1cd705c56540..f07370adb5c2 100644
--- a/lldb/test/API/CMakeLists.txt
+++ b/lldb/test/API/CMakeLists.txt
@@ -98,7 +98,7 @@ endif()
 
 if(CMAKE_HOST_APPLE)
   if(LLDB_BUILD_FRAMEWORK)
-list(APPEND LLDB_TEST_COMMON_ARGS --framework 
${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework)
+set(LLDB_FRAMEWORK_DIR ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework)
   endif()
 
   # Use the same identity for testing

diff  --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index 5da12eea4504..893418cc16d3 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -203,6 +203,9 @@ def delete_module_cache(path):
 if is_configured('lldb_libs_dir'):
   dotest_cmd += ['--lldb-libs-dir', config.lldb_libs_dir]
 
+if is_configured('lldb_framework_dir'):
+  dotest_cmd += ['--framework', config.lldb_framework_dir]
+
 if 'lldb-repro-capture' in config.available_features or \
 'lldb-repro-replay' in config.available_features:
   dotest_cmd += ['--skip-category=lldb-vscode', '--skip-category=std-module']

diff  --git a/lldb/test/API/lit.site.cfg.py.in 
b/lldb/test/API/lit.site.cfg.py.in
index b746b22a84e1..6554d05d7df9 100644
--- a/lldb/test/API/lit.site.cfg.py.in
+++ b/lldb/test/API/lit.site.cfg.py.in
@@ -11,6 +11,7 @@ config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
 config.lldb_src_root = "@LLDB_SOURCE_DIR@"
 config.lldb_libs_dir = "@LLDB_LIBS_DIR@"
+config.lldb_framework_dir = "@LLDB_FRAMEWORK_DIR@"
 config.cmake_cxx_compiler = "@CMAKE_CXX_COMPILER@"
 config.host_os = "@HOST_OS@"
 config.host_triple = "@LLVM_HOST_TRIPLE@"

diff  --git a/lldb/utils/lldb-dotest/lldb-dotest.in 
b/lldb/utils/lldb-dotest/lldb-dotest.in
index f573d5bf28d4..86f05bea9bdc 100755
--- a/lldb/utils/lldb-dotest/lldb-dotest.in
+++ b/lldb/utils/lldb-dotest/lldb-dotest.in
@@ -12,6 +12,7 @@ dsymutil = '@LLDB_TEST_DSYMUTIL_CONFIGURED@'
 filecheck = '@LLDB_TEST_FILECHECK_CONFIGURED@'
 yaml2obj = '@LLDB_TEST_YAML2OBJ_CONFIGURED@'
 lldb_libs_dir = "@LLDB_LIBS_DIR_CONFIGURED@"
+lldb_framework_dir = "@LLDB_FRAMEWORK_DIR@"
 lldb_build_intel_pt = "@LLDB_BUILD_INTEL_PT@"
 
 if __name__ == '__main__':
@@ -28,6 +29,7 @@ if __name__ == '__main__':
 cmd.extend(['--yaml2obj', yaml2obj])
 cmd.extend(['--filecheck', filecheck])
 cmd.extend(['--lldb-libs-dir', lldb_libs_dir])
+cmd.extend(['--framework', lldb_framework_dir])
 if lldb_build_intel_pt == "1":
 cmd.extend(['--enable-plugin', 'intel-pt'])
 cmd.extend(wrapper_args)



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

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 288747.
wallace added a comment.

fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Target/TraceSettingsParser.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/CommandObjectTrace.h
  lldb/source/Commands/Options.td
  lldb/source/Core/PluginManager.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/Trace/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSettingsParser.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceSettingsParser.cpp
  lldb/source/Utility/StructuredData.cpp
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/TestTraceSchema.py
  lldb/test/API/commands/trace/intelpt-trace/3842849.trace
  lldb/test/API/commands/trace/intelpt-trace/a.out
  lldb/test/API/commands/trace/intelpt-trace/main.cpp
  lldb/test/API/commands/trace/intelpt-trace/trace.json
  lldb/test/API/commands/trace/intelpt-trace/trace_bad.json
  lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
  lldb/test/API/commands/trace/intelpt-trace/trace_bad3.json
  llvm/include/llvm/Support/JSON.h
  llvm/lib/Support/JSON.cpp

Index: llvm/lib/Support/JSON.cpp
===
--- llvm/lib/Support/JSON.cpp
+++ llvm/lib/Support/JSON.cpp
@@ -77,6 +77,62 @@
 return V->getAsArray();
   return nullptr;
 }
+
+llvm::Error Object::createMissingKeyError(llvm::StringRef K) const {
+  std::string str;
+  llvm::raw_string_ostream OS(str);
+  json::Object obj = *this;
+  OS << llvm::formatv(
+  "JSON object is missing the \"{0}\" field.\nValue::\n{1:2}", K,
+  json::Value(std::move(obj)));
+  OS.flush();
+
+  return llvm::createStringError(std::errc::invalid_argument, OS.str().c_str());
+}
+
+llvm::Expected Object::getOrError(StringRef K) const {
+  if (const json::Value *value = get(K))
+return value;
+  else
+return createMissingKeyError(K);
+}
+
+llvm::Expected Object::getIntegerOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsIntegerOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected Object::getStringOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsStringOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected Object::getArrayOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsArrayOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected
+Object::getObjectOrError(StringRef K) const {
+  if (llvm::Expected V = getOrError(K))
+return V.get()->getAsObjectOrError();
+  else
+return V.takeError();
+}
+
+llvm::Expected>
+Object::getOptionalStringOrError(StringRef K) const {
+  if (const json::Value *V = get(K))
+return V->getAsStringOrError();
+  return llvm::None;
+}
+
 bool operator==(const Object &LHS, const Object &RHS) {
   if (LHS.size() != RHS.size())
 return false;
@@ -150,6 +206,42 @@
   }
 }
 
+llvm::Error Value::createWrongTypeError(llvm::StringRef type) const {
+  std::string str;
+  llvm::raw_string_ostream OS(str);
+  OS << llvm::formatv("JSON value is expected to be \"{0}\".\nValue:\n{1:2}",
+  type, *this);
+  OS.flush();
+
+  return llvm::createStringError(std::errc::invalid_argument, OS.str().c_str());
+}
+
+llvm::Expected Value::getAsIntegerOrError() const {
+  llvm::Optional value = getAsInteger();
+  if (value.hasValue())
+return *value;
+  return createWrongTypeError("integer");
+}
+
+llvm::Expected Value::getAsStringOrError() const {
+  llvm::Optional value = getAsString();
+  if (value.hasValue())
+return *value;
+  return createWrongTypeError("string");
+}
+
+llvm::Expected Value::getAsArrayOrError() const {
+  if (const json::Array *value = getAsArray())
+return value;
+  return createWrongTypeError("array");
+}
+
+llvm::Expected Value::getAsObjectOrError() const {
+  if (const json::Object *value = getAsObject())
+return value;
+  return createWrongTypeError("object");
+}
+
 void Value::destroy() {
   switch (Type) {
   case T_Null:
@@ -715,4 +807,3 @@
 llvm_unreachable("json::Value format options should be an integer");
   json::OStream(OS, IndentAmount).value(E);
 }
-
Index: llvm/include/llvm/Support/JSON.h
===
--- llvm/in

[Lldb-commits] [PATCH] D86670: [intel-pt] Add a basic implementation of the dump command

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 288751.
wallace added a comment.
Herald added subscribers: dang, danielkiss.

Addressed comments

- Now the "dump" command specifically refers to the currently selected thread. 
That will be aligned with the future stepping commands we will be adding.
- The format of the command is "trace dump [-t tid]"
- The verbose option is used to dump the original JSON settings
- For now, I'm printing the trace file of each thread. Eventually we'll have 
more information about each thread.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86670

Files:
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/Trace.cpp
  lldb/test/API/commands/trace/TestTraceDump.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/intelpt-trace/trace2.json

Index: lldb/test/API/commands/trace/intelpt-trace/trace2.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace2.json
@@ -0,0 +1,53 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "processes": [
+{
+  "pid": 1,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 11,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+},
+{
+  "pid": 2,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 21,
+  "traceFile": "3842849.trace"
+},
+{
+  "tid": 22,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+}
+  ]
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -55,3 +55,5 @@
 self.expect("trace load -v " + trace_definition_file2, error=True,
 substrs=['error: JSON object is missing the "pid" field.'])
 self.assertEqual(self.dbg.GetNumTargets(), 0)
+
+self.expect("trace dump", substrs=["error: no trace data in this target"])
Index: lldb/test/API/commands/trace/TestTraceDump.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceDump.py
@@ -0,0 +1,44 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceDump(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+TestBase.setUp(self)
+if 'intel-pt' not in configuration.enabled_plugins:
+self.skipTest("The intel-pt test plugin is not enabled")
+
+
+def testDumpTrace(self):
+self.expect("trace dump", substrs=["error: no trace data in this target"])
+
+src_dir = self.getSourceDir()
+trace_definition_file = os.path.join(src_dir, "intelpt-trace", "trace2.json")
+self.expect("trace load " + trace_definition_file)
+
+# An invalid thread id should show an error message
+self.expect("trace dump -t 5678", substrs=['error: the thread id "5678" does not exist in this target'])
+
+# Only the specified thread should be printed
+self.expect("trace dump -t 22", substrs=["Trace files"],
+  patterns=["pid: '2', tid: '22' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+# Verbose should output the entire JSON settings besides the thread specific information
+self.expect("trace dump -t 22 -v",
+  substrs=["Settings", "3842849.trace", "intel-pt", "Settings directory"],
+  patterns=["pid: '2', tid: '22' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+# In this case all threads should be printed
+self.expect("trace dump", substrs=["Trace files"],
+  patterns=["pid: '2', tid: '21' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace",
+"pid: '2', tid: '22' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+# We switch targets and

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

A few simple changes from inlined comments. Other than that looks good. Should 
get another OK from others. Also not sure if the JSON stuff in llvm needs to be 
done separately?




Comment at: lldb/include/lldb/Target/Trace.h:115
+  ///   implementation, or an error object in case of failures.
+  virtual llvm::Expected>
+  ParsePluginSettings(Debugger &debugger) = 0;

would std::unique_ptr<> might be better? Are we actually handing out multiple 
instances of this and sharing it anywhere?



Comment at: lldb/include/lldb/lldb-private-interfaces.h:14
 
+#include "lldb/Utility/StructuredData.h"
 #include "lldb/lldb-enumerations.h"

We should avoid this import and use forward declaration here below the includes:

```
namespace llvm {
  namespace json {
class Object;
  }
}
```



Comment at: lldb/source/Commands/Options.td:205-206
   def breakpoint_set_file_colon_line : Option<"joint-specifier", "y">, 
Group<12>, Arg<"FileLineColumn">,
-Required, Completion<"SourceFile">, 
-Desc<"A specifier in the form filename:line[:column] for setting file & 
line breakpoints.">;  
+Required, Completion<"SourceFile">,
+Desc<"A specifier in the form filename:line[:column] for setting file & 
line breakpoints.">;
   /* Don't add this option till it actually does something useful...

revert whitespace only changes.



Comment at: lldb/source/Commands/Options.td:754
   def source_list_file_colon_line : Option<"joint-specifier", "y">, Group<5>,
-Arg<"FileLineColumn">, Completion<"SourceFile">, 
+Arg<"FileLineColumn">, Completion<"SourceFile">,
 Desc<"A specifier in the form filename:line[:column] from which to display"

revert whitespace only changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705

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

[Lldb-commits] [PATCH] D86417: [lldb] do not propagate eTrapHandlerFrame repeatedly

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.
Herald added a subscriber: danielkiss.

Hi, could you try

diff --git 
a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp 
b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
index 36e7b90cad2..66eb86fe1c0 100644

- a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp

+++ b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
@@ -1573,6 +1573,7 @@ bool 
x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite(

  unwind_plan.SetSourceName(unwind_plan_source.c_str());
  unwind_plan.SetSourcedFromCompiler(eLazyBoolNo);
  unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolYes);

+unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo);

  }
  return true;

}

and let me know how that goes?  I don't have a linux system here to try it on, 
but I have a feeling this may fix the problem you're seeing.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D86417

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