[Lldb-commits] [PATCH] D68874: [lldb] Fix offset intersection bug between MPX and AVX registers

[Pavel Labath via Phabricator via lldb-commits]

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

Thanks for doing this, and especially thank you for writing the test.

For posterity: more discussion on this patch and the problem it solves can be 
found in https://reviews.llvm.org/D62931.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68874



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

[Lldb-commits] [PATCH] D68662: Redo D68354 - [platform process list] add a flag for showing the processes of all users

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68662#1706316 , @wallace wrote:

> will update the previous diff


There's no need to do that now. It's just a thing to keep in mind for the 
future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68662



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

[Lldb-commits] [PATCH] D68853: uint32_t options -> File::OpenOptions options

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Host/common/File.cpp:42
 
-static const char *GetStreamOpenModeFromOptions(uint32_t options) {
+static Expected GetStreamOpenModeFromOptions(uint32_t options) {
   if (options & File::eOpenOptionAppend) {

lawrence_danna wrote:
> labath wrote:
> > I'd consider leaving the static function as `const char *` -- the function 
> > doesn't do anything complex, and there's only one way it can fail (which 
> > can be signalled by a nullptr), so having the Expected wrapper around that 
> > does not seem all that useful -- I'll leave it up to you though.
> If I do that I'd still just have to convert it to Error in the callers, so I 
> think I'll leave it this way.
Yes, but OTOH, if you did that, there would also be callers where you wouldn't 
need the `consumeError(.takeError())` stuff. Anyway, I think this is fine too...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68853



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

[Lldb-commits] [PATCH] D68546: remove FILE* usage from ReportEventState() and HandleProcessEvent()

[Pavel Labath via Phabricator via lldb-commits]

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

In D68546#1706424 , @lawrence_danna 
wrote:

> Ok I'll just update it to go though `StreamFile` internally and leave the 
> decision of whether a `SBStream` version should be added to a later patch 
> and/or someone else to decide.


SGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68546



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

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

[Pavel Labath via Phabricator via lldb-commits]

labath added subscribers: lawrence_danna, labath.
labath added inline comments.



Comment at: lldb/include/lldb/Interpreter/ScriptInterpreter.h:469
+  
+  virtual int GetNumArgumentsForCallable(const char *callable_name) { 
+return -1;

In light of varargs functions (`*args, **kwargs`), which are fairly popular in 
python, the concept of "number of arguments of a callable" does not seem that 
well defined. The current implementation seems to return the number of fixed 
arguments, which might be fine, but I think this behavior should be documented. 
Also, it would be good to modernize this function signature -- have it take a 
StringRef, and return a `Expected` -- ongoing work by 
@lawrence_danna will make it possible to return errors from the python 
interpreter, and this will make it possible to display those, instead of just 
guessing that this is because the callable was not found (it could in fact be 
because the named thing is not a callable, of because resolving the name 
produced an exception, ...).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68671



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

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py:31
+#  which is not necessarily the order on the screen.)
+self.child.timeout = 2
+self.child.send("s") # step

What's the reason for that? It'd be best to not mess with the timeout in the 
test, and particularly to not set such a small time out as it will likely lead 
to flakyness on slow/heavily loaded machines.


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

https://reviews.llvm.org/D68541



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

[Lldb-commits] [PATCH] D68910: use LLVM_LIBDIR_SUFFIX for python lib path

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: hhb.
labath added a subscriber: hhb.
labath added a comment.

As @mgorny said, I'm afraid the solution is not going to be as simple as 
attaching LLVM_LIBDIR_SUFFIX to the lib path. There is some ongoing work to 
refactor/fix the way python paths are handled, but unfortunately I've lost 
track of what exactly is the state of it right now. @mgorny, @hhb, would it be 
correct to say that once your python changes land, the above use case should 
work?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68910



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

[Lldb-commits] [PATCH] D67954: [LLDB] [Windows] Initial support for ARM64 register contexts

[Pavel Labath via Phabricator via lldb-commits]

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

Thank you _very_ much for those tests. +@mgorny, in case he has any comments on 
those.

In D67954#1707292 , @mstorsjo wrote:

> The tests pass both with and without use of lldb-server. However, when using 
> lldb-server with NativeRegisterContext, while the register values are 
> correct, I don't get a correct working backtrace with it. Without 
> lldb-server, I get a perfect backtrace. (The tested binary uses SEH unwind 
> tables, but DWARF debug info.) Any clues about what might be going wrong 
> there?


Hard to say off-hand, but the first thing I'd check is whether the information 
about loaded modules and their addresses is making its way into lldb. You can 
use the "image list" command to inspect that. Then there's the "image 
show-unwind" command which can show you how lldb will try to unwind for a given 
function/address. Also, if you enable the "unwind" log channel (log enable lldb 
unwind), you'll get a trace of what lldb did while attempting to unwind.


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

https://reviews.llvm.org/D67954



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

[Lldb-commits] [PATCH] D68918: eliminate virtual methods from PythonDataObjects

[Pavel Labath via Phabricator via lldb-commits]

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

Yes, this definitely looks better. Ideally, I'd like to get rid of the `Reset` 
functions altogether, and just ensure we already create/return fully valid 
objects (probably via factory functions returning `Expected`s, `Optional`s or 
whatever)..




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:188-192
   // PythonObject is implicitly convertible to PyObject *, which will call the
   // wrong overload.  We want to explicitly disallow this, since a PyObject
   // *always* owns its reference.  Therefore the overload which takes a
   // PyRefType doesn't make sense, and the copy constructor should be used.
   void Reset(PyRefType type, const PythonObject &ref) = delete;

BTW, is this needed? My impression was that PythonObject was/is not convertible 
to a `PyObject*`..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68918



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

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:394
+  mapOptionalHex(IO, "Exception Address", Exception.ExceptionAddress, 0);
+  IO.mapOptional("Number Parameters", Exception.NumberParameters,
+ support::ulittle32_t(0u));

JosephTremoulet wrote:
> labath wrote:
> > This file has a helper function for this (`mapOptional(IO, "name", value, 
> > 0)`. I'd consider changing the field name to "Number of Parameters" even 
> > though it does not match the field name, as it reads weird without that. 
> > I'm not sure why the microsoft naming is inconsistent here -- most of the 
> > other minidump structs have "of" in their name already (BaseOfImage, 
> > SizeOfImage, etc.), but at least we can be consistent.
> Updated to use the helper, and changed the name in the YAML to "Number of 
> Parameters".  Let me know if it's important to you to also change the name of 
> the field in the llvm::minidump::Exception type to `NumberOfParameters` -- I 
> wasn't sure if you were suggesting that, and regardless my preference would 
> be to leave that as-is to match breakpad aside from casing, as otherwise it's 
> hard to know where to stop (e.g. change "ExceptionInformation" to 
> "Parameters" to match "NumberOfParameters" and the YAML?  Reconcile the 
> several different ways that alignment padding fields are named?  etc.)
I wasn't sure about that either -- that's why it was phrased so vaguely. While 
I generally tried to stick to the original naming, I have already done some 
renames to the field names in existing structures -- IIRC, it was limited to 
unifying the naming for the various "reserved"/"unused" fields. Anyway, I think 
that keeping the original name in this particular case is fine.



Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:377-383
+  // TODO: We could provide a reasonable default for ThreadContext by searching
+  // the Thread stream for a thread with the given ID and using its Context.
+  // That would require a couple changes:
+  //   1. We'd need to pass the whole dump around as a context argument.
+  //   2. We'd need to ensure that the Thread stream got processed before
+  //  the Exception stream (or make Exception's ThreadContext required
+  //  when the Exception stream is processed before the Thread stream).

I've been thinking about this for a while now, and while that idea has some 
appeal, I am not sure if this would ever really be a "good" idea. Currently, 
this format allows you to easily create 
syntactically-valid-but-probably-nonsensical minidumps (multiple thread list 
streams, multiple threads with the same ID, etc..). All of that would be more 
difficult if we started depending on strict "correctness" of other parts of the 
minidump in order to compute something here.

Even if I was doing this, I'd probably implement this differently -- make the 
context always optional, but then check for consistency higher up (the validate 
call of the entire minidump object or something). Anyway, maybe just delete 
this todo? :)



Comment at: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp:310-316
+  ASSERT_FALSE(ExpectedFile);
+  auto Unhandled =
+  handleErrors(ExpectedFile.takeError(), [](const StringError &error) {
+EXPECT_EQ(static_cast(std::errc::invalid_argument),
+  error.convertToErrorCode().value());
+  });
+  EXPECT_THAT_ERROR(std::move(Unhandled), Succeeded());

Maybe:
```
EXPECT_THAT_EXPECTED(ExpectedFile, Failed(
testing::Property(&StringError::convertToErrorCode,
 make_error_code(errc::invalid_argument;
```
?

Though, this might actually be best off as a lit test where you just FileCheck 
the exact error message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68657



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

[Lldb-commits] [PATCH] D67954: [LLDB] [Windows] Initial support for ARM64 register contexts

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D67954#1707791 , @labath wrote:

> Thank you _very_ much for those tests. +@mgorny, in case he has any comments 
> on those.
>
> In D67954#1707292 , @mstorsjo wrote:
>
> > The tests pass both with and without use of lldb-server. However, when 
> > using lldb-server with NativeRegisterContext, while the register values are 
> > correct, I don't get a correct working backtrace with it. Without 
> > lldb-server, I get a perfect backtrace. (The tested binary uses SEH unwind 
> > tables, but DWARF debug info.) Any clues about what might be going wrong 
> > there?
>
>
> Hard to say off-hand, but the first thing I'd check is whether the 
> information about loaded modules and their addresses is making its way into 
> lldb. You can use the "image list" command to inspect that. Then there's the 
> "image show-unwind" command which can show you how lldb will try to unwind 
> for a given function/address. Also, if you enable the "unwind" log channel 
> (log enable lldb unwind), you'll get a trace of what lldb did while 
> attempting to unwind.


Thanks for the debugging tips. When I run `image list` I get `error: the target 
has no associated executable images`, so that pretty clearly shows that 
something's missing. On x86_64, `image list` shows only the debugged exe 
itself, when run with `LLDB_USE_LLDB_SERVER=1`, while it shows the exe and a 
few system dlls (ntdll, kernel32, kernelbase and ucrtbase) when run without 
lldb-server.


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

https://reviews.llvm.org/D67954



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

[Lldb-commits] [PATCH] D68939: [LLDB] [PECOFF] Use a "pc" vendor name in aarch64 triples

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, compnerd, aleksandr.urakov.
Herald added subscribers: JDevlieghere, kristof.beyls.
Herald added a project: LLDB.

This matches all other architectures listed in the same file.

This fixes debugging aarch64 executables with lldb-server, which otherwise 
fails, with log messages like these:

  Target::SetArchitecture changing architecture to aarch64 
(aarch64-pc-windows-msvc)
  Target::SetArchitecture Trying to select executable file architecture aarch64 
(aarch64-pc-windows-msvc)

ArchSpec::SetArchitecture sets the vendor to llvm::Triple::PC for any 
coff/win32 combination, and if this doesn't match the triple set by the PECOFF 
module, things doesn't seem to work with when using lldb-server.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68939

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/test/Shell/ObjectFile/PECOFF/basic-info-arm64.yaml


Index: lldb/test/Shell/ObjectFile/PECOFF/basic-info-arm64.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/basic-info-arm64.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/basic-info-arm64.yaml
@@ -2,7 +2,7 @@
 # RUN: lldb-test object-file %t | FileCheck %s
 
 # CHECK: Plugin name: pe-coff
-# CHECK: Architecture: aarch64-unknown-windows-msvc
+# CHECK: Architecture: aarch64-pc-windows-msvc
 # CHECK: UUID: 
 # CHECK: Executable: true
 # CHECK: Stripped: false
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -200,7 +200,7 @@
 specs.Append(module_spec);
 break;
   case MachineArm64:
-spec.SetTriple("aarch64-unknown-windows");
+spec.SetTriple("aarch64-pc-windows");
 specs.Append(module_spec);
 break;
   default:


Index: lldb/test/Shell/ObjectFile/PECOFF/basic-info-arm64.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/basic-info-arm64.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/basic-info-arm64.yaml
@@ -2,7 +2,7 @@
 # RUN: lldb-test object-file %t | FileCheck %s
 
 # CHECK: Plugin name: pe-coff
-# CHECK: Architecture: aarch64-unknown-windows-msvc
+# CHECK: Architecture: aarch64-pc-windows-msvc
 # CHECK: UUID: 
 # CHECK: Executable: true
 # CHECK: Stripped: false
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -200,7 +200,7 @@
 specs.Append(module_spec);
 break;
   case MachineArm64:
-spec.SetTriple("aarch64-unknown-windows");
+spec.SetTriple("aarch64-pc-windows");
 specs.Append(module_spec);
 break;
   default:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67954: [LLDB] [Windows] Initial support for ARM64 register contexts

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D67954#1707791 , @labath wrote:

> Hard to say off-hand, but the first thing I'd check is whether the 
> information about loaded modules and their addresses is making its way into 
> lldb. You can use the "image list" command to inspect that. Then there's the 
> "image show-unwind" command which can show you how lldb will try to unwind 
> for a given function/address. Also, if you enable the "unwind" log channel 
> (log enable lldb unwind), you'll get a trace of what lldb did while 
> attempting to unwind.


Thanks for the debugging pointers. I managed to track this down, with a fix 
suggestion in D68939 . I guess that indicates 
another issue elsewhere, but it's pretty much out of scope for me to dig 
further into that issue now. I guess it should be possible to reproduce the 
same issue on x86_64 by changing the triple similarly there as well.


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

https://reviews.llvm.org/D67954



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

[Lldb-commits] [lldb] r374769 - DWARFExpression: Fix/add support for (v4) debug_loc base address selection entries

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Oct 14 05:49:06 2019
New Revision: 374769

URL: http://llvm.org/viewvc/llvm-project?rev=374769&view=rev
Log:
DWARFExpression: Fix/add support for (v4) debug_loc base address selection 
entries

The DWARFExpression is parsing the location lists in about five places.
Of those, only one actually had proper support for base address
selection entries.

Since r374600, llvm has started to produce location expressions with
base address selection entries more aggresively, which caused some tests
to fail.

This patch adds support for these entries to the places which had it
missing, fixing the failing tests. It also adds a targeted test for the
two of the three fixes, which should continue testing this functionality
even if the llvm output changes. I am not aware of a way to write a
targeted test for the third fix (DWARFExpression::Evaluate).

Modified:
lldb/trunk/source/Expression/DWARFExpression.cpp
lldb/trunk/test/Shell/SymbolFile/DWARF/debug_loc.s

Modified: lldb/trunk/source/Expression/DWARFExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/DWARFExpression.cpp?rev=374769&r1=374768&r2=374769&view=diff
==
--- lldb/trunk/source/Expression/DWARFExpression.cpp (original)
+++ lldb/trunk/source/Expression/DWARFExpression.cpp Mon Oct 14 05:49:06 2019
@@ -636,6 +636,11 @@ bool DWARFExpression::LocationListContai
   if (lo_pc == 0 && hi_pc == 0)
 break;
 
+  if ((m_data.GetAddressByteSize() == 4 && (lo_pc == UINT32_MAX)) ||
+  (m_data.GetAddressByteSize() == 8 && (lo_pc == UINT64_MAX))) {
+loclist_base_addr = hi_pc + m_loclist_slide;
+continue;
+  }
   lo_pc += loclist_base_addr - m_loclist_slide;
   hi_pc += loclist_base_addr - m_loclist_slide;
 
@@ -671,6 +676,12 @@ bool DWARFExpression::GetLocation(addr_t
   if (lo_pc == 0 && hi_pc == 0)
 break;
 
+  if ((m_data.GetAddressByteSize() == 4 && (lo_pc == UINT32_MAX)) ||
+  (m_data.GetAddressByteSize() == 8 && (lo_pc == UINT64_MAX))) {
+curr_base_addr = hi_pc + m_loclist_slide;
+continue;
+  }
+
   lo_pc += curr_base_addr - m_loclist_slide;
   hi_pc += curr_base_addr - m_loclist_slide;
 
@@ -967,6 +978,13 @@ bool DWARFExpression::Evaluate(Execution
 if (lo_pc == 0 && hi_pc == 0)
   break;
 
+if ((m_data.GetAddressByteSize() == 4 &&
+ (lo_pc == UINT32_MAX)) ||
+(m_data.GetAddressByteSize() == 8 &&
+ (lo_pc == UINT64_MAX))) {
+  curr_loclist_base_load_addr = hi_pc + m_loclist_slide;
+  continue;
+}
 lo_pc += curr_loclist_base_load_addr - m_loclist_slide;
 hi_pc += curr_loclist_base_load_addr - m_loclist_slide;
 

Modified: lldb/trunk/test/Shell/SymbolFile/DWARF/debug_loc.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/Shell/SymbolFile/DWARF/debug_loc.s?rev=374769&r1=374768&r2=374769&view=diff
==
--- lldb/trunk/test/Shell/SymbolFile/DWARF/debug_loc.s (original)
+++ lldb/trunk/test/Shell/SymbolFile/DWARF/debug_loc.s Mon Oct 14 05:49:06 2019
@@ -17,14 +17,22 @@
 # CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg0 
RAX,
 # CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
 # CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
+# CHECK: Variable: {{.*}}, name = "x3", type = "int", location = DW_OP_reg1 
RDX,
 
 .type   f,@function
 f:  # @f
 .Lfunc_begin0:
-movl%edi, %eax
+nop
 .Ltmp0:
-retq
+nop
 .Ltmp1:
+nop
+.Ltmp2:
+nop
+.Ltmp3:
+nop
+.Ltmp4:
+nop
 .Lfunc_end0:
 .size   f, .Lfunc_end0-f
 
@@ -35,12 +43,6 @@ f:
 .asciz  "f"
 .Linfo_string4:
 .asciz  "int"
-.Lx0:
-.asciz  "x0"
-.Lx1:
-.asciz  "x1"
-.Lx2:
-.asciz  "x2"
 
 .section.debug_loc,"",@progbits
 .Ldebug_loc0:
@@ -54,6 +56,17 @@ f:
 .byte   80  # super-register DW_OP_reg0
 .quad   0
 .quad   0
+
+.Ldebug_loc3:
+.quad   -1  # Select base address
+.quad   .Ltmp1
+.quad   .Ltmp1-.Ltmp1
+.quad   .Ltmp2-.Ltmp1
+.short  1   # Loc expr size
+.byte   81  # super-register DW_OP_reg1
+.quad   0
+.quad   0
+
 .Ldebug_loc2:
 .quad   .Lfunc_begin0-.Lfunc_begin0
 .quad   .Lfunc_end0-.Lfunc_begin0
@@ -88,7 +101,7 @@ f:
 .byte   2   # DW_AT_location
 .byte   23  # DW_FORM_sec_offset
 .byte   3   # DW_AT_name
-.byte   14  # DW_FORM_strp
+.byte   8   # DW_FORM_string
  

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 224838.
kwk added a comment.

- Silence FileCheck in test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943

Files:
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
  lldb/test/Shell/ObjectFile/ELF/no-symtab-generated.yaml
  lldb/test/Shell/ObjectFile/ELF/symtab-generated.yaml
  llvm/include/llvm/ObjectYAML/ELFYAML.h
  llvm/lib/ObjectYAML/ELFEmitter.cpp
  llvm/tools/obj2yaml/elf2yaml.cpp

Index: llvm/tools/obj2yaml/elf2yaml.cpp
===
--- llvm/tools/obj2yaml/elf2yaml.cpp
+++ llvm/tools/obj2yaml/elf2yaml.cpp
@@ -200,8 +200,8 @@
   return TableOrErr.takeError();
 ShndxTable = *TableOrErr;
   }
-  if (SymTab)
-if (Error E = dumpSymbols(SymTab, Y->Symbols))
+  if (SymTab && Y->Symbols)
+if (Error E = dumpSymbols(SymTab, *Y->Symbols))
   return std::move(E);
   if (DynSymTab)
 if (Error E = dumpSymbols(DynSymTab, Y->DynamicSymbols))
Index: llvm/lib/ObjectYAML/ELFEmitter.cpp
===
--- llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -210,8 +210,10 @@
 Doc.Sections.begin(),
 std::make_unique(
 ELFYAML::Section::SectionKind::RawContent, /*IsImplicit=*/true));
-
-  std::vector ImplicitSections = {".symtab", ".strtab", ".shstrtab"};
+  
+  std::vector ImplicitSections = {".strtab", ".shstrtab"};
+  if (Doc.Symbols) 
+ImplicitSections.push_back(".symtab");
   if (!Doc.DynamicSymbols.empty())
 ImplicitSections.insert(ImplicitSections.end(), {".dynsym", ".dynstr"});
 
@@ -508,7 +510,7 @@
  ELFYAML::Section *YAMLSec) {
 
   bool IsStatic = STType == SymtabType::Static;
-  const auto &Symbols = IsStatic ? Doc.Symbols : Doc.DynamicSymbols;
+  const auto &Symbols = (IsStatic && Doc.Symbols) ? *Doc.Symbols : Doc.DynamicSymbols;
 
   ELFYAML::RawContentSection *RawSec =
   dyn_cast_or_null(YAMLSec);
@@ -556,8 +558,9 @@
 
   // If the symbol table section is explicitly described in the YAML
   // then we should set the fields requested.
-  SHeader.sh_info = (RawSec && RawSec->Info) ? (unsigned)(*RawSec->Info)
- : findFirstNonGlobal(Symbols) + 1;
+  SHeader.sh_info = (RawSec && RawSec->Info)
+? (unsigned)(*RawSec->Info)
+: findFirstNonGlobal(Symbols) + 1;
   SHeader.sh_entsize = (YAMLSec && YAMLSec->EntSize)
? (uint64_t)(*YAMLSec->EntSize)
: sizeof(Elf_Sym);
@@ -1044,14 +1047,16 @@
 }
   };
 
-  Build(Doc.Symbols, SymN2I);
+  if (Doc.Symbols)
+Build(*Doc.Symbols, SymN2I);
   Build(Doc.DynamicSymbols, DynSymN2I);
 }
 
 template  void ELFState::finalizeStrings() {
   // Add the regular symbol names to .strtab section.
-  for (const ELFYAML::Symbol &Sym : Doc.Symbols)
-DotStrtab.add(ELFYAML::dropUniqueSuffix(Sym.Name));
+  if (Doc.Symbols)
+for (const ELFYAML::Symbol &Sym : *Doc.Symbols)
+  DotStrtab.add(ELFYAML::dropUniqueSuffix(Sym.Name));
   DotStrtab.finalize();
 
   // Add the dynamic symbol names to .dynstr section.
Index: llvm/include/llvm/ObjectYAML/ELFYAML.h
===
--- llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -376,7 +376,7 @@
   // cleaner and nicer if we read them from the YAML as a separate
   // top-level key, which automatically ensures that invariants like there
   // being a single SHT_SYMTAB section are upheld.
-  std::vector Symbols;
+  Optional> Symbols;
   std::vector DynamicSymbols;
 };
 
Index: lldb/test/Shell/ObjectFile/ELF/symtab-generated.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/symtab-generated.yaml
@@ -0,0 +1,37 @@
+# In this test we define a "Symbols" entry in the YAML and expect a .symtab
+# section to be generated. Notice that this is automatically added even though
+# we don't have .symtab listed in the "Sections" entry.
+
+# RUN: yaml2obj %s > %t.obj
+# RUN: %lldb -b -o "image dump sections" %t.obj | FileCheck %s
+
+# CHECK: SectID Type File Address Perm File Off.  File Size  Flags  Section Name
+# CHECK-NEXT: --  ---   -- -- -- 
+# CHECK-NEXT: {{.*}}.text
+# CHECK-NEXT: {{.*}}.strtab
+# CHECK-NEXT: {{.*}}.shstrtab
+# CHECK-NEXT: {{.*}}.symtab
+# CHECK-EMPTY:
+
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Ma

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
kwk added a reviewer: labath.
Herald added subscribers: llvm-commits, lldb-commits, MaskRay, hiraditya, 
arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: alexshap.
Herald added projects: LLDB, LLVM.
kwk updated this revision to Diff 224838.
kwk added a comment.

- Silence FileCheck in test


Before `yaml2obj ` would always add an implicit `.symtab` section
even if there were no symbols defined in the YAML ``. Now, only
when there's a `Symbols` entry, we will generate a `.symtab` section.

Old minidebuginfo tests that manually removed the `.symtab` section
have been adjusted because it is no longer needed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68943

Files:
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
  lldb/test/Shell/ObjectFile/ELF/no-symtab-generated.yaml
  lldb/test/Shell/ObjectFile/ELF/symtab-generated.yaml
  llvm/include/llvm/ObjectYAML/ELFYAML.h
  llvm/lib/ObjectYAML/ELFEmitter.cpp
  llvm/tools/obj2yaml/elf2yaml.cpp

Index: llvm/tools/obj2yaml/elf2yaml.cpp
===
--- llvm/tools/obj2yaml/elf2yaml.cpp
+++ llvm/tools/obj2yaml/elf2yaml.cpp
@@ -200,8 +200,8 @@
   return TableOrErr.takeError();
 ShndxTable = *TableOrErr;
   }
-  if (SymTab)
-if (Error E = dumpSymbols(SymTab, Y->Symbols))
+  if (SymTab && Y->Symbols)
+if (Error E = dumpSymbols(SymTab, *Y->Symbols))
   return std::move(E);
   if (DynSymTab)
 if (Error E = dumpSymbols(DynSymTab, Y->DynamicSymbols))
Index: llvm/lib/ObjectYAML/ELFEmitter.cpp
===
--- llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -210,8 +210,10 @@
 Doc.Sections.begin(),
 std::make_unique(
 ELFYAML::Section::SectionKind::RawContent, /*IsImplicit=*/true));
-
-  std::vector ImplicitSections = {".symtab", ".strtab", ".shstrtab"};
+  
+  std::vector ImplicitSections = {".strtab", ".shstrtab"};
+  if (Doc.Symbols) 
+ImplicitSections.push_back(".symtab");
   if (!Doc.DynamicSymbols.empty())
 ImplicitSections.insert(ImplicitSections.end(), {".dynsym", ".dynstr"});
 
@@ -508,7 +510,7 @@
  ELFYAML::Section *YAMLSec) {
 
   bool IsStatic = STType == SymtabType::Static;
-  const auto &Symbols = IsStatic ? Doc.Symbols : Doc.DynamicSymbols;
+  const auto &Symbols = (IsStatic && Doc.Symbols) ? *Doc.Symbols : Doc.DynamicSymbols;
 
   ELFYAML::RawContentSection *RawSec =
   dyn_cast_or_null(YAMLSec);
@@ -556,8 +558,9 @@
 
   // If the symbol table section is explicitly described in the YAML
   // then we should set the fields requested.
-  SHeader.sh_info = (RawSec && RawSec->Info) ? (unsigned)(*RawSec->Info)
- : findFirstNonGlobal(Symbols) + 1;
+  SHeader.sh_info = (RawSec && RawSec->Info)
+? (unsigned)(*RawSec->Info)
+: findFirstNonGlobal(Symbols) + 1;
   SHeader.sh_entsize = (YAMLSec && YAMLSec->EntSize)
? (uint64_t)(*YAMLSec->EntSize)
: sizeof(Elf_Sym);
@@ -1044,14 +1047,16 @@
 }
   };
 
-  Build(Doc.Symbols, SymN2I);
+  if (Doc.Symbols)
+Build(*Doc.Symbols, SymN2I);
   Build(Doc.DynamicSymbols, DynSymN2I);
 }
 
 template  void ELFState::finalizeStrings() {
   // Add the regular symbol names to .strtab section.
-  for (const ELFYAML::Symbol &Sym : Doc.Symbols)
-DotStrtab.add(ELFYAML::dropUniqueSuffix(Sym.Name));
+  if (Doc.Symbols)
+for (const ELFYAML::Symbol &Sym : *Doc.Symbols)
+  DotStrtab.add(ELFYAML::dropUniqueSuffix(Sym.Name));
   DotStrtab.finalize();
 
   // Add the dynamic symbol names to .dynstr section.
Index: llvm/include/llvm/ObjectYAML/ELFYAML.h
===
--- llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -376,7 +376,7 @@
   // cleaner and nicer if we read them from the YAML as a separate
   // top-level key, which automatically ensures that invariants like there
   // being a single SHT_SYMTAB section are upheld.
-  std::vector Symbols;
+  Optional> Symbols;
   std::vector DynamicSymbols;
 };
 
Index: lldb/test/Shell/ObjectFile/ELF/symtab-generated.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/symtab-generated.yaml
@@ -0,0 +1,37 @@
+# In this test we define a "Symbols" entry in the YAML and expect a .symtab
+# section to be generated. Notice that this is automatically added even though
+# we don't have .symtab listed in the "Sections" entry.
+
+# RUN: yaml2obj %s > %t.obj
+# RUN: %lldb -b -o "image dump sections" %t.obj | FileCheck %s
+
+# CHECK: SectID 

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 224839.
kwk added a comment.

- restore formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943

Files:
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
  lldb/test/Shell/ObjectFile/ELF/no-symtab-generated.yaml
  lldb/test/Shell/ObjectFile/ELF/symtab-generated.yaml
  llvm/include/llvm/ObjectYAML/ELFYAML.h
  llvm/lib/ObjectYAML/ELFEmitter.cpp
  llvm/tools/obj2yaml/elf2yaml.cpp

Index: llvm/tools/obj2yaml/elf2yaml.cpp
===
--- llvm/tools/obj2yaml/elf2yaml.cpp
+++ llvm/tools/obj2yaml/elf2yaml.cpp
@@ -200,8 +200,8 @@
   return TableOrErr.takeError();
 ShndxTable = *TableOrErr;
   }
-  if (SymTab)
-if (Error E = dumpSymbols(SymTab, Y->Symbols))
+  if (SymTab && Y->Symbols)
+if (Error E = dumpSymbols(SymTab, *Y->Symbols))
   return std::move(E);
   if (DynSymTab)
 if (Error E = dumpSymbols(DynSymTab, Y->DynamicSymbols))
Index: llvm/lib/ObjectYAML/ELFEmitter.cpp
===
--- llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -210,8 +210,10 @@
 Doc.Sections.begin(),
 std::make_unique(
 ELFYAML::Section::SectionKind::RawContent, /*IsImplicit=*/true));
-
-  std::vector ImplicitSections = {".symtab", ".strtab", ".shstrtab"};
+  
+  std::vector ImplicitSections = {".strtab", ".shstrtab"};
+  if (Doc.Symbols) 
+ImplicitSections.push_back(".symtab");
   if (!Doc.DynamicSymbols.empty())
 ImplicitSections.insert(ImplicitSections.end(), {".dynsym", ".dynstr"});
 
@@ -508,7 +510,7 @@
  ELFYAML::Section *YAMLSec) {
 
   bool IsStatic = STType == SymtabType::Static;
-  const auto &Symbols = IsStatic ? Doc.Symbols : Doc.DynamicSymbols;
+  const auto &Symbols = (IsStatic && Doc.Symbols) ? *Doc.Symbols : Doc.DynamicSymbols;
 
   ELFYAML::RawContentSection *RawSec =
   dyn_cast_or_null(YAMLSec);
@@ -1044,14 +1046,16 @@
 }
   };
 
-  Build(Doc.Symbols, SymN2I);
+  if (Doc.Symbols)
+Build(*Doc.Symbols, SymN2I);
   Build(Doc.DynamicSymbols, DynSymN2I);
 }
 
 template  void ELFState::finalizeStrings() {
   // Add the regular symbol names to .strtab section.
-  for (const ELFYAML::Symbol &Sym : Doc.Symbols)
-DotStrtab.add(ELFYAML::dropUniqueSuffix(Sym.Name));
+  if (Doc.Symbols)
+for (const ELFYAML::Symbol &Sym : *Doc.Symbols)
+  DotStrtab.add(ELFYAML::dropUniqueSuffix(Sym.Name));
   DotStrtab.finalize();
 
   // Add the dynamic symbol names to .dynstr section.
Index: llvm/include/llvm/ObjectYAML/ELFYAML.h
===
--- llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -376,7 +376,7 @@
   // cleaner and nicer if we read them from the YAML as a separate
   // top-level key, which automatically ensures that invariants like there
   // being a single SHT_SYMTAB section are upheld.
-  std::vector Symbols;
+  Optional> Symbols;
   std::vector DynamicSymbols;
 };
 
Index: lldb/test/Shell/ObjectFile/ELF/symtab-generated.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/symtab-generated.yaml
@@ -0,0 +1,37 @@
+# In this test we define a "Symbols" entry in the YAML and expect a .symtab
+# section to be generated. Notice that this is automatically added even though
+# we don't have .symtab listed in the "Sections" entry.
+
+# RUN: yaml2obj %s > %t.obj
+# RUN: %lldb -b -o "image dump sections" %t.obj | FileCheck %s
+
+# CHECK: SectID Type File Address Perm File Off.  File Size  Flags  Section Name
+# CHECK-NEXT: --  ---   -- -- -- 
+# CHECK-NEXT: {{.*}}.text
+# CHECK-NEXT: {{.*}}.strtab
+# CHECK-NEXT: {{.*}}.shstrtab
+# CHECK-NEXT: {{.*}}.symtab
+# CHECK-EMPTY:
+
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_X86_64
+  Entry:   0x004004C0
+Sections:
+- Name:.text
+  Type:SHT_PROGBITS
+  Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+  Address: 0x004003D0
+  AddressAlign:0x0010
+  Content: DEADBEEFBAADF00D
+Symbols:
+- Name:main
+  Type:STT_FUNC
+  Section: .text
+  Value:   0x004003D0
+  Size:0x0008
+...
Index: lldb/test/Shell/ObjectFile/ELF/no-symtab-generated.yaml
===
--- /dev/null
+++ l

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

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



Comment at: llvm/include/llvm/ObjectYAML/ELFYAML.h:379-380
   // being a single SHT_SYMTAB section are upheld.
-  std::vector Symbols;
+  Optional> Symbols;
   std::vector DynamicSymbols;
 };

labath wrote:
> This creates an inconsistency between how .symtab and .dynsym sections are 
> handled, which is hard to justify. I actually prefer the Optional<> version, 
> because that enables one to say "the symtab section is there, but it is 
> _empty_", something which is impossible with the way .dynsym emission works 
> (the section gets surpressed if it is empty), but the question is, shouldn't 
> then the .dynsym emission work the same way ? Have you looked at by any 
> chance how hard it would be to change .dynsym to follow the same pattern?
> 
> (In any case, I think this is up to yaml2obj maintainers to decide how to 
> handle this. I'll add some to this review.)
@labath I haven't looked but will investigate how hard it is to declare 
`Optional> DynamicSymbols;`. That's what I understood at 
least.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943



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

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk planned changes to this revision.
kwk added a comment.

Move tests over to `llvm/llvm/test/ObjectYAML/ELF/`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943



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

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Pavel Labath via Phabricator via lldb-commits]

labath added reviewers: grimar, MaskRay, jhenderson.
labath added a comment.

+llvm yaml2obj folks.

Thank you for creating this diff. Please see comments inline.




Comment at: lldb/test/Shell/ObjectFile/ELF/no-symtab-generated.yaml:1
+# In this test we don't explicitly define a "Symbols" entry in the YAML and
+# expect no .symtab section to be generated.

This test (and the next one) should live in the llvm subtree, as they're really 
testing yaml2obj, not lldb (instead of lldb, you can use something like 
llvm-objdump/readobj to inspect the generated files).



Comment at: llvm/include/llvm/ObjectYAML/ELFYAML.h:379-380
   // being a single SHT_SYMTAB section are upheld.
-  std::vector Symbols;
+  Optional> Symbols;
   std::vector DynamicSymbols;
 };

This creates an inconsistency between how .symtab and .dynsym sections are 
handled, which is hard to justify. I actually prefer the Optional<> version, 
because that enables one to say "the symtab section is there, but it is 
_empty_", something which is impossible with the way .dynsym emission works 
(the section gets surpressed if it is empty), but the question is, shouldn't 
then the .dynsym emission work the same way ? Have you looked at by any chance 
how hard it would be to change .dynsym to follow the same pattern?

(In any case, I think this is up to yaml2obj maintainers to decide how to 
handle this. I'll add some to this review.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943



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

[Lldb-commits] [PATCH] D68939: [LLDB] [PECOFF] Use a "pc" vendor name in aarch64 triples

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Would you say that "pc" is a reasonable value for the "vendor" field for the 
win+aarch64 combo? I am asking because I don't have a clue about that, and 
given that this platform is being brought up right now, changing this now would 
be way easier than doing it later. (The reason why things don't work is the 
incompatibility between the two things that compute the ArchSpec, but that can 
also be fixed by changing the other mechanism, if that is better/more correct.) 
My guess is the other mechanism is ArchSpec::SetArchitecture function, line 
928...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68939



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

[Lldb-commits] [PATCH] D68939: [LLDB] [PECOFF] Use a "pc" vendor name in aarch64 triples

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D68939#1707985 , @labath wrote:

> Would you say that "pc" is a reasonable value for the "vendor" field for the 
> win+aarch64 combo? I am asking because I don't have a clue about that, and 
> given that this platform is being brought up right now, changing this now 
> would be way easier than doing it later. (The reason why things don't work is 
> the incompatibility between the two things that compute the ArchSpec, but 
> that can also be fixed by changing the other mechanism, if that is 
> better/more correct.) My guess is the other mechanism is 
> ArchSpec::SetArchitecture function, line 928...


I'd say "pc" is fine here; such machines are available for sale (although with 
a bit scarce availability) as normal power efficient laptops - google for e.g. 
HP Envy X2, for one that is available with both arm and x86 cpu options.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68939



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

[Lldb-commits] [PATCH] D68939: [LLDB] [PECOFF] Use a "pc" vendor name in aarch64 triples

[Pavel Labath via Phabricator via lldb-commits]

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

In D68939#1707998 , @mstorsjo wrote:

> In D68939#1707985 , @labath wrote:
>
> > Would you say that "pc" is a reasonable value for the "vendor" field for 
> > the win+aarch64 combo? I am asking because I don't have a clue about that, 
> > and given that this platform is being brought up right now, changing this 
> > now would be way easier than doing it later. (The reason why things don't 
> > work is the incompatibility between the two things that compute the 
> > ArchSpec, but that can also be fixed by changing the other mechanism, if 
> > that is better/more correct.) My guess is the other mechanism is 
> > ArchSpec::SetArchitecture function, line 928...
>
>
> I'd say "pc" is fine here; such machines are available for sale (although 
> with a bit scarce availability) as normal power efficient laptops - google 
> for e.g. HP Envy X2, for one that is available with both arm and x86 cpu 
> options.


So, am I correct to assume that "pc" is used/can be used for any "personal 
computer"? I was under the impression that pc stands for the PCs which are 
descended/compatible with the original IBM PCs...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68939



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

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet updated this revision to Diff 224853.
JosephTremoulet added a comment.

- Remove TODO, lit-ify negative test and tighten check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68657

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
  llvm/include/llvm/ObjectYAML/MinidumpYAML.h
  llvm/lib/ObjectYAML/MinidumpEmitter.cpp
  llvm/lib/ObjectYAML/MinidumpYAML.cpp
  llvm/test/ObjectYAML/minidump/ExceptionMissingParameter.yaml
  llvm/test/tools/obj2yaml/basic-minidump.yaml
  llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,200 @@
   (ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
   makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
 }
+
+// Test that we can parse a normal-looking ExceptionStream
+TEST(MinidumpYAML, ExceptionStream) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:Exception
+Thread ID:  0x7
+Exception Record:
+  Exception Code:  0x23
+  Exception Flags: 0x5
+  Exception Record: 0x0102030405060708
+  Exception Address: 0x0a0b0c0d0e0f1011
+  Number of Parameters: 2
+  Parameter 0: 0x22
+  Parameter 1: 0x24
+Thread Context:  3DeadBeefDefacedABadCafe)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  Expected ExpectedStream =
+  File.getExceptionStream();
+
+  ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+  const minidump::ExceptionStream &Stream = *ExpectedStream;
+  EXPECT_EQ(0x7u, Stream.ThreadId);
+  const minidump::Exception &Exception = Stream.ExceptionRecord;
+  EXPECT_EQ(0x23u, Exception.ExceptionCode);
+  EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+  EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+  EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+  EXPECT_EQ(2u, Exception.NumberParameters);
+  EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+  EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+  Expected> ExpectedContext =
+  File.getRawData(Stream.ThreadContext);
+  ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+  EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+   0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:Exception
+Thread ID:  0x7
+Exception Record:
+  Exception Code:  0x23
+  Exception Flags: 0x5
+  Exception Record: 0x0102030405060708
+  Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context:  3DeadBeefDefacedABadCafe)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  Expected ExpectedStream =
+  File.getExceptionStream();
+
+  ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+  const minidump::ExceptionStream &Stream = *ExpectedStream;
+  EXPECT_EQ(0x7u, Stream.ThreadId);
+  const minidump::Exception &Exception = Stream.ExceptionRecord;
+  EXPECT_EQ(0x23u, Exception.ExceptionCode);
+  EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+  EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+  EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+  EXPECT_EQ(0u, Exception.NumberParameters);
+
+  Expected> ExpectedContext =
+  File.getRawData(Stream.ThreadContext);
+  ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+  EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+   0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:Exception
+Thread ID:  0x8
+Exception Record:
+  Exception Code: 0
+  Number of Parameters: 16
+  Parameter 0: 0x0
+  Parameter 1: 0xff
+  Parameter 2: 0xee
+  Parameter 3: 0xdd
+  Parameter 4: 0xcc
+  Parameter 5: 0xbb
+  Parameter 6: 0xaa
+  Parameter 7: 0x99
+  Parameter 8: 0x88
+  Parameter 9: 0x77
+  Parameter 10: 0x66
+  Parameter 11: 0x55
+  Parameter 12: 0x44
+  Parameter 13: 

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked an inline comment as done.
kwk added a comment.

Moved `yaml2obj` tests under llvm subtree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943



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

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 224852.
kwk added a comment.

- moved test files over to llvm subtree


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943

Files:
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
  llvm/include/llvm/ObjectYAML/ELFYAML.h
  llvm/lib/ObjectYAML/ELFEmitter.cpp
  llvm/test/ObjectYAML/ELF/no-symtab-generated.yaml
  llvm/test/ObjectYAML/ELF/symtab-generated.yaml
  llvm/tools/obj2yaml/elf2yaml.cpp

Index: llvm/tools/obj2yaml/elf2yaml.cpp
===
--- llvm/tools/obj2yaml/elf2yaml.cpp
+++ llvm/tools/obj2yaml/elf2yaml.cpp
@@ -200,8 +200,8 @@
   return TableOrErr.takeError();
 ShndxTable = *TableOrErr;
   }
-  if (SymTab)
-if (Error E = dumpSymbols(SymTab, Y->Symbols))
+  if (SymTab && Y->Symbols)
+if (Error E = dumpSymbols(SymTab, *Y->Symbols))
   return std::move(E);
   if (DynSymTab)
 if (Error E = dumpSymbols(DynSymTab, Y->DynamicSymbols))
Index: llvm/test/ObjectYAML/ELF/symtab-generated.yaml
===
--- /dev/null
+++ llvm/test/ObjectYAML/ELF/symtab-generated.yaml
@@ -0,0 +1,45 @@
+# In this test we define a "Symbols" entry in the YAML and expect a .symtab
+# section to be generated. Notice that this is automatically added even though
+# we don't have .symtab listed in the "Sections" entry.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj --sections %t | FileCheck %s
+
+# CHECK:  Sections [
+# CHECK:Section {
+# CHECK:  Name: .symtab (25)
+# CHECK-NEXT: Type: SHT_SYMTAB (0x2)
+# CHECK-NEXT: Flags [ (0x0)
+# CHECK-NEXT: ]
+# CHECK-NEXT: Address: 0x0
+# CHECK-NEXT: Offset: 0x70
+# CHECK-NEXT: Size: 48
+# CHECK-NEXT: Link: 2
+# CHECK-NEXT: Info: 2
+# CHECK-NEXT: AddressAlignment: 8
+# CHECK-NEXT: EntrySize: 24
+# CHECK-NEXT:  }
+# CHECK-NEXT:]
+# CHECK-EMPTY:
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_X86_64
+  Entry:   0x004004C0
+Sections:
+- Name:.text
+  Type:SHT_PROGBITS
+  Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+  Address: 0x004003D0
+  AddressAlign:0x0010
+  Content: DEADBEEFBAADF00D
+Symbols:
+- Name:main
+  Type:STT_FUNC
+  Section: .text
+  Value:   0x004003D0
+  Size:0x0008
+...
Index: llvm/test/ObjectYAML/ELF/no-symtab-generated.yaml
===
--- /dev/null
+++ llvm/test/ObjectYAML/ELF/no-symtab-generated.yaml
@@ -0,0 +1,19 @@
+# In this test we don't explicitly define a "Symbols" entry in the YAML and
+# expect no .symtab section to be generated.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj --sections %t | FileCheck %s
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_386
+Sections:
+  - Name:.debug_line
+Type:SHT_PROGBITS
+Flags:   [ SHF_COMPRESSED ]
+
+# CHECK-NOT: Name: .symtab
+# CHECK-NOT: Type: SHT_SYMTAB
Index: llvm/lib/ObjectYAML/ELFEmitter.cpp
===
--- llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -210,8 +210,10 @@
 Doc.Sections.begin(),
 std::make_unique(
 ELFYAML::Section::SectionKind::RawContent, /*IsImplicit=*/true));
-
-  std::vector ImplicitSections = {".symtab", ".strtab", ".shstrtab"};
+  
+  std::vector ImplicitSections = {".strtab", ".shstrtab"};
+  if (Doc.Symbols) 
+ImplicitSections.push_back(".symtab");
   if (!Doc.DynamicSymbols.empty())
 ImplicitSections.insert(ImplicitSections.end(), {".dynsym", ".dynstr"});
 
@@ -508,7 +510,7 @@
  ELFYAML::Section *YAMLSec) {
 
   bool IsStatic = STType == SymtabType::Static;
-  const auto &Symbols = IsStatic ? Doc.Symbols : Doc.DynamicSymbols;
+  const auto &Symbols = (IsStatic && Doc.Symbols) ? *Doc.Symbols : Doc.DynamicSymbols;
 
   ELFYAML::RawContentSection *RawSec =
   dyn_cast_or_null(YAMLSec);
@@ -1044,14 +1046,16 @@
 }
   };
 
-  Build(Doc.Symbols, SymN2I);
+  if (Doc.Symbols)
+Build(*Doc.Symbols, SymN2I);
   Build(Doc.DynamicSymbols, DynSymN2I);
 }
 
 template  void ELFState::finalizeStrings() {
   // Add the regular symbol names to .strtab section.
-  for (const ELFYAML::Symbol &Sym : Doc.Symbols)
-DotStrtab.add(ELFYAML::dropUniqueSuffix(Sym.Name));
+  if (Doc.Symbols)
+for (const ELFYAML::Symbol &Sym : *Doc.Symbols)
+  DotStrtab.add(ELFYAML::dropUni

[Lldb-commits] [PATCH] D68270: DWARFDebugLoc: Add a function to get the address range of an entry

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: lib/DebugInfo/DWARF/DWARFDebugLoc.cpp:291-295
+  EntryIterator Absolute =
+  getAbsoluteLocations(
+  SectionedAddress{BaseAddr, SectionedAddress::UndefSection},
+  LookupPooledAddress)
+  .begin();

dblaikie wrote:
> labath wrote:
> > dblaikie wrote:
> > > labath wrote:
> > > > dblaikie wrote:
> > > > > labath wrote:
> > > > > > dblaikie wrote:
> > > > > > > labath wrote:
> > > > > > > > This parallel iteration is not completely nice, but I think 
> > > > > > > > it's worth being able to reuse the absolute range computation 
> > > > > > > > code. I'm open to ideas for improvement though.
> > > > > > > Ah, I see - this is what you meant about "In particular it makes 
> > > > > > > it possible to reuse this stuff in the dumping code, which would 
> > > > > > > have been pretty hard with callbacks.".
> > > > > > > 
> > > > > > > I'm wondering if that might be worth revisiting somewhat. A full 
> > > > > > > iterator abstraction for one user here (well, two once you 
> > > > > > > include lldb - but I assume it's likely going to build its own 
> > > > > > > data structure from the iteration anyway, right? (it's not going 
> > > > > > > to keep the iterator around, do anything interesting like partial 
> > > > > > > iterations, re-iterate/etc - such that a callback would suffice))
> > > > > > > 
> > > > > > > I could imagine two callback APIs for this - one that gets 
> > > > > > > entries and locations and one that only gets locations by 
> > > > > > > filtering on the entry version.
> > > > > > > 
> > > > > > > eg:
> > > > > > > 
> > > > > > >   // for non-verbose output:
> > > > > > >   LL.forEachEntry([&](const Entry &E, Expected L) {
> > > > > > > if (Verbose && actually dumping debug_loc)
> > > > > > >   print(E) // print any LLE_*, raw parameters, etc
> > > > > > > if (L)
> > > > > > >   print(*L) // print the resulting address range, section 
> > > > > > > name (if verbose), 
> > > > > > > else
> > > > > > >   print(error stuff)
> > > > > > >   });
> > > > > > > 
> > > > > > > One question would be "when/where do we print the DWARF 
> > > > > > > expression" - if there's an error computing the address range, we 
> > > > > > > can still print the expression, so maybe that happens 
> > > > > > > unconditionally at the end of the callback, using the expression 
> > > > > > > in the Entry? (then, arguably, the expression doesn't need to be 
> > > > > > > in the DWARFLocation - and I'd say make the DWARFLocation a 
> > > > > > > sectioned range, exactly the same type as for ranges so that part 
> > > > > > > of the dumping code, etc, can be maximally reused)
> > > > > > Actually, what lldb currently does is that it does not build any 
> > > > > > data structures at all (except storing the pointer to the right 
> > > > > > place in the debug_loc section. Then, whenever it wants to do 
> > > > > > something to the loclist, it parses it afresh. I don't know why it 
> > > > > > does this exactly, but I assume it has something to do with most 
> > > > > > locations never being used, or being only a couple of times, and 
> > > > > > the actual parsing being fairly fast. What this means is that lldb 
> > > > > > is not really a single "user", but there are like four or five 
> > > > > > places where it iterates through the list, depending on what does 
> > > > > > it actually want to do with it. It also does partial iteration 
> > > > > > where it stops as soon as it find the entry it was interested in.
> > > > > > Now, all of that is possible with a callback (though I am generally 
> > > > > > trying to avoid them), but it does resurface the issue of what 
> > > > > > should be the value of the second argument for DW_LLE_base_address 
> > > > > > entries (the thing which I originally used a error type for).
> > > > > > Maybe this should be actually one callback API, taking two callback 
> > > > > > functions, with one of them being invoked for base_address entries, 
> > > > > > and one for others? However, if we stick to the current approaches 
> > > > > > in both LLE and RLE of making the address pool resolution function 
> > > > > > a parameter (which I'd like to keep, as it makes my job in lldb 
> > > > > > easier), then this would actually be three callbacks, which starts 
> > > > > > to get unwieldy. Though one of those callbacks could be removed 
> > > > > > with the "DWARFUnit implementing a AddrOffsetResolver interface" 
> > > > > > idea, which I really like. :)
> > > > > Ah, thanks for the details on LLDB's location parsing logic. That's 
> > > > > interesting indeed!
> > > > > 
> > > > > I can appreciate an iterator-based API if that's the sort of usage 
> > > > > we've got, though I expect it doesn't have any interest in the 
> > > > > low-level encoding & just wants the fully processed address 
> > > > > ranges/locations - it doesn't 

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet marked 4 inline comments as done.
JosephTremoulet added inline comments.



Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:377-383
+  // TODO: We could provide a reasonable default for ThreadContext by searching
+  // the Thread stream for a thread with the given ID and using its Context.
+  // That would require a couple changes:
+  //   1. We'd need to pass the whole dump around as a context argument.
+  //   2. We'd need to ensure that the Thread stream got processed before
+  //  the Exception stream (or make Exception's ThreadContext required
+  //  when the Exception stream is processed before the Thread stream).

labath wrote:
> I've been thinking about this for a while now, and while that idea has some 
> appeal, I am not sure if this would ever really be a "good" idea. Currently, 
> this format allows you to easily create 
> syntactically-valid-but-probably-nonsensical minidumps (multiple thread list 
> streams, multiple threads with the same ID, etc..). All of that would be more 
> difficult if we started depending on strict "correctness" of other parts of 
> the minidump in order to compute something here.
> 
> Even if I was doing this, I'd probably implement this differently -- make the 
> context always optional, but then check for consistency higher up (the 
> validate call of the entire minidump object or something). Anyway, maybe just 
> delete this todo? :)
I've removed the TODO, but I reserve the right to think this would be a good 
idea :). Speficially because I think you could still model in YAML anything you 
could put in a minidump file, by explicitly providing the ThreadContext even 
when it has a default.

(This is in contrast to the other validation stuff I had in earlier revisions, 
where I was just misunderstanding the point of yaml validation -- so thanks for 
explaining it!) 



Comment at: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp:310-316
+  ASSERT_FALSE(ExpectedFile);
+  auto Unhandled =
+  handleErrors(ExpectedFile.takeError(), [](const StringError &error) {
+EXPECT_EQ(static_cast(std::errc::invalid_argument),
+  error.convertToErrorCode().value());
+  });
+  EXPECT_THAT_ERROR(std::move(Unhandled), Succeeded());

labath wrote:
> Maybe:
> ```
> EXPECT_THAT_EXPECTED(ExpectedFile, Failed(
> testing::Property(&StringError::convertToErrorCode,
>  make_error_code(errc::invalid_argument;
> ```
> ?
> 
> Though, this might actually be best off as a lit test where you just 
> FileCheck the exact error message.
Moved to lit, thanks for the suggestion.  The obvious place seemed to be 
llvm/test/ObjectYAML, but there weren't any minidump tests there so I added a 
minidump subdirectory for the new test.  Please let me know if there was a 
better place that I just overlooked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68657



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

[Lldb-commits] [PATCH] D68939: [LLDB] [PECOFF] Use a "pc" vendor name in aarch64 triples

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D68939#1708020 , @labath wrote:

> In D68939#1707998 , @mstorsjo wrote:
>
> > In D68939#1707985 , @labath wrote:
> >
> > > Would you say that "pc" is a reasonable value for the "vendor" field for 
> > > the win+aarch64 combo? I am asking because I don't have a clue about 
> > > that, and given that this platform is being brought up right now, 
> > > changing this now would be way easier than doing it later. (The reason 
> > > why things don't work is the incompatibility between the two things that 
> > > compute the ArchSpec, but that can also be fixed by changing the other 
> > > mechanism, if that is better/more correct.) My guess is the other 
> > > mechanism is ArchSpec::SetArchitecture function, line 928...
> >
> >
> > I'd say "pc" is fine here; such machines are available for sale (although 
> > with a bit scarce availability) as normal power efficient laptops - google 
> > for e.g. HP Envy X2, for one that is available with both arm and x86 cpu 
> > options.
>
>
> So, am I correct to assume that "pc" is used/can be used for any "personal 
> computer"? I was under the impression that pc stands for the PCs which are 
> descended/compatible with the original IBM PCs...


I don't really know TBH - in that case it should only be valid on x86... Is 
there any canonical definition anywhere? Not sure if it has any practical 
effect anywhere though, other than making the triples different, messing up 
this case.

I guess the alternative would be to remove line 928 in ArchSpec.cpp. I can test 
that to see if it fixes the issue later tonight.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68939



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

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp:310-316
+  ASSERT_FALSE(ExpectedFile);
+  auto Unhandled =
+  handleErrors(ExpectedFile.takeError(), [](const StringError &error) {
+EXPECT_EQ(static_cast(std::errc::invalid_argument),
+  error.convertToErrorCode().value());
+  });
+  EXPECT_THAT_ERROR(std::move(Unhandled), Succeeded());

JosephTremoulet wrote:
> labath wrote:
> > Maybe:
> > ```
> > EXPECT_THAT_EXPECTED(ExpectedFile, Failed(
> > testing::Property(&StringError::convertToErrorCode,
> >  make_error_code(errc::invalid_argument;
> > ```
> > ?
> > 
> > Though, this might actually be best off as a lit test where you just 
> > FileCheck the exact error message.
> Moved to lit, thanks for the suggestion.  The obvious place seemed to be 
> llvm/test/ObjectYAML, but there weren't any minidump tests there so I added a 
> minidump subdirectory for the new test.  Please let me know if there was a 
> better place that I just overlooked.
there are some in `test/tools/yaml2obj`. I'd put it next to those.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68657



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

[Lldb-commits] [lldb] r374776 - minidump: Use yaml for memory info tests

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Oct 14 07:16:39 2019
New Revision: 374776

URL: http://llvm.org/viewvc/llvm-project?rev=374776&view=rev
Log:
minidump: Use yaml for memory info tests

Also, delete some minidump binary files that are no longer used in any
test.

Removed:
lldb/trunk/unittests/Process/minidump/Inputs/dump-content.dmp
lldb/trunk/unittests/Process/minidump/Inputs/linux-x86_64_not_crashed.dmp
Modified:
lldb/trunk/unittests/Process/minidump/CMakeLists.txt
lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp

Modified: lldb/trunk/unittests/Process/minidump/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/CMakeLists.txt?rev=374776&r1=374775&r2=374776&view=diff
==
--- lldb/trunk/unittests/Process/minidump/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Process/minidump/CMakeLists.txt Mon Oct 14 07:16:39 
2019
@@ -19,7 +19,6 @@ set(test_inputs
fizzbuzz_no_heap.dmp
fizzbuzz_wow64.dmp
linux-x86_64.dmp
-   linux-x86_64_not_crashed.dmp
regions-memlist64.dmp
)
 

Removed: lldb/trunk/unittests/Process/minidump/Inputs/dump-content.dmp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/Inputs/dump-content.dmp?rev=374775&view=auto
==
Binary file - no diff available.

Removed: 
lldb/trunk/unittests/Process/minidump/Inputs/linux-x86_64_not_crashed.dmp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/Inputs/linux-x86_64_not_crashed.dmp?rev=374775&view=auto
==
Binary file - no diff available.

Modified: lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp?rev=374776&r1=374775&r2=374776&view=diff
==
--- lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp (original)
+++ lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp Mon Oct 14 
07:16:39 2019
@@ -365,7 +365,53 @@ constexpr auto no = MemoryRegionInfo::eN
 constexpr auto unknown = MemoryRegionInfo::eDontKnow;
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfo) {
-  SetUpData("fizzbuzz_wow64.dmp");
+  ASSERT_THAT_ERROR(SetUpFromYaml(R"(
+--- !minidump
+Streams:
+  - Type:MemoryInfoList
+Memory Ranges:
+  - Base Address:0x
+Allocation Protect: [  ]
+Region Size: 0x0001
+State:   [ MEM_FREE ]
+Protect: [ PAGE_NO_ACCESS ]
+Type:[  ]
+  - Base Address:0x0001
+Allocation Protect: [ PAGE_READ_WRITE ]
+Region Size: 0x0001
+State:   [ MEM_COMMIT ]
+Type:[ MEM_MAPPED ]
+  - Base Address:0x0002
+Allocation Protect: [ PAGE_READ_WRITE ]
+Region Size: 0x0001
+State:   [ MEM_COMMIT ]
+Type:[ MEM_MAPPED ]
+  - Base Address:0x0003
+Allocation Protect: [ PAGE_READ_WRITE ]
+Region Size: 0x1000
+State:   [ MEM_COMMIT ]
+Type:[ MEM_MAPPED ]
+  - Base Address:0x0004
+Allocation Protect: [ PAGE_EXECUTE_WRITE_COPY ]
+Region Size: 0x1000
+State:   [ MEM_COMMIT ]
+Protect: [ PAGE_READ_ONLY ]
+Type:[ MEM_IMAGE ]
+  - Base Address:0x7FFE
+Allocation Protect: [ PAGE_READ_ONLY ]
+Region Size: 0x1000
+State:   [ MEM_COMMIT ]
+Type:[ MEM_PRIVATE ]
+  - Base Address:0x7FFE1000
+Allocation Base: 0x7FFE
+Allocation Protect: [ PAGE_READ_ONLY ]
+Region Size: 0xF000
+State:   [ MEM_RESERVE ]
+Protect: [ PAGE_NO_ACCESS ]
+Type:[ MEM_PRIVATE ]
+...
+)"),
+llvm::Succeeded());
 
   check_region(*parser, 0x, 0x0001, no, no, no, no);
   check_region(*parser, 0x0001, 0x0002, yes, yes, no, yes);


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

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet updated this revision to Diff 224857.
JosephTremoulet marked 2 inline comments as done.
JosephTremoulet added a comment.

- Move test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68657

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
  llvm/include/llvm/ObjectYAML/MinidumpYAML.h
  llvm/lib/ObjectYAML/MinidumpEmitter.cpp
  llvm/lib/ObjectYAML/MinidumpYAML.cpp
  llvm/test/tools/obj2yaml/basic-minidump.yaml
  llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml
  llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,200 @@
   (ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
   makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
 }
+
+// Test that we can parse a normal-looking ExceptionStream
+TEST(MinidumpYAML, ExceptionStream) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:Exception
+Thread ID:  0x7
+Exception Record:
+  Exception Code:  0x23
+  Exception Flags: 0x5
+  Exception Record: 0x0102030405060708
+  Exception Address: 0x0a0b0c0d0e0f1011
+  Number of Parameters: 2
+  Parameter 0: 0x22
+  Parameter 1: 0x24
+Thread Context:  3DeadBeefDefacedABadCafe)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  Expected ExpectedStream =
+  File.getExceptionStream();
+
+  ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+  const minidump::ExceptionStream &Stream = *ExpectedStream;
+  EXPECT_EQ(0x7u, Stream.ThreadId);
+  const minidump::Exception &Exception = Stream.ExceptionRecord;
+  EXPECT_EQ(0x23u, Exception.ExceptionCode);
+  EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+  EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+  EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+  EXPECT_EQ(2u, Exception.NumberParameters);
+  EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+  EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+  Expected> ExpectedContext =
+  File.getRawData(Stream.ThreadContext);
+  ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+  EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+   0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:Exception
+Thread ID:  0x7
+Exception Record:
+  Exception Code:  0x23
+  Exception Flags: 0x5
+  Exception Record: 0x0102030405060708
+  Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context:  3DeadBeefDefacedABadCafe)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  Expected ExpectedStream =
+  File.getExceptionStream();
+
+  ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+  const minidump::ExceptionStream &Stream = *ExpectedStream;
+  EXPECT_EQ(0x7u, Stream.ThreadId);
+  const minidump::Exception &Exception = Stream.ExceptionRecord;
+  EXPECT_EQ(0x23u, Exception.ExceptionCode);
+  EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+  EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+  EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+  EXPECT_EQ(0u, Exception.NumberParameters);
+
+  Expected> ExpectedContext =
+  File.getRawData(Stream.ThreadContext);
+  ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+  EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+   0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:Exception
+Thread ID:  0x8
+Exception Record:
+  Exception Code: 0
+  Number of Parameters: 16
+  Parameter 0: 0x0
+  Parameter 1: 0xff
+  Parameter 2: 0xee
+  Parameter 3: 0xdd
+  Parameter 4: 0xcc
+  Parameter 5: 0xbb
+  Parameter 6: 0xaa
+  Parameter 7: 0x99
+  Parameter 8: 0x88
+  Parameter 9: 0x77
+  Parameter 10: 0x66
+  Parameter 11: 0x55
+  Parameter 12: 0x44
+  P

[Lldb-commits] [lldb] r374583 - make ConstString allocate memory in non-tiny chunks

[Lubos Lunak via lldb-commits]

Author: llunak
Date: Fri Oct 11 12:34:39 2019
New Revision: 374583

URL: http://llvm.org/viewvc/llvm-project?rev=374583&view=rev
Log:
make ConstString allocate memory in non-tiny chunks

BumpPtrAllocator allocates in 4KiB chunks, which with any larger
project is going to result in a large number of allocations.
Increasing allocation size this way can save 10%-20% of symbol
load time for a huge C++ project with correctly built debuginfo.

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

Modified:
lldb/trunk/source/Utility/ConstString.cpp

Modified: lldb/trunk/source/Utility/ConstString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ConstString.cpp?rev=374583&r1=374582&r2=374583&view=diff
==
--- lldb/trunk/source/Utility/ConstString.cpp (original)
+++ lldb/trunk/source/Utility/ConstString.cpp Fri Oct 11 12:34:39 2019
@@ -31,7 +31,10 @@ using namespace lldb_private;
 class Pool {
 public:
   typedef const char *StringPoolValueType;
-  typedef llvm::StringMap
+  // BumpPtrAllocator allocates in 4KiB chunks, any larger C++ project is going
+  // to have megabytes of symbols, so allocate in larger chunks.
+  typedef llvm::BumpPtrAllocatorImpl Allocator;
+  typedef llvm::StringMap
   StringPool;
   typedef llvm::StringMapEntry StringPoolEntryType;
 
@@ -152,7 +155,9 @@ protected:
 
   struct PoolEntry {
 mutable llvm::sys::SmartRWMutex m_mutex;
-StringPool m_string_map;
+// StringMap by default starts with 16 buckets, any larger project is
+// going to have many symbols, so start with a larger value.
+StringPool m_string_map = StringPool( 65536 );
   };
 
   std::array m_string_pools;


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

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks. I think this is great. @grimar, do you have any more comments?




Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:377-383
+  // TODO: We could provide a reasonable default for ThreadContext by searching
+  // the Thread stream for a thread with the given ID and using its Context.
+  // That would require a couple changes:
+  //   1. We'd need to pass the whole dump around as a context argument.
+  //   2. We'd need to ensure that the Thread stream got processed before
+  //  the Exception stream (or make Exception's ThreadContext required
+  //  when the Exception stream is processed before the Thread stream).

JosephTremoulet wrote:
> labath wrote:
> > I've been thinking about this for a while now, and while that idea has some 
> > appeal, I am not sure if this would ever really be a "good" idea. 
> > Currently, this format allows you to easily create 
> > syntactically-valid-but-probably-nonsensical minidumps (multiple thread 
> > list streams, multiple threads with the same ID, etc..). All of that would 
> > be more difficult if we started depending on strict "correctness" of other 
> > parts of the minidump in order to compute something here.
> > 
> > Even if I was doing this, I'd probably implement this differently -- make 
> > the context always optional, but then check for consistency higher up (the 
> > validate call of the entire minidump object or something). Anyway, maybe 
> > just delete this todo? :)
> I've removed the TODO, but I reserve the right to think this would be a good 
> idea :). Speficially because I think you could still model in YAML anything 
> you could put in a minidump file, by explicitly providing the ThreadContext 
> even when it has a default.
> 
> (This is in contrast to the other validation stuff I had in earlier 
> revisions, where I was just misunderstanding the point of yaml validation -- 
> so thanks for explaining it!) 
Fair enough. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68657



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

[Lldb-commits] [PATCH] D68868: Fix build under musl

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a reviewer: JDevlieghere.
teemperor added a comment.

(Adding Jonas because reproducers)

Btw, you can just upload a diff to phabricator (e.g. the output you get with 
`git diff -U > fix-build.patch` when using the monorepo 
 ) and that's how the code change here 
gets properly rendered.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68868



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

[Lldb-commits] [PATCH] D68939: [LLDB] [PECOFF] Use a "pc" vendor name in aarch64 triples

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

Removing line 928 in ArchSpec.cpp or changing it to unknown doesn't seem to 
help though, so apparently there's something else forcing a pc vendor as well, 
somewhere.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68939



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

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

This is not correct. A lot of yaml2obj tests fail with:

> yaml2obj: error: unknown section referenced: '.symtab' by YAML section 
> '.rela.text'

The problem is that .symtab can be the sh_link field of relocation sections, 
.llvm_addrsig, and some sections that may be added in the future.

Making .symtab conditionally define is difficult. I'll take a closer look 
tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943



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

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

So, are there any concerns about this (beside the typo which I'll fix 
pre-commit) and Shafik's request for documentation (which will be another NFC 
commit)?


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

https://reviews.llvm.org/D68130



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

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

In D68943#1708068 , @MaskRay wrote:

> This is not correct. A lot of yaml2obj tests fail with:
>
> > yaml2obj: error: unknown section referenced: '.symtab' by YAML section 
> > '.rela.text'


@MaskRay I'm so sorry. You're absolutely right. I was working only in LLDB for 
the last months and used `check-lldb` only. That's probably because I missed 
those errors.

> The problem is that .symtab can be the sh_link field of relocation sections, 
> .llvm_addrsig, and some sections that may be added in the future.
> 
> Making .symtab conditionally define is difficult. I'll take a closer look 
> tomorrow.

Thank you. Meanwhile I will run more tests to see what exactly fails.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943



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

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

In D67347#1706405 , @stella.stamenova 
wrote:

> It looks like this changed fixed at least one of the XFAILed tests on Windows:
>
> http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/9751
>
> So now the test results would be red because of the unexpectedly passing test 
> (if there wasn't another failure). Could you have a look at whether any of 
> the other tests that were XFAILed for the same bug are also now passing and 
> remove the expected failure tags as appropriate?


This is still "failing" on the windows bot. Please fix the issue or revert the 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67347



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

[Lldb-commits] [PATCH] D68738: update TestRunCommandInterpreterAPI to use SBFile

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

This is still failing on the Windows bot. Please fix it ASAP or revert it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68738



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

[Lldb-commits] [PATCH] D68738: update TestRunCommandInterpreterAPI to use SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

In D68738#1708159 , @stella.stamenova 
wrote:

> This is still failing on the Windows bot. Please fix it ASAP or revert it.


Sorry,i didn’t see this.  I’ll fix it today


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68738



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

[Lldb-commits] [PATCH] D68868: Fix build under musl

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Given that the `FILE` struct lives in `stdio.h` this makes sense.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68868



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Make the RedirectingFileSystem hold on to its own working directory.

[Bruno Cardoso Lopes via Phabricator via lldb-commits]

bruno accepted this revision.
bruno added a comment.

This approach looks overall much better! Unless @sammccall has any extra 
comments, it LGTM.


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

https://reviews.llvm.org/D65677



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

Re: [Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

[Aleksandr Urakov via lldb-commits]

Sorry, I'm OOO now. I'll take a look tomorrow, thanks for catching that!

On Mon, 14 Oct 2019, 19:20 Stella Stamenova via Phabricator, <
revi...@reviews.llvm.org> wrote:

> stella.stamenova added a comment.
>
> In D67347#1706405 ,
> @stella.stamenova wrote:
>
> > It looks like this changed fixed at least one of the XFAILed tests on
> Windows:
> >
> > http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/9751
> >
> > So now the test results would be red because of the unexpectedly passing
> test (if there wasn't another failure). Could you have a look at whether
> any of the other tests that were XFAILed for the same bug are also now
> passing and remove the expected failure tags as appropriate?
>
>
> This is still "failing" on the windows bot. Please fix the issue or revert
> the change.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D67347/new/
>
> https://reviews.llvm.org/D67347
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68951: Fix test breakage caused by r374424

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, jasonmolenda, labath, 
stella.stamenova.
Herald added a project: LLDB.

The build directory name is based on the test method name, so having
two test methods with the same name in the same test file is a
problem, even if they're in different test classes.

On linux and darwin this conflict can go unnoticed, but windows 
has different filesystem semantics and it will fail when one
process tries to delete files still held open by another.

The problem is fixed just by changing the name of one of the test
methods.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68951

Files:
  
lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py


Index: 
lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
===
--- 
lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
+++ 
lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
@@ -30,7 +30,7 @@
 self.dbg.SetErrorFileHandle (self.devnull, False)
 
 @add_test_categories(['pyapi'])
-def test_run_session_with_error_and_quit(self):
+def test_run_session_with_error_and_quit_legacy(self):
 """Run non-existing and quit command returns appropriate values"""
 
 n_errors, quit_requested, has_crashed = self.dbg.RunCommandInterpreter(


Index: lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
+++ lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
@@ -30,7 +30,7 @@
 self.dbg.SetErrorFileHandle (self.devnull, False)
 
 @add_test_categories(['pyapi'])
-def test_run_session_with_error_and_quit(self):
+def test_run_session_with_error_and_quit_legacy(self):
 """Run non-existing and quit command returns appropriate values"""
 
 n_errors, quit_requested, has_crashed = self.dbg.RunCommandInterpreter(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68738: update TestRunCommandInterpreterAPI to use SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

In D68738#1708159 , @stella.stamenova 
wrote:

> This is still failing on the Windows bot. Please fix it ASAP or revert it.


fix here https://reviews.llvm.org/D68951


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68738



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

[Lldb-commits] [lldb] r374803 - Fix test breakage caused by r374424

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Mon Oct 14 11:53:27 2019
New Revision: 374803

URL: http://llvm.org/viewvc/llvm-project?rev=374803&view=rev
Log:
Fix test breakage caused by r374424

Summary:
The build directory name is based on the test method name, so having
two test methods with the same name in the same test file is a
problem, even if they're in different test classes.

On linux and darwin this conflict can go unnoticed, but windows
has different filesystem semantics and it will fail when one
process tries to delete files still held open by another.

The problem is fixed just by changing the name of one of the test
methods.

Reviewers: JDevlieghere, jasonmolenda, labath, stella.stamenova

Reviewed By: stella.stamenova

Subscribers: lldb-commits

Tags: #lldb

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py?rev=374803&r1=374802&r2=374803&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
 Mon Oct 14 11:53:27 2019
@@ -30,7 +30,7 @@ class CommandRunInterpreterLegacyAPICase
 self.dbg.SetErrorFileHandle (self.devnull, False)
 
 @add_test_categories(['pyapi'])
-def test_run_session_with_error_and_quit(self):
+def test_run_session_with_error_and_quit_legacy(self):
 """Run non-existing and quit command returns appropriate values"""
 
 n_errors, quit_requested, has_crashed = self.dbg.RunCommandInterpreter(


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

[Lldb-commits] [PATCH] D68858: [lldb] Creates _liblldb symlink from cmake

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha added a comment.

F10259577: D68858d.diff 

@hhb, please, take a look. Slightly changed your patch to make it work for 
Visual Studio.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68858



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

[Lldb-commits] [PATCH] D68951: Fix test breakage caused by r374424

[Lawrence D'Anna via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd5768e3d0e88: Fix test breakage caused by r374424 (authored 
by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68951

Files:
  
lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py


Index: 
lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
===
--- 
lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
+++ 
lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
@@ -30,7 +30,7 @@
 self.dbg.SetErrorFileHandle (self.devnull, False)
 
 @add_test_categories(['pyapi'])
-def test_run_session_with_error_and_quit(self):
+def test_run_session_with_error_and_quit_legacy(self):
 """Run non-existing and quit command returns appropriate values"""
 
 n_errors, quit_requested, has_crashed = self.dbg.RunCommandInterpreter(


Index: lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
+++ lldb/packages/Python/lldbsuite/test/python_api/interpreter/TestRunCommandInterpreterAPI.py
@@ -30,7 +30,7 @@
 self.dbg.SetErrorFileHandle (self.devnull, False)
 
 @add_test_categories(['pyapi'])
-def test_run_session_with_error_and_quit(self):
+def test_run_session_with_error_and_quit_legacy(self):
 """Run non-existing and quit command returns appropriate values"""
 
 n_errors, quit_requested, has_crashed = self.dbg.RunCommandInterpreter(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r374816 - remove FILE* usage from ReportEventState() and HandleProcessEvent()

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Mon Oct 14 13:15:28 2019
New Revision: 374816

URL: http://llvm.org/viewvc/llvm-project?rev=374816&view=rev
Log:
remove FILE* usage from ReportEventState() and HandleProcessEvent()

Summary:
This patch adds FileSP and SBFile versions of the API methods
ReportEventState and  HandleProcessEvent.   It points the SWIG
wrappers at these instead of the ones that use FILE* streams.

Reviewers: JDevlieghere, jasonmolenda, labath, jingham

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/include/lldb/API/SBDebugger.h
lldb/trunk/include/lldb/API/SBFile.h
lldb/trunk/include/lldb/API/SBProcess.h

lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_debugger.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_process.py
lldb/trunk/scripts/interface/SBDebugger.i
lldb/trunk/scripts/interface/SBProcess.i
lldb/trunk/source/API/SBDebugger.cpp
lldb/trunk/source/API/SBProcess.cpp

Modified: lldb/trunk/include/lldb/API/SBDebugger.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBDebugger.h?rev=374816&r1=374815&r2=374816&view=diff
==
--- lldb/trunk/include/lldb/API/SBDebugger.h (original)
+++ lldb/trunk/include/lldb/API/SBDebugger.h Mon Oct 14 13:15:28 2019
@@ -117,7 +117,14 @@ public:
   lldb::SBListener GetListener();
 
   void HandleProcessEvent(const lldb::SBProcess &process,
-  const lldb::SBEvent &event, FILE *out, FILE *err);
+  const lldb::SBEvent &event, FILE *out,
+  FILE *err); // DEPRECATED
+
+  void HandleProcessEvent(const lldb::SBProcess &process,
+  const lldb::SBEvent &event, SBFile out, SBFile err);
+
+  void HandleProcessEvent(const lldb::SBProcess &process,
+  const lldb::SBEvent &event, FileSP out, FileSP err);
 
   lldb::SBTarget CreateTarget(const char *filename, const char *target_triple,
   const char *platform_name,

Modified: lldb/trunk/include/lldb/API/SBFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBFile.h?rev=374816&r1=374815&r2=374816&view=diff
==
--- lldb/trunk/include/lldb/API/SBFile.h (original)
+++ lldb/trunk/include/lldb/API/SBFile.h Mon Oct 14 13:15:28 2019
@@ -16,6 +16,7 @@ namespace lldb {
 class LLDB_API SBFile {
   friend class SBDebugger;
   friend class SBCommandReturnObject;
+  friend class SBProcess;
 
 public:
   SBFile();

Modified: lldb/trunk/include/lldb/API/SBProcess.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBProcess.h?rev=374816&r1=374815&r2=374816&view=diff
==
--- lldb/trunk/include/lldb/API/SBProcess.h (original)
+++ lldb/trunk/include/lldb/API/SBProcess.h Mon Oct 14 13:15:28 2019
@@ -67,6 +67,10 @@ public:
 
   void ReportEventState(const lldb::SBEvent &event, FILE *out) const;
 
+  void ReportEventState(const lldb::SBEvent &event, SBFile file) const;
+
+  void ReportEventState(const lldb::SBEvent &event, FileSP file) const;
+
   void AppendEventStateReport(const lldb::SBEvent &event,
   lldb::SBCommandReturnObject &result);
 

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_debugger.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_debugger.py?rev=374816&r1=374815&r2=374816&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_debugger.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_debugger.py
 Mon Oct 14 13:15:28 2019
@@ -19,7 +19,10 @@ def fuzz_obj(obj):
 obj.GetCommandInterpreter()
 obj.HandleCommand("nothing here")
 listener = obj.GetListener()
-obj.HandleProcessEvent(lldb.SBProcess(), lldb.SBEvent(), None, None)
+try:
+obj.HandleProcessEvent(lldb.SBProcess(), lldb.SBEvent(), None, None)
+except Exception:
+pass
 obj.CreateTargetWithFileAndTargetTriple("a.out", "A-B-C")
 obj.CreateTargetWithFileAndArch("b.out", "arm")
 obj.CreateTarget("c.out")

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_process.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_process.py?rev=374816&r1=374815&r2=374816&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_p

[Lldb-commits] [lldb] r374817 - uint32_t options -> File::OpenOptions options

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Mon Oct 14 13:15:34 2019
New Revision: 374817

URL: http://llvm.org/viewvc/llvm-project?rev=374817&view=rev
Log:
uint32_t options -> File::OpenOptions options

Summary:
This patch re-types everywhere that passes a File::OpenOptions
as a uint32_t so it actually uses File::OpenOptions.

It also converts some OpenOptions related functions that fail
by returning 0 or NULL into llvm::Expected

split off from https://reviews.llvm.org/D68737

Reviewers: JDevlieghere, jasonmolenda, labath

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/include/lldb/Core/StreamFile.h
lldb/trunk/include/lldb/Host/File.h
lldb/trunk/include/lldb/Host/FileCache.h
lldb/trunk/include/lldb/Host/FileSystem.h
lldb/trunk/include/lldb/Target/Platform.h
lldb/trunk/include/lldb/Target/RemoteAwarePlatform.h
lldb/trunk/source/API/SBFile.cpp
lldb/trunk/source/API/SBStream.cpp
lldb/trunk/source/Commands/CommandObjectMemory.cpp
lldb/trunk/source/Commands/CommandObjectSettings.cpp
lldb/trunk/source/Core/StreamFile.cpp
lldb/trunk/source/Host/common/File.cpp
lldb/trunk/source/Host/common/FileCache.cpp
lldb/trunk/source/Host/common/FileSystem.cpp

lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/trunk/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/trunk/source/Target/Platform.cpp
lldb/trunk/source/Target/RemoteAwarePlatform.cpp
lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/include/lldb/Core/StreamFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/StreamFile.h?rev=374817&r1=374816&r2=374817&view=diff
==
--- lldb/trunk/include/lldb/Core/StreamFile.h (original)
+++ lldb/trunk/include/lldb/Core/StreamFile.h Mon Oct 14 13:15:34 2019
@@ -30,7 +30,7 @@ public:
 
   StreamFile(const char *path);
 
-  StreamFile(const char *path, uint32_t options,
+  StreamFile(const char *path, File::OpenOptions options,
  uint32_t permissions = lldb::eFilePermissionsFileDefault);
 
   StreamFile(FILE *fh, bool transfer_ownership);

Modified: lldb/trunk/include/lldb/Host/File.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/File.h?rev=374817&r1=374816&r2=374817&view=diff
==
--- lldb/trunk/include/lldb/Host/File.h (original)
+++ lldb/trunk/include/lldb/Host/File.h Mon Oct 14 13:15:34 2019
@@ -13,6 +13,7 @@
 #include "lldb/Utility/IOObject.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-private.h"
+#include "llvm/ADT/BitmaskEnum.h"
 
 #include 
 #include 
@@ -21,6 +22,8 @@
 
 namespace lldb_private {
 
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
 /// \class File File.h "lldb/Host/File.h"
 /// An abstract base class for files.
 ///
@@ -35,7 +38,12 @@ public:
 
   // NB this enum is used in the lldb platform gdb-remote packet
   // vFile:open: and existing values cannot be modified.
-  enum OpenOptions {
+  //
+  // FIXME
+  // These values do not match the values used by GDB
+  // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
+  // * rdar://problem/46788934
+  enum OpenOptions : uint32_t {
 eOpenOptionRead = (1u << 0),  // Open file for reading
 eOpenOptionWrite = (1u << 1), // Open file for writing
 eOpenOptionAppend =
@@ -47,11 +55,12 @@ public:
 (1u << 6), // Can create file only if it doesn't already exist
 eOpenOptionDontFollowSymlinks = (1u << 7),
 eOpenOptionCloseOnExec =
-(1u << 8) // Close the file when executing a new process
+(1u << 8), // Close the file when executing a new process
+LLVM_MARK_AS_BITMASK_ENUM(/* largest_value= */ eOpenOptionCloseOnExec)
   };
 
-  static mode_t ConvertOpenOptionsForPOSIXOpen(uint32_t open_options);
-  static uint32_t GetOptionsFromMode(llvm::StringRef mode);
+  static mode_t ConvertOpenOptionsForPOSIXOpen(OpenOptions open_options);
+  static llvm::Expected GetOptionsFromMode(llvm::StringRef mode);
   static bool DescriptorIsValid(int descriptor) { return descriptor >= 0; };
 
   File()
@@ -358,13 +367,13 @@ class NativeFile : public File {
 public:
   NativeFile()
   : m_descriptor(kInvalidDescriptor), m_own_descriptor(false),
-m_stream(kInvalidStream), m_options(0), m_own_stream(false) {}
+m_stream(kInvalidStream), m_options(), m_own_stream(false) {}
 

[Lldb-commits] [PATCH] D68546: remove FILE* usage from ReportEventState() and HandleProcessEvent()

[Lawrence D'Anna via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG322f12afc367: remove FILE* usage from ReportEventState() and 
HandleProcessEvent() (authored by lawrence_danna).

Changed prior to commit:
  https://reviews.llvm.org/D68546?vs=224668&id=224895#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68546

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/API/SBProcess.h
  
lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_debugger.py
  
lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_process.py
  lldb/scripts/interface/SBDebugger.i
  lldb/scripts/interface/SBProcess.i
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBProcess.cpp

Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -29,11 +29,11 @@
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Stream.h"
 
-
 #include "lldb/API/SBBroadcaster.h"
 #include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBEvent.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
 #include "lldb/API/SBMemoryRegionInfoList.h"
@@ -331,23 +331,34 @@
   return LLDB_RECORD_RESULT(trace_instance);
 }
 
+void SBProcess::ReportEventState(const SBEvent &event, SBFile out) const {
+  LLDB_RECORD_METHOD_CONST(void, SBProcess, ReportEventState,
+   (const SBEvent &, SBFile), event, out);
+
+  return ReportEventState(event, out.m_opaque_sp);
+}
+
 void SBProcess::ReportEventState(const SBEvent &event, FILE *out) const {
   LLDB_RECORD_METHOD_CONST(void, SBProcess, ReportEventState,
(const lldb::SBEvent &, FILE *), event, out);
+  FileSP outfile = std::make_shared(out, false);
+  return ReportEventState(event, outfile);
+}
+
+void SBProcess::ReportEventState(const SBEvent &event, FileSP out) const {
 
-  if (out == nullptr)
+  LLDB_RECORD_METHOD_CONST(void, SBProcess, ReportEventState,
+   (const SBEvent &, FileSP), event, out);
+
+  if (!out || !out->IsValid())
 return;
 
   ProcessSP process_sp(GetSP());
   if (process_sp) {
+StreamFile stream(out);
 const StateType event_state = SBProcess::GetStateFromEvent(event);
-char message[1024];
-int message_len = ::snprintf(
-message, sizeof(message), "Process %" PRIu64 " %s\n",
+stream.Printf("Process %" PRIu64 " %s\n",
 process_sp->GetID(), SBDebugger::StateAsCString(event_state));
-
-if (message_len > 0)
-  ::fwrite(message, 1, message_len, out);
   }
 }
 
@@ -1310,6 +1321,10 @@
(lldb::SBTraceOptions &, lldb::SBError &));
   LLDB_REGISTER_METHOD_CONST(void, SBProcess, ReportEventState,
  (const lldb::SBEvent &, FILE *));
+  LLDB_REGISTER_METHOD_CONST(void, SBProcess, ReportEventState,
+ (const lldb::SBEvent &, FileSP));
+  LLDB_REGISTER_METHOD_CONST(void, SBProcess, ReportEventState,
+ (const lldb::SBEvent &, SBFile));
   LLDB_REGISTER_METHOD(
   void, SBProcess, AppendEventStateReport,
   (const lldb::SBEvent &, lldb::SBCommandReturnObject &));
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -494,8 +494,7 @@
 while (lldb_listener_sp->GetEventForBroadcaster(
 process_sp.get(), event_sp, std::chrono::seconds(0))) {
   SBEvent event(event_sp);
-  HandleProcessEvent(process, event, GetOutputFileHandle(),
- GetErrorFileHandle());
+  HandleProcessEvent(process, event, GetOutputFile(), GetErrorFile());
 }
   }
 }
@@ -513,6 +512,17 @@
 }
 
 void SBDebugger::HandleProcessEvent(const SBProcess &process,
+const SBEvent &event, SBFile out,
+SBFile err) {
+  LLDB_RECORD_METHOD(
+  void, SBDebugger, HandleProcessEvent,
+  (const lldb::SBProcess &, const lldb::SBEvent &, SBFile, SBFile), process,
+  event, out, err);
+
+  return HandleProcessEvent(process, event, out.m_opaque_sp, err.m_opaque_sp);
+}
+
+void SBDebugger::HandleProcessEvent(const SBProcess &process,
 const SBEvent &event, FILE *out,
 FILE *err) {
   LLDB_RECORD_METHOD(
@@ -520,6 +530,20 @@
   (const lldb::SBProcess &, const lldb::SBEvent &, FILE *, FILE *), process,
   event, out, err);
 
+  FileSP outfile = std::make_shared(out, false);
+  FileSP errfile = std::make_shared(err, false);
+  return HandleProcessEvent(process, event, outfile, errf

[Lldb-commits] [PATCH] D68853: uint32_t options -> File::OpenOptions options

[Lawrence D'Anna via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG62c9fe4273e8: uint32_t options -> File::OpenOptions 
options (authored by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68853

Files:
  lldb/include/lldb/Core/StreamFile.h
  lldb/include/lldb/Host/File.h
  lldb/include/lldb/Host/FileCache.h
  lldb/include/lldb/Host/FileSystem.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Target/RemoteAwarePlatform.h
  lldb/source/API/SBFile.cpp
  lldb/source/API/SBStream.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Core/StreamFile.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Host/common/FileCache.cpp
  lldb/source/Host/common/FileSystem.cpp
  
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.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/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/RemoteAwarePlatform.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -996,10 +996,9 @@
   }
 
   StreamFile out_file(path.c_str(),
-  File::OpenOptions::eOpenOptionTruncate |
-  File::OpenOptions::eOpenOptionWrite |
-  File::OpenOptions::eOpenOptionCanCreate |
-  File::OpenOptions::eOpenOptionCloseOnExec,
+  File::eOpenOptionTruncate | File::eOpenOptionWrite |
+  File::eOpenOptionCanCreate |
+  File::eOpenOptionCloseOnExec,
   lldb::eFilePermissionsFileDefault);
   if (!out_file.GetFile().IsValid()) {
 error.SetErrorStringWithFormat("Unable to open output file: %s.",
Index: lldb/source/Target/RemoteAwarePlatform.cpp
===
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -61,8 +61,8 @@
 }
 
 lldb::user_id_t RemoteAwarePlatform::OpenFile(const FileSpec &file_spec,
-  uint32_t flags, uint32_t mode,
-  Status &error) {
+  File::OpenOptions flags,
+  uint32_t mode, Status &error) {
   if (IsHost())
 return FileCache::GetInstance().OpenFile(file_spec, flags, mode, error);
   if (m_remote_platform_sp)
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1220,7 +1220,7 @@
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM));
   LLDB_LOGF(log, "[PutFile] Using block by block transfer\n");
 
-  uint32_t source_open_options =
+  auto source_open_options =
   File::eOpenOptionRead | File::eOpenOptionCloseOnExec;
   namespace fs = llvm::sys::fs;
   if (fs::is_symlink_file(source.GetPath()))
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1090,8 +1090,12 @@
   // File object knows about that.
   PythonString py_mode = GetAttributeValue("mode").AsType();
   auto options = File::GetOptionsFromMode(py_mode.GetString());
-  auto file = std::unique_ptr(
-  new NativeFile(PyObject_AsFileDescriptor(m_py_obj), options, false));
+  if (!options) {
+llvm::consumeError(options.takeError());
+return nullptr;
+  }
+  auto file = std::unique_ptr(new NativeFile(
+  PyObject_AsFileDescriptor(m_py_obj), options.get(), false));
   if (!file->IsValid())
 return nullptr;
   return file;
@@ -1165,9 +1169,10 @@
 
 char PythonException::ID = 0;
 
-llvm::Expected GetOptionsForPyObject(const PythonObject &obj) {
-  uint32_t options = 0;
+llvm::Expected
+GetOptionsForPyObject(const PythonObject &obj) {
 #if PY_MAJOR_VERSION >= 3
+  auto options = File::OpenOptions(0);
   auto readable = As(obj.CallMethod("readable"));
   if (!readable)
 return readable.takeError();
@@ -1178,11 +1183,11 @@
 options |= File::eOpenOptionRead;
   if (writable.get())
 options |= File::eOpenOptionWrite

[Lldb-commits] [lldb] r374820 - remove FILE* bindings from SBInstruction.

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Mon Oct 14 13:59:57 2019
New Revision: 374820

URL: http://llvm.org/viewvc/llvm-project?rev=374820&view=rev
Log:
remove FILE* bindings from SBInstruction.

Summary:
This patch replaces the FILE* python bindings for SBInstruction and
SBInstructionList and replaces them with the new, safe SBFile and FileSP
bindings.

I also re-enable `Test_Disassemble_VST1_64`, because now we can use
the file bindings as an additional test of the disassembler, and we
can use the disassembler test as a test of the file bindings.

The bugs referred to in the comments appear to have been fixed.   The
radar is closed now and the bugzilla bug does not reproduce with the
instructions given.

Reviewers: JDevlieghere, jasonmolenda, labath

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/include/lldb/API/SBInstruction.h
lldb/trunk/include/lldb/API/SBInstructionList.h

lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
lldb/trunk/scripts/interface/SBInstruction.i
lldb/trunk/scripts/interface/SBInstructionList.i
lldb/trunk/source/API/SBInstruction.cpp
lldb/trunk/source/API/SBInstructionList.cpp

Modified: lldb/trunk/include/lldb/API/SBInstruction.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBInstruction.h?rev=374820&r1=374819&r2=374820&view=diff
==
--- lldb/trunk/include/lldb/API/SBInstruction.h (original)
+++ lldb/trunk/include/lldb/API/SBInstruction.h Mon Oct 14 13:59:57 2019
@@ -55,6 +55,10 @@ public:
 
   void Print(FILE *out);
 
+  void Print(SBFile out);
+
+  void Print(FileSP out);
+
   bool GetDescription(lldb::SBStream &description);
 
   bool EmulateWithFrame(lldb::SBFrame &frame, uint32_t evaluate_options);

Modified: lldb/trunk/include/lldb/API/SBInstructionList.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBInstructionList.h?rev=374820&r1=374819&r2=374820&view=diff
==
--- lldb/trunk/include/lldb/API/SBInstructionList.h (original)
+++ lldb/trunk/include/lldb/API/SBInstructionList.h Mon Oct 14 13:59:57 2019
@@ -46,6 +46,10 @@ public:
 
   void Print(FILE *out);
 
+  void Print(SBFile out);
+
+  void Print(FileSP out);
+
   bool GetDescription(lldb::SBStream &description);
 
   bool DumpEmulationForAllInstructions(const char *triple);
@@ -56,6 +60,8 @@ protected:
   friend class SBTarget;
 
   void SetDisassembler(const lldb::DisassemblerSP &opaque_sp);
+  bool GetDescription(lldb_private::Stream &description);
+
 
 private:
   lldb::DisassemblerSP m_opaque_sp;

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py?rev=374820&r1=374819&r2=374820&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py
 Mon Oct 14 13:59:57 2019
@@ -9,7 +9,10 @@ def fuzz_obj(obj):
 obj.GetAddress()
 obj.GetByteSize()
 obj.DoesBranch()
-obj.Print(None)
+try:
+obj.Print(None)
+except Exception:
+pass
 obj.GetDescription(lldb.SBStream())
 obj.EmulateWithFrame(lldb.SBFrame(), 0)
 obj.DumpEmulation("armv7")

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py?rev=374820&r1=374819&r2=374820&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py
 Mon Oct 14 13:59:57 2019
@@ -9,7 +9,10 @@ def fuzz_obj(obj):
 obj.GetSize()
 obj.GetInstructionAtIndex(0x)
 obj.AppendInstruction(lldb.SBInstruction())
-obj.Print(None)
+try:
+obj.Print(None)
+except Exception:
+pass
 obj.GetDescription(lldb.SBStream())
 obj.DumpEmulationForAllInstructions("armv7")
 obj.Clear()

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk

[Lldb-commits] [PATCH] D68890: remove FILE* bindings from SBInstruction.

[Lawrence D'Anna via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe7a9115680e2: remove FILE* bindings from SBInstruction. 
(authored by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68890

Files:
  lldb/include/lldb/API/SBInstruction.h
  lldb/include/lldb/API/SBInstructionList.h
  
lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py
  
lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py
  
lldb/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
  lldb/scripts/interface/SBInstruction.i
  lldb/scripts/interface/SBInstructionList.i
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp

Index: lldb/source/API/SBInstructionList.cpp
===
--- lldb/source/API/SBInstructionList.cpp
+++ lldb/source/API/SBInstructionList.cpp
@@ -11,8 +11,10 @@
 #include "lldb/API/SBAddress.h"
 #include "lldb/API/SBInstruction.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/Module.h"
+#include "lldb/Core/StreamFile.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Utility/Stream.h"
 
@@ -118,21 +120,41 @@
 
 void SBInstructionList::Print(FILE *out) {
   LLDB_RECORD_METHOD(void, SBInstructionList, Print, (FILE *), out);
-
   if (out == nullptr)
 return;
+  StreamFile stream(out, false);
+  GetDescription(stream);
 }
 
-bool SBInstructionList::GetDescription(lldb::SBStream &description) {
+void SBInstructionList::Print(SBFile out) {
+  LLDB_RECORD_METHOD(void, SBInstructionList, Print, (SBFile), out);
+  if (!out.IsValid())
+return;
+  StreamFile stream(out.GetFile());
+  GetDescription(stream);
+}
+
+void SBInstructionList::Print(FileSP out_sp) {
+  LLDB_RECORD_METHOD(void, SBInstructionList, Print, (FileSP), out_sp);
+  if (!out_sp || !out_sp->IsValid())
+return;
+  StreamFile stream(out_sp);
+  GetDescription(stream);
+}
+
+bool SBInstructionList::GetDescription(lldb::SBStream &stream) {
   LLDB_RECORD_METHOD(bool, SBInstructionList, GetDescription,
- (lldb::SBStream &), description);
+ (lldb::SBStream &), stream);
+  return GetDescription(stream.ref());
+}
+
+bool SBInstructionList::GetDescription(Stream &sref) {
 
   if (m_opaque_sp) {
 size_t num_instructions = GetSize();
 if (num_instructions) {
   // Call the ref() to make sure a stream is created if one deesn't exist
   // already inside description...
-  Stream &sref = description.ref();
   const uint32_t max_opcode_byte_size =
   m_opaque_sp->GetInstructionList().GetMaxOpcocdeByteSize();
   FormatEntity::Entry format;
@@ -200,6 +222,8 @@
   LLDB_REGISTER_METHOD(void, SBInstructionList, AppendInstruction,
(lldb::SBInstruction));
   LLDB_REGISTER_METHOD(void, SBInstructionList, Print, (FILE *));
+  LLDB_REGISTER_METHOD(void, SBInstructionList, Print, (SBFile));
+  LLDB_REGISTER_METHOD(void, SBInstructionList, Print, (FileSP));
   LLDB_REGISTER_METHOD(bool, SBInstructionList, GetDescription,
(lldb::SBStream &));
   LLDB_REGISTER_METHOD(bool, SBInstructionList,
Index: lldb/source/API/SBInstruction.cpp
===
--- lldb/source/API/SBInstruction.cpp
+++ lldb/source/API/SBInstruction.cpp
@@ -11,6 +11,7 @@
 
 #include "lldb/API/SBAddress.h"
 #include "lldb/API/SBFrame.h"
+#include "lldb/API/SBFile.h"
 
 #include "lldb/API/SBInstruction.h"
 #include "lldb/API/SBStream.h"
@@ -255,10 +256,21 @@
   return false;
 }
 
-void SBInstruction::Print(FILE *out) {
-  LLDB_RECORD_METHOD(void, SBInstruction, Print, (FILE *), out);
+void SBInstruction::Print(FILE *outp) {
+  LLDB_RECORD_METHOD(void, SBInstruction, Print, (FILE *), outp);
+  FileSP out = std::make_shared(outp, /*take_ownership=*/false);
+  Print(out);
+}
+
+void SBInstruction::Print(SBFile out) {
+  LLDB_RECORD_METHOD(void, SBInstruction, Print, (SBFile), out);
+  Print(out.GetFile());
+}
+
+void SBInstruction::Print(FileSP out_sp) {
+  LLDB_RECORD_METHOD(void, SBInstruction, Print, (FileSP), out_sp);
 
-  if (out == nullptr)
+  if (!out_sp || !out_sp->IsValid())
 return;
 
   lldb::InstructionSP inst_sp(GetOpaque());
@@ -269,7 +281,7 @@
 if (module_sp)
   module_sp->ResolveSymbolContextForAddress(addr, eSymbolContextEverything,
 sc);
-StreamFile out_stream(out, false);
+StreamFile out_stream(out_sp);
 FormatEntity::Entry format;
 FormatEntity::Parse("${addr}: ", format);
 inst_sp->Dump(&out_stream, 0, true, false, nullptr, &sc, nullptr, &format,
@@ -358,6 +370,8 @@
   LLDB_REGISTER_METHOD(bool, SBInstruction, GetDescription,
(lldb::SBStre

[Lldb-commits] [lldb] r374825 - build fix for SBInstruction.

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Mon Oct 14 14:51:02 2019
New Revision: 374825

URL: http://llvm.org/viewvc/llvm-project?rev=374825&view=rev
Log:
build fix for SBInstruction.

oops!  I cherry-picked  rL374820 thinking it was completely
independent of D68737, but it wasn't.  It makes an incidental
use of SBFile::GetFile, which is introduced there, so I broke the
build.

The docs say you can commit without review for "obvious".   I think
this qualifies.   If this kind of fix isn't considered obvious, let
me know and I'll revert instead.

Fixes: rL374820

Modified:
lldb/trunk/include/lldb/API/SBFile.h
lldb/trunk/source/API/SBInstruction.cpp
lldb/trunk/source/API/SBInstructionList.cpp

Modified: lldb/trunk/include/lldb/API/SBFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBFile.h?rev=374825&r1=374824&r2=374825&view=diff
==
--- lldb/trunk/include/lldb/API/SBFile.h (original)
+++ lldb/trunk/include/lldb/API/SBFile.h Mon Oct 14 14:51:02 2019
@@ -14,6 +14,8 @@
 namespace lldb {
 
 class LLDB_API SBFile {
+  friend class SBInstruction;
+  friend class SBInstructionList;
   friend class SBDebugger;
   friend class SBCommandReturnObject;
   friend class SBProcess;

Modified: lldb/trunk/source/API/SBInstruction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBInstruction.cpp?rev=374825&r1=374824&r2=374825&view=diff
==
--- lldb/trunk/source/API/SBInstruction.cpp (original)
+++ lldb/trunk/source/API/SBInstruction.cpp Mon Oct 14 14:51:02 2019
@@ -264,7 +264,7 @@ void SBInstruction::Print(FILE *outp) {
 
 void SBInstruction::Print(SBFile out) {
   LLDB_RECORD_METHOD(void, SBInstruction, Print, (SBFile), out);
-  Print(out.GetFile());
+  Print(out.m_opaque_sp);
 }
 
 void SBInstruction::Print(FileSP out_sp) {

Modified: lldb/trunk/source/API/SBInstructionList.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBInstructionList.cpp?rev=374825&r1=374824&r2=374825&view=diff
==
--- lldb/trunk/source/API/SBInstructionList.cpp (original)
+++ lldb/trunk/source/API/SBInstructionList.cpp Mon Oct 14 14:51:02 2019
@@ -130,7 +130,7 @@ void SBInstructionList::Print(SBFile out
   LLDB_RECORD_METHOD(void, SBInstructionList, Print, (SBFile), out);
   if (!out.IsValid())
 return;
-  StreamFile stream(out.GetFile());
+  StreamFile stream(out.m_opaque_sp);
   GetDescription(stream);
 }
 


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

[Lldb-commits] [PATCH] D68918: eliminate virtual methods from PythonDataObjects

[Lawrence D'Anna via Phabricator via lldb-commits]

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



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:188-192
   // PythonObject is implicitly convertible to PyObject *, which will call the
   // wrong overload.  We want to explicitly disallow this, since a PyObject
   // *always* owns its reference.  Therefore the overload which takes a
   // PyRefType doesn't make sense, and the copy constructor should be used.
   void Reset(PyRefType type, const PythonObject &ref) = delete;

labath wrote:
> BTW, is this needed? My impression was that PythonObject was/is not 
> convertible to a `PyObject*`..
Yea it's not actually convertible, i'll remove this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68918



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

[Lldb-commits] [PATCH] D68737: SBFile::GetFile: convert SBFile back into python native files.

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 224922.
lawrence_danna added a comment.

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68737

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Host/File.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -638,7 +638,7 @@
   void Reset(PyRefType type, PyObject *py_obj) override;
 
   ArgInfo GetNumArguments() const;
-  
+
   // If the callable is a Py_Class, then find the number of arguments
   // of the __init__ method.
   ArgInfo GetNumInitArguments() const;
@@ -658,7 +658,6 @@
 class PythonFile : public PythonObject {
 public:
   PythonFile();
-  PythonFile(File &file, const char *mode);
   PythonFile(PyRefType type, PyObject *o);
 
   ~PythonFile() override;
@@ -668,7 +667,21 @@
   using PythonObject::Reset;
 
   void Reset(PyRefType type, PyObject *py_obj) override;
-  void Reset(File &file, const char *mode);
+
+  static llvm::Expected FromFile(File &file,
+ const char *mode = nullptr);
+
+  // FIXME delete this after FILE* typemaps are deleted
+  // and ScriptInterpreterPython is fixed
+  PythonFile(File &file, const char *mode = nullptr) {
+auto f = FromFile(file, mode);
+if (f)
+  *this = std::move(f.get());
+else {
+  Reset();
+  llvm::consumeError(f.takeError());
+}
+  }
 
   lldb::FileUP GetUnderlyingFile() const;
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -22,6 +22,7 @@
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Errno.h"
 
@@ -1012,8 +1013,6 @@
 
 PythonFile::PythonFile() : PythonObject() {}
 
-PythonFile::PythonFile(File &file, const char *mode) { Reset(file, mode); }
-
 PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); }
 
 PythonFile::~PythonFile() {}
@@ -1063,25 +1062,6 @@
   PythonObject::Reset(PyRefType::Borrowed, result.get());
 }
 
-void PythonFile::Reset(File &file, const char *mode) {
-  if (!file.IsValid()) {
-Reset();
-return;
-  }
-
-  char *cmode = const_cast(mode);
-#if PY_MAJOR_VERSION >= 3
-  Reset(PyRefType::Owned, PyFile_FromFd(file.GetDescriptor(), nullptr, cmode,
--1, nullptr, "ignore", nullptr, 0));
-#else
-  // Read through the Python source, doesn't seem to modify these strings
-  Reset(PyRefType::Owned,
-PyFile_FromFile(file.GetStream(), const_cast(""), cmode,
-nullptr));
-#endif
-}
-
-
 FileUP PythonFile::GetUnderlyingFile() const {
   if (!IsValid())
 return nullptr;
@@ -1238,6 +1218,13 @@
 return base_error;
   };
 
+  PyObject *GetPythonObject() const {
+assert(m_py_obj.IsValid());
+return m_py_obj.get();
+  }
+
+  static bool classof(const File *file) = delete;
+
 protected:
   PythonFile m_py_obj;
   bool m_borrowed;
@@ -1252,7 +1239,14 @@
   SimplePythonFile(const PythonFile &file, bool borrowed, int fd,
File::OpenOptions options)
   : OwnedPythonFile(file, borrowed, fd, options, false) {}
+
+  static char ID;
+  bool isA(const void *classID) const override {
+return classID == &ID || NativeFile::isA(classID);
+  }
+  static bool classof(const File *file) { return file->isA(&ID); }
 };
+char SimplePythonFile::ID = 0;
 } // namespace
 
 #if PY_MAJOR_VERSION >= 3
@@ -1321,7 +1315,18 @@
 return Status();
   }
 
+  Expected GetOptions() const override {
+GIL takeGIL;
+return GetOptionsForPyObject(m_py_obj);
+  }
+
+  static char ID;
+  bool isA(const void *classID) const override {
+return classID == &ID || File::isA(classID);
+  }
+  static bool classof(const File *file) { return file->isA(&ID); }
 };
+char PythonIOFile::ID = 0;
 } // namespace
 
 namespace {
@@ -1542,4 +1547,42 @@
 #endif
 }
 
+Expected PythonFile::FromFile(File &file, const char *mode) {
+  if (!file.IsValid())
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid file");
+
+  if (auto *simple = llvm::dyn_cast(&file))
+  

[Lldb-commits] [PATCH] D68737: SBFile::GetFile: convert SBFile back into python native files.

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

@labath, how's this look now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68737



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

[Lldb-commits] [PATCH] D68918: eliminate virtual methods from PythonDataObjects

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 224924.
lawrence_danna added a comment.

rebased, and added in explicit default constructors for MSVC's sake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68918

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -25,6 +25,25 @@
 // Expected<> is considered deprecated and should not be
 // used in new code.  If you need to use it, fix it first.
 //
+//
+// TODOs for this file
+//
+// * Make all methods safe for exceptions.
+//
+// * Eliminate method signatures that must translate exceptions into
+//   empty objects or NULLs.   Almost everything here should return
+//   Expected<>.   It should be acceptable for certain operations that
+//   can never fail to assert instead, such as the creation of
+//   PythonString from a string literal.
+//
+// * Elimintate Reset(), and make all non-default constructors private.
+//   Python objects should be created with Retain<> or Take<>, and they
+//   should be assigned with operator=
+//
+// * Eliminate default constructors, make python objects always
+//   nonnull, and use optionals where necessary.
+//
+
 
 #ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H
 #define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H
@@ -170,20 +189,15 @@
 rhs.m_py_obj = nullptr;
   }
 
-  virtual ~PythonObject() { Reset(); }
+  ~PythonObject() { Reset(); }
 
   void Reset() {
-// Avoid calling the virtual method since it's not necessary
-// to actually validate the type of the PyObject if we're
-// just setting to null.
 if (m_py_obj && Py_IsInitialized())
   Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
   void Reset(const PythonObject &rhs) {
-// Avoid calling the virtual method if it's not necessary
-// to actually validate the type of the PyObject.
 if (!rhs.IsValid())
   Reset();
 else
@@ -196,9 +210,7 @@
   // PyRefType doesn't make sense, and the copy constructor should be used.
   void Reset(PyRefType type, const PythonObject &ref) = delete;
 
-  // FIXME We shouldn't have virtual anything.  PythonObject should be a
-  // strictly pass-by-value type.
-  virtual void Reset(PyRefType type, PyObject *py_obj) {
+  void Reset(PyRefType type, PyObject *py_obj) {
 if (py_obj == m_py_obj)
   return;
 
@@ -376,21 +388,37 @@
 
 } // namespace python
 
-class PythonBytes : public PythonObject {
+template  class TypedPythonObject : public PythonObject {
 public:
-  PythonBytes();
-  explicit PythonBytes(llvm::ArrayRef bytes);
-  PythonBytes(const uint8_t *bytes, size_t length);
-  PythonBytes(PyRefType type, PyObject *o);
+  // override to perform implicit type conversions on Reset
+  // This can be eliminated once we drop python 2 support.
+  static void Convert(PyRefType &type, PyObject *&py_obj) {}
 
-  ~PythonBytes() override;
+  using PythonObject::Reset;
 
-  static bool Check(PyObject *py_obj);
+  void Reset(PyRefType type, PyObject *py_obj) {
+Reset();
+if (!py_obj)
+  return;
+T::Convert(type, py_obj);
+if (T::Check(py_obj))
+  PythonObject::Reset(type, py_obj);
+else if (type == PyRefType::Owned)
+  Py_DECREF(py_obj);
+  }
 
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
+  TypedPythonObject(PyRefType type, PyObject *py_obj) { Reset(type, py_obj); }
+
+  TypedPythonObject() {}
+};
+
+class PythonBytes : public TypedPythonObject {
+public:
+  using TypedPythonObject::TypedPythonObject;
+  explicit PythonBytes(llvm::ArrayRef bytes);
+  PythonBytes(const uint8_t *bytes, size_t length);
 
-  void Reset(PyRefType type, PyObject *py_obj) override;
+  static bool Check(PyObject *py_obj);
 
   llvm::ArrayRef GetBytes() const;
 
@@ -401,23 +429,15 @@
   StructuredData::StringSP CreateStructuredString() const;
 };
 
-class PythonByteArray : public PythonObject {
+class PythonByteArray : public TypedPythonObject {
 public:
-  PythonByteArray();
+  using TypedPythonObject::TypedPythonObject;
   explicit PythonByteArray(llvm::ArrayRef bytes);
   PythonByteArray(const uint8_t *bytes, size_t length);
-  PythonByteArray(PyRefType type, PyObject *o);
   PythonByteArray(const PythonBytes &object);
 
-  ~PythonByteArray() override;
-
   static bool Check(PyObject *py_obj);
 
-  // Bring in the no-argument base class version
-  using PythonObject::Reset;
-
-  void Reset(PyRefType type, PyObject *py_obj) override;
-
   llvm::ArrayRef GetBytes() const;
 
   size_t GetSize() const;
@@ -427,22 +447,17 @@
   StructuredData::StringSP CreateStructuredString() c

[Lldb-commits] [PATCH] D68856: convert SBDebugger::***FileHandle() wrappers to native files.

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 224923.
lawrence_danna added a comment.

rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68856

Files:
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/interface/SBDebugger.i

Index: lldb/scripts/interface/SBDebugger.i
===
--- lldb/scripts/interface/SBDebugger.i
+++ lldb/scripts/interface/SBDebugger.i
@@ -165,29 +165,44 @@
 void
 SkipLLDBInitFiles (bool b);
 
-%feature("autodoc", "DEPRECATED, use SetInputFile");
-void
-SetInputFileHandle (FILE *f, bool transfer_ownership);
+%pythoncode %{
+def SetOutputFileHandle(self, file, transfer_ownership):
+"DEPRECATED, use SetOutputFile"
+if file is None:
+import sys
+file = sys.stdout
+self.SetOutputFile(SBFile.Create(file, borrow=True))
+
+def SetInputFileHandle(self, file, transfer_ownership):
+"DEPRECATED, use SetInputFile"
+if file is None:
+import sys
+file = sys.stdin
+self.SetInputFile(SBFile.Create(file, borrow=True))
+
+def SetErrorFileHandle(self, file, transfer_ownership):
+"DEPRECATED, use SetErrorFile"
+if file is None:
+import sys
+file = sys.stderr
+self.SetErrorFile(SBFile.Create(file, borrow=True))
+%}
 
-%feature("autodoc", "DEPRECATED, use SetOutputFile");
-void
-SetOutputFileHandle (FILE *f, bool transfer_ownership);
 
-%feature("autodoc", "DEPRECATED, use SetErrorFile");
-void
-SetErrorFileHandle (FILE *f, bool transfer_ownership);
+%extend {
 
-%feature("autodoc", "DEPRECATED, use GetInputFile");
-FILE *
-GetInputFileHandle ();
+lldb::FileSP GetInputFileHandle() {
+return self->GetInputFile().GetFile();
+}
 
-%feature("autodoc", "DEPRECATED, use GetOutputFile");
-FILE *
-GetOutputFileHandle ();
+lldb::FileSP GetOutputFileHandle() {
+return self->GetOutputFile().GetFile();
+}
 
-%feature("autodoc", "DEPRECATED, use GetErrorFile");
-FILE *
-GetErrorFileHandle ();
+lldb::FileSP GetErrorFileHandle() {
+return self->GetErrorFile().GetFile();
+}
+}
 
 SBError
 SetInputFile (SBFile file);
Index: lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -129,8 +129,6 @@
 
 
 @add_test_categories(['pyapi'])
-@skipIfWindows # FIXME pre-existing bug, should be fixed
-   # when we delete the FILE* typemaps.
 def test_legacy_file_out_script(self):
 with open(self.out_filename, 'w') as f:
 self.debugger.SetOutputFileHandle(f, False)
@@ -155,8 +153,6 @@
 self.assertIn('deadbeef', f.read())
 
 @add_test_categories(['pyapi'])
-@skipIfWindows # FIXME pre-existing bug, should be fixed
-   # when we delete the FILE* typemaps.
 def test_legacy_file_err_with_get(self):
 with open(self.out_filename, 'w') as f:
 self.debugger.SetErrorFileHandle(f, False)
@@ -194,11 +190,11 @@
 @add_test_categories(['pyapi'])
 def test_sbfile_type_errors(self):
 sbf = lldb.SBFile()
-self.assertRaises(TypeError, sbf.Write, None)
-self.assertRaises(TypeError, sbf.Read, None)
-self.assertRaises(TypeError, sbf.Read, b'this bytes is not mutable')
-self.assertRaises(TypeError, sbf.Write, u"ham sandwich")
-self.assertRaises(TypeError, sbf.Read, u"ham sandwich")
+self.assertRaises(Exception, sbf.Write, None)
+self.assertRaises(Exception, sbf.Read, None)
+self.assertRaises(Exception, sbf.Read, b'this bytes is not mutable')
+self.assertRaises(Exception, sbf.Write, u"ham sandwich")
+self.assertRaises(Exception, sbf.Read, u"ham sandwich")
 
 
 @add_test_categories(['pyapi'])
@@ -856,3 +852,40 @@
 with open(self.out_filename, 'r') as f:
 self.assertEqual(list(range(10)), list(map(int, f.read().strip().split(
 
+
+@add_test_categories(['pyapi'])
+def test_set_filehandle_none(self):
+self.assertRaises(Exception, self.debugger.SetOutputFile, None)
+self.assertRaises(Exception, self.debugger.SetOutputFile, "ham sandwich")
+self.assertRaises(Exception, self.debugger.SetOutputFileHandle, "ham sandwich")
+self.assertRaises(Exception, self.debugger.SetInputFile, None)
+self.assertRaises(Exception, self.debugger.SetInputFile, "ham sa

[Lldb-commits] [PATCH] D68289: [lldb-server/android] Show more processes by relaxing some checks

[walter erquinigo via Phabricator via lldb-commits]

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

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68289

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
@@ -144,68 +144,79 @@
   }
 }
 
-static bool GetProcessAndStatInfo(::pid_t pid,
-  ProcessInstanceInfo &process_info,
-  ProcessState &State, ::pid_t &tracerpid) {
-  tracerpid = 0;
-  process_info.Clear();
+static void GetProcessArgs(::pid_t pid, ProcessInstanceInfo &process_info) {
+  auto BufferOrError = getProcFile(pid, "cmdline");
+  if (!BufferOrError)
+return;
+  std::unique_ptr Cmdline = std::move(*BufferOrError);
+
+  llvm::StringRef Arg0, Rest;
+  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
+  process_info.SetArg0(Arg0);
+  while (!Rest.empty()) {
+llvm::StringRef Arg;
+std::tie(Arg, Rest) = Rest.split('\0');
+process_info.GetArguments().AppendArgument(Arg);
+  }
+}
 
+static void GetExePathAndArch(::pid_t pid, ProcessInstanceInfo &process_info) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+  std::string ExePath(PATH_MAX, '\0');
 
   // We can't use getProcFile here because proc/[pid]/exe is a symbolic link.
   llvm::SmallString<64> ProcExe;
   (llvm::Twine("/proc/") + llvm::Twine(pid) + "/exe").toVector(ProcExe);
-  std::string ExePath(PATH_MAX, '\0');
 
   ssize_t len = readlink(ProcExe.c_str(), &ExePath[0], PATH_MAX);
-  if (len <= 0) {
+  if (len > 0) {
+ExePath.resize(len);
+  } else {
 LLDB_LOG(log, "failed to read link exe link for {0}: {1}", pid,
  Status(errno, eErrorTypePOSIX));
-return false;
+ExePath.resize(0);
   }
-  ExePath.resize(len);
-
   // If the binary has been deleted, the link name has " (deleted)" appended.
   // Remove if there.
   llvm::StringRef PathRef = ExePath;
   PathRef.consume_back(" (deleted)");
 
-  process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  if (!PathRef.empty()) {
+process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
+process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  }
+}
 
+static void GetProcessEnviron(::pid_t pid, ProcessInstanceInfo &process_info) {
   // Get the process environment.
   auto BufferOrError = getProcFile(pid, "environ");
   if (!BufferOrError)
-return false;
-  std::unique_ptr Environ = std::move(*BufferOrError);
-
-  // Get the command line used to start the process.
-  BufferOrError = getProcFile(pid, "cmdline");
-  if (!BufferOrError)
-return false;
-  std::unique_ptr Cmdline = std::move(*BufferOrError);
-
-  // Get User and Group IDs and get tracer pid.
-  if (!GetStatusInfo(pid, process_info, State, tracerpid))
-return false;
-
-  process_info.SetProcessID(pid);
-  process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
+return;
 
+  std::unique_ptr Environ = std::move(*BufferOrError);
   llvm::StringRef Rest = Environ->getBuffer();
   while (!Rest.empty()) {
 llvm::StringRef Var;
 std::tie(Var, Rest) = Rest.split('\0');
 process_info.GetEnvironment().insert(Var);
   }
+}
 
-  llvm::StringRef Arg0;
-  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
-  process_info.SetArg0(Arg0);
-  while (!Rest.empty()) {
-llvm::StringRef Arg;
-std::tie(Arg, Rest) = Rest.split('\0');
-process_info.GetArguments().AppendArgument(Arg);
-  }
+static bool GetProcessAndStatInfo(::pid_t pid,
+  ProcessInstanceInfo &process_info,
+  ProcessState &State, ::pid_t &tracerpid) {
+  tracerpid = 0;
+  process_info.Clear();
+
+  process_info.SetProcessID(pid);
+
+  GetExePathAndArch(pid, process_info);
+  GetProcessArgs(pid, process_info);
+  GetProcessEnviron(pid, process_info);
+
+  // Get User and Group IDs and get tracer pid.
+  if (!GetStatusInfo(pid, process_info, State, tracerpid))
+return false;
 
   return true;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68960: remove FILE* usage from SBStream.i

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, jasonmolenda, labath.
Herald added a project: LLDB.

This patch removes FILE* and replaces it with SBFile and FileSP the 
SWIG interface for `SBStream.i`.   And this is the last one.   With 
this change, nothing in the python API will can access a FILE* method 
on the C++ side.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68960

Files:
  lldb/include/lldb/API/SBStream.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/interface/SBStream.i
  lldb/source/API/SBStream.cpp

Index: lldb/source/API/SBStream.cpp
===
--- lldb/source/API/SBStream.cpp
+++ lldb/source/API/SBStream.cpp
@@ -9,6 +9,7 @@
 #include "lldb/API/SBStream.h"
 
 #include "SBReproducerPrivate.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Utility/Status.h"
@@ -108,8 +109,19 @@
 void SBStream::RedirectToFileHandle(FILE *fh, bool transfer_fh_ownership) {
   LLDB_RECORD_METHOD(void, SBStream, RedirectToFileHandle, (FILE *, bool), fh,
  transfer_fh_ownership);
+  FileSP file = std::make_unique(fh, transfer_fh_ownership);
+  return RedirectToFile(file);
+}
+
+void SBStream::RedirectToFile(SBFile file) {
+  LLDB_RECORD_METHOD(void, SBStream, RedirectToFile, (SBFile), file)
+  RedirectToFile(file.GetFile());
+}
+
+void SBStream::RedirectToFile(FileSP file_sp) {
+  LLDB_RECORD_METHOD(void, SBStream, RedirectToFile, (FileSP), file_sp);
 
-  if (fh == nullptr)
+  if (!file_sp || !file_sp->IsValid())
 return;
 
   std::string local_data;
@@ -120,7 +132,7 @@
   local_data = static_cast(m_opaque_up.get())->GetString();
   }
 
-  m_opaque_up = std::make_unique(fh, transfer_fh_ownership);
+  m_opaque_up = std::make_unique(file_sp);
   m_is_file = true;
 
   // If we had any data locally in our StreamString, then pass that along to
@@ -184,6 +196,8 @@
   LLDB_REGISTER_METHOD(const char *, SBStream, GetData, ());
   LLDB_REGISTER_METHOD(size_t, SBStream, GetSize, ());
   LLDB_REGISTER_METHOD(void, SBStream, RedirectToFile, (const char *, bool));
+  LLDB_REGISTER_METHOD(void, SBStream, RedirectToFile, (FileSP));
+  LLDB_REGISTER_METHOD(void, SBStream, RedirectToFile, (SBFile));
   LLDB_REGISTER_METHOD(void, SBStream, RedirectToFileHandle, (FILE *, bool));
   LLDB_REGISTER_METHOD(void, SBStream, RedirectToFileDescriptor, (int, bool));
   LLDB_REGISTER_METHOD(void, SBStream, Clear, ());
Index: lldb/scripts/interface/SBStream.i
===
--- lldb/scripts/interface/SBStream.i
+++ lldb/scripts/interface/SBStream.i
@@ -75,7 +75,18 @@
 RedirectToFile (const char *path, bool append);
 
 void
-RedirectToFileHandle (FILE *fh, bool transfer_fh_ownership);
+RedirectToFile (lldb::SBFile file);
+
+void
+RedirectToFile (lldb::FileSP file);
+
+%extend {
+%feature("autodoc", "DEPRECATED, use RedirectToFile");
+void
+RedirectToFileHandle (lldb::FileSP file, bool transfer_fh_ownership) {
+self->RedirectToFile(file);
+}
+}
 
 void
 RedirectToFileDescriptor (int fd, bool transfer_fh_ownership);
Index: lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -889,3 +889,30 @@
 sbf = self.debugger.GetInputFile()
 if sys.version_info.major >= 3:
 self.assertEqual(sbf.GetFile().fileno(), 0)
+
+
+@add_test_categories(['pyapi'])
+def test_sbstream(self):
+
+with open(self.out_filename, 'w') as f:
+stream = lldb.SBStream()
+stream.RedirectToFile(f)
+stream.Print("zork")
+with open(self.out_filename, 'r') as f:
+self.assertEqual(f.read().strip(), "zork")
+
+with open(self.out_filename, 'w') as f:
+stream = lldb.SBStream()
+stream.RedirectToFileHandle(f, True)
+stream.Print("Yendor")
+with open(self.out_filename, 'r') as f:
+self.assertEqual(f.read().strip(), "Yendor")
+
+stream = lldb.SBStream()
+f = open(self.out_filename,  'w')
+stream.RedirectToFile(lldb.SBFile.Create(f, borrow=False))
+stream.Print("Frobozz")
+stream = None
+self.assertTrue(f.closed)
+with open(self.out_filename, 'r') as f:
+self.assertEqual(f.read().strip(), "Frobozz")
Index: lldb/include/lldb/API/SBStream.h
===
--- lldb/include/lldb/API/SBStream.h
+++ lldb/include/lldb/API/SBStream.h
@@ -39,6 +39,10 @@
 
   void Redirect

[Lldb-commits] [lldb] r374846 - fix

[Walter Erquinigo via lldb-commits]

Author: wallace
Date: Mon Oct 14 16:32:46 2019
New Revision: 374846

URL: http://llvm.org/viewvc/llvm-project?rev=374846&view=rev
Log:
fix

Modified:
lldb/trunk/source/Host/linux/Host.cpp

Modified: lldb/trunk/source/Host/linux/Host.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/Host.cpp?rev=374846&r1=374845&r2=374846&view=diff
==
--- lldb/trunk/source/Host/linux/Host.cpp (original)
+++ lldb/trunk/source/Host/linux/Host.cpp Mon Oct 14 16:32:46 2019
@@ -144,68 +144,79 @@ static ArchSpec GetELFProcessCPUType(llv
   }
 }
 
-static bool GetProcessAndStatInfo(::pid_t pid,
-  ProcessInstanceInfo &process_info,
-  ProcessState &State, ::pid_t &tracerpid) {
-  tracerpid = 0;
-  process_info.Clear();
+static void GetProcessArgs(::pid_t pid, ProcessInstanceInfo &process_info) {
+  auto BufferOrError = getProcFile(pid, "cmdline");
+  if (!BufferOrError)
+return;
+  std::unique_ptr Cmdline = std::move(*BufferOrError);
+
+  llvm::StringRef Arg0, Rest;
+  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
+  process_info.SetArg0(Arg0);
+  while (!Rest.empty()) {
+llvm::StringRef Arg;
+std::tie(Arg, Rest) = Rest.split('\0');
+process_info.GetArguments().AppendArgument(Arg);
+  }
+}
 
+static void GetExePathAndArch(::pid_t pid, ProcessInstanceInfo &process_info) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+  std::string ExePath(PATH_MAX, '\0');
 
   // We can't use getProcFile here because proc/[pid]/exe is a symbolic link.
   llvm::SmallString<64> ProcExe;
   (llvm::Twine("/proc/") + llvm::Twine(pid) + "/exe").toVector(ProcExe);
-  std::string ExePath(PATH_MAX, '\0');
 
   ssize_t len = readlink(ProcExe.c_str(), &ExePath[0], PATH_MAX);
-  if (len <= 0) {
+  if (len > 0) {
+ExePath.resize(len);
+  } else {
 LLDB_LOG(log, "failed to read link exe link for {0}: {1}", pid,
  Status(errno, eErrorTypePOSIX));
-return false;
+ExePath.resize(0);
   }
-  ExePath.resize(len);
-
   // If the binary has been deleted, the link name has " (deleted)" appended.
   // Remove if there.
   llvm::StringRef PathRef = ExePath;
   PathRef.consume_back(" (deleted)");
 
-  process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  if (!PathRef.empty()) {
+process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
+process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  }
+}
 
+static void GetProcessEnviron(::pid_t pid, ProcessInstanceInfo &process_info) {
   // Get the process environment.
   auto BufferOrError = getProcFile(pid, "environ");
   if (!BufferOrError)
-return false;
-  std::unique_ptr Environ = std::move(*BufferOrError);
-
-  // Get the command line used to start the process.
-  BufferOrError = getProcFile(pid, "cmdline");
-  if (!BufferOrError)
-return false;
-  std::unique_ptr Cmdline = std::move(*BufferOrError);
-
-  // Get User and Group IDs and get tracer pid.
-  if (!GetStatusInfo(pid, process_info, State, tracerpid))
-return false;
-
-  process_info.SetProcessID(pid);
-  process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
+return;
 
+  std::unique_ptr Environ = std::move(*BufferOrError);
   llvm::StringRef Rest = Environ->getBuffer();
   while (!Rest.empty()) {
 llvm::StringRef Var;
 std::tie(Var, Rest) = Rest.split('\0');
 process_info.GetEnvironment().insert(Var);
   }
+}
 
-  llvm::StringRef Arg0;
-  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
-  process_info.SetArg0(Arg0);
-  while (!Rest.empty()) {
-llvm::StringRef Arg;
-std::tie(Arg, Rest) = Rest.split('\0');
-process_info.GetArguments().AppendArgument(Arg);
-  }
+static bool GetProcessAndStatInfo(::pid_t pid,
+  ProcessInstanceInfo &process_info,
+  ProcessState &State, ::pid_t &tracerpid) {
+  tracerpid = 0;
+  process_info.Clear();
+
+  process_info.SetProcessID(pid);
+
+  GetExePathAndArch(pid, process_info);
+  GetProcessArgs(pid, process_info);
+  GetProcessEnviron(pid, process_info);
+
+  // Get User and Group IDs and get tracer pid.
+  if (!GetStatusInfo(pid, process_info, State, tracerpid))
+return false;
 
   return true;
 }


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

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: aprantl, teemperor.

We add support for `DW_AT_export_symbols` to detect anonymous struct on top of 
the heuristics implemented in D66175 

This should allow us to differentiate anonymous structs and unnamed structs.

We also fix `TestTypeList.py` which was incorrectly detecting an unnamed struct 
as an anonymous struct.


https://reviews.llvm.org/D68961

Files:
  include/lldb/Symbol/ClangASTContext.h
  packages/Python/lldbsuite/test/python_api/type/TestTypeList.py
  packages/Python/lldbsuite/test/python_api/type/main.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  source/Symbol/ClangASTContext.cpp
  test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp

Index: test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
===
--- /dev/null
+++ test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
@@ -0,0 +1,21 @@
+// Test to verify we are corectly generating anonymous flags when parsing
+// anonymous class and unnamed structs from DWARF to the a clang AST node.
+
+// RUN: %clang++ -g -c -o %t.o %s
+// RUN: lldb-test symbols -dump-clang-ast -name "A::(anonymous struct)" %t.o |
+// FileCheck %s
+
+struct A {
+  struct {
+int x;
+  };
+  struct {
+int y;
+  } C;
+} a;
+
+// CHECK: A::(anonymous struct)
+// CHECK: |-DefinitionData is_anonymous pass_in_registers aggregate
+// standard_layout trivially_copyable pod trivial literal CHECK: A::(anonymous
+// struct) CHECK: |-DefinitionData pass_in_registers aggregate standard_layout
+// trivially_copyable pod trivial literal
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -1346,11 +1346,9 @@
 
 #pragma mark Structure, Unions, Classes
 
-CompilerType ClangASTContext::CreateRecordType(DeclContext *decl_ctx,
-   AccessType access_type,
-   const char *name, int kind,
-   LanguageType language,
-   ClangASTMetadata *metadata) {
+CompilerType ClangASTContext::CreateRecordType(
+DeclContext *decl_ctx, AccessType access_type, const char *name, int kind,
+LanguageType language, ClangASTMetadata *metadata, bool exports_symbols) {
   ASTContext *ast = getASTContext();
   assert(ast != nullptr);
 
@@ -1404,7 +1402,7 @@
 //
 // FIXME: An unnamed class within a class is also wrongly recognized as an
 // anonymous struct.
-if (isa(decl_ctx))
+if (isa(decl_ctx) && exports_symbols)
   decl->setAnonymousStructOrUnion(true);
   }
 
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -181,6 +181,7 @@
   bool is_scoped_enum = false;
   bool is_vector = false;
   bool is_virtual = false;
+  bool exports_symbols = false;
   clang::StorageClass storage = clang::SC_None;
   const char *mangled_name = nullptr;
   lldb_private::ConstString name;
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -347,6 +347,9 @@
 case DW_AT_GNU_vector:
   is_vector = form_value.Boolean();
   break;
+case DW_AT_export_symbols:
+  exports_symbols = form_value.Boolean();
+  break;
 }
   }
 }
@@ -1543,7 +1546,7 @@
   clang_type_was_created = true;
   clang_type = m_ast.CreateRecordType(
   decl_ctx, attrs.accessibility, attrs.name.GetCString(), tag_decl_kind,
-  attrs.class_language, &metadata);
+  attrs.class_language, &metadata, attrs.exports_symbols);
 }
   }
 
Index: packages/Python/lldbsuite/test/python_api/type/main.cpp
===
--- packages/Python/lldbsuite/test/python_api/type/main.cpp
+++ packages/Python/lldbsuite/test/python_api/type/main.cpp
@@ -15,6 +15,14 @@
 TASK_TYPE_1,
 TASK_TYPE_2
 } type;
+// This struct is anonymous b/c it does not have a name
+// and it is not unnamed class.
+// Anonymous classes are a GNU extension.
+struct {
+  int y;
+};
+// This struct is an unnamed class see [class.pre]p1
+// http://eel.is/c++draft/class#pre-1.sentence-6
 struct {
   int x;
 } my_type_is_nameless;
Index: packages/Python/lldbsuite/test/python_api/type/TestTypeList.py
===

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Make the RedirectingFileSystem hold on to its own working directory.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

ping @sammccall


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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 224929.
jingham added a comment.

Addressed Pavel's comments.

I explicitly want the number of fixed arguments, so I changed the function name 
to GetNumFixedArguments.  The docs say explicitly that you have to make the 
function take either three or four fixed arguments, and that's what I have to 
check against.  I don't want to know about varargs and optional named arguments.

I added the Expected, though I didn't plumb the change into 
PythonCallable::GetNumArguments.  That's of marginal extra benefit, and 
orthogonal to the purposes of this patch.

Note also, the fact that I was now checking argument signatures when adding the 
command means you can no longer do:

  (lldb) break command add -F myfunc.function
  (lldb) command script import myfunc.py

which TestBreakpointCommand.py was doing.  You have to make the Python function 
available BEFORE adding it to the breakpoint.  I don't think this was an 
feature, and I don't see any harm in adding that requirement.  But it is a 
change in behavior so I thought I'd bring it up.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68671

Files:
  lldb/include/lldb/API/SBBreakpoint.h
  lldb/include/lldb/API/SBBreakpointLocation.h
  lldb/include/lldb/API/SBBreakpointName.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommandsFromPython.py
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/bktptcmd.py
  lldb/scripts/Python/python-wrapper.swig
  lldb/scripts/interface/SBBreakpoint.i
  lldb/scripts/interface/SBBreakpointLocation.i
  lldb/scripts/interface/SBBreakpointName.i
  lldb/source/API/SBBreakpoint.cpp
  lldb/source/API/SBBreakpointLocation.cpp
  lldb/source/API/SBBreakpointName.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Interpreter/OptionGroupPythonClassWithDict.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -62,7 +62,8 @@
 extern "C" bool LLDBSwigPythonBreakpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
 const lldb::StackFrameSP &sb_frame,
-const lldb::BreakpointLocationSP &sb_bp_loc) {
+const lldb::BreakpointLocationSP &sb_bp_loc,
+StructuredDataImpl *args_impl) {
   return false;
 }
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -179,8 +179,10 @@
   Status GenerateFunction(const char *signature,
   const StringList &input) override;
 
-  Status GenerateBreakpointCommandCallbackData(StringList &input,
-   std::string &output) override;
+  Status GenerateBreakpointCommandCallbackData(
+  StringList &input,
+  std::string &output,
+  bool has_extra_args) override;
 
   bool GenerateWatchpointCommandCallbackData(StringList &input,
  std::string &output) override;
@@ -244,14 +246,22 @@
   Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
   const char *callback_body) override;
 
-  void SetBreakpointCommandCallbackFunction(BreakpointOptions *bp_options,
-const char *function_name) override;
+  Status SetBreakpointCommandCallbackFunction(
+  BreakpointOptions *bp_options,
+  const char *function_name,
+  StructuredData::ObjectSP extra_args_sp) override;
 
   /// This one is for deserialization:
   Status SetBreakpointCommandCallback(
   BreakpointOptions *bp_options,
   std::unique_ptr &data_up) override;
 
+  Status SetBreakpointCommandCallback(
+  BreakpointOptions *bp_options, 
+   const char *command_body_text,
+ 

[Lldb-commits] [PATCH] D68962: update ScriptInterpreterPython to use File, not FILE*

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, jasonmolenda, labath.
Herald added a project: LLDB.

ScriptInterpreterPython needs to save and restore sys.stdout and 
friends when LLDB runs a python script.

It currently does this using FILE*, which is not optimal.  If 
whatever was in sys.stdout can not be represented as a FILE*, then 
it will not be restored correctly when the script is finished.

It also means that if the debugger's own output stream is not 
representable as a file, ScriptInterpreterPython will not be able 
to redirect python's  output correctly.

This patch updates ScriptInterpreterPython to represent files with 
lldb_private::File, and to represent whatever the user had in 
sys.stdout as simply a PythonObject.

This will make lldb interoperate better with other scripts or programs
that need to manipulate sys.stdout.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68962

Files:
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/source/Core/Debugger.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -294,17 +294,19 @@
   TearDownSession = 0x0004
 };
 
-Locker(ScriptInterpreterPythonImpl *py_interpreter = nullptr,
+Locker(ScriptInterpreterPythonImpl *py_interpreter,
uint16_t on_entry = AcquireLock | InitSession,
-   uint16_t on_leave = FreeLock | TearDownSession, FILE *in = nullptr,
-   FILE *out = nullptr, FILE *err = nullptr);
+   uint16_t on_leave = FreeLock | TearDownSession,
+   lldb::FileSP in = nullptr, lldb::FileSP out = nullptr,
+   lldb::FileSP err = nullptr);
 
 ~Locker() override;
 
   private:
 bool DoAcquireLock();
 
-bool DoInitSession(uint16_t on_entry_flags, FILE *in, FILE *out, FILE *err);
+bool DoInitSession(uint16_t on_entry_flags, lldb::FileSP in,
+   lldb::FileSP out, lldb::FileSP err);
 
 bool DoFreeLock();
 
@@ -312,7 +314,6 @@
 
 bool m_teardown_session;
 ScriptInterpreterPythonImpl *m_python_interpreter;
-//	FILE*m_tmp_fh;
 PyGILState_STATE m_GILState;
   };
 
@@ -341,7 +342,8 @@
 
   static void AddToSysPath(AddLocation location, std::string path);
 
-  bool EnterSession(uint16_t on_entry_flags, FILE *in, FILE *out, FILE *err);
+  bool EnterSession(uint16_t on_entry_flags, lldb::FileSP in, lldb::FileSP out,
+lldb::FileSP err);
 
   void LeaveSession();
 
@@ -369,12 +371,12 @@
 
   bool GetEmbeddedInterpreterModuleObjects();
 
-  bool SetStdHandle(File &file, const char *py_name, PythonFile &save_file,
-const char *mode);
+  bool SetStdHandle(lldb::FileSP file, const char *py_name,
+PythonObject &save_file, const char *mode);
 
-  PythonFile m_saved_stdin;
-  PythonFile m_saved_stdout;
-  PythonFile m_saved_stderr;
+  PythonObject m_saved_stdin;
+  PythonObject m_saved_stdout;
+  PythonObject m_saved_stderr;
   PythonObject m_main_module;
   PythonDictionary m_session_dict;
   PythonDictionary m_sys_module_dict;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -370,7 +370,7 @@
 
 ScriptInterpreterPythonImpl::Locker::Locker(
 ScriptInterpreterPythonImpl *py_interpreter, uint16_t on_entry,
-uint16_t on_leave, FILE *in, FILE *out, FILE *err)
+uint16_t on_leave, FileSP in, FileSP out, FileSP err)
 : ScriptInterpreterLocker(),
   m_teardown_session((on_leave & TearDownSession) == TearDownSession),
   m_python_interpreter(py_interpreter) {
@@ -400,8 +400,8 @@
 }
 
 bool ScriptInterpreterPythonImpl::Locker::DoInitSession(uint16_t on_entry_flags,
-FILE *in, FILE *out,
-FILE *err) {
+FileSP in, FileSP out,
+FileSP err) {
   if (!m_python_interpreter)
 return false;
   return m_python_interpreter->EnterSession(on_entry_flags, in, out, err);
@@ -636,28 +636,31 @@
   m_session_is_active = false;
 }
 
-bool ScriptInterpreterPythonImpl::SetStdHandle(File &file, const char *py_name,
-   PythonFile &save_file,
+bool ScriptInterpreterPy

[Lldb-commits] [PATCH] D68963: delete SWIG typemaps for FILE*

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, jasonmolenda, labath.
Herald added a project: LLDB.
lawrence_danna added parent revisions: D68962: update ScriptInterpreterPython 
to use File, not FILE*, D68960: remove FILE* usage from SBStream.i.

The SWIG typemaps for FILE* are no longer used, so 
this patch deletes them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68963

Files:
  lldb/include/lldb/Host/File.h
  lldb/scripts/Python/python-typemaps.swig
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -585,8 +585,9 @@
   auto file = FileSystem::Instance().Open(FileSpec(FileSystem::DEV_NULL),
   File::eOpenOptionRead);
   ASSERT_THAT_EXPECTED(file, llvm::Succeeded());
-  PythonFile py_file(*file.get(), "r");
-  EXPECT_TRUE(PythonFile::Check(py_file.get()));
+  auto py_file = PythonFile::FromFile(*file.get(), "r");
+  ASSERT_THAT_EXPECTED(py_file, llvm::Succeeded());
+  EXPECT_TRUE(PythonFile::Check(py_file.get().get()));
 }
 
 TEST_F(PythonDataObjectsTest, TestObjectAttributes) {
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -651,10 +651,13 @@
 
   PythonDictionary &sys_module_dict = GetSysModuleDictionary();
 
+  auto new_file = PythonFile::FromFile(file, mode);
+  if (!new_file)
+return false;
+
   save_file = sys_module_dict.GetItemForKey(PythonString(py_name));
 
-  PythonFile new_file(file, mode);
-  sys_module_dict.SetItemForKey(PythonString(py_name), new_file);
+  sys_module_dict.SetItemForKey(PythonString(py_name), new_file.get());
   return true;
 }
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -637,20 +637,6 @@
   static llvm::Expected FromFile(File &file,
  const char *mode = nullptr);
 
-  // FIXME delete this after FILE* typemaps are deleted
-  // and ScriptInterpreterPython is fixed
-  PythonFile(File &file, const char *mode = nullptr) {
-auto f = FromFile(file, mode);
-if (f)
-  *this = std::move(f.get());
-else {
-  Reset();
-  llvm::consumeError(f.takeError());
-}
-  }
-
-  lldb::FileUP GetUnderlyingFile() const;
-
   llvm::Expected ConvertToFile(bool borrowed = false);
   llvm::Expected
   ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -837,25 +837,6 @@
 #endif
 }
 
-FileUP PythonFile::GetUnderlyingFile() const {
-  if (!IsValid())
-return nullptr;
-
-  // We don't own the file descriptor returned by this function, make sure the
-  // File object knows about that.
-  PythonString py_mode = GetAttributeValue("mode").AsType();
-  auto options = File::GetOptionsFromMode(py_mode.GetString());
-  if (!options) {
-llvm::consumeError(options.takeError());
-return nullptr;
-  }
-  auto file = std::unique_ptr(new NativeFile(
-  PyObject_AsFileDescriptor(m_py_obj), options.get(), false));
-  if (!file->IsValid())
-return nullptr;
-  return file;
-}
-
 namespace {
 class GIL {
 public:
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -117,8 +117,6 @@
   return std::error_code(ENOTSUP, std::system_category());
 }
 
-FILE *File::TakeStreamAndClear() { return nullptr; }
-
 int File::GetDescriptor() const { return kInvalidDescriptor; }
 
 FILE *File::GetStream() { return nullptr; }
@@ -331,18 +329,6 @@
   return error;
 }
 
-FILE *NativeFile::TakeStreamAndClear() {
-  FILE *stream = GetStream();
-  m_stream = NULL;
-  m_descriptor = kInvalidDescriptor;
-  m_options = OpenOptions();
-  m_own_stream = false;
-  m_own_descriptor = false;
-  m_is_inter

[Lldb-commits] [lldb] r374852 - Revert "fix"

[Walter Erquinigo via lldb-commits]

Author: wallace
Date: Mon Oct 14 16:56:54 2019
New Revision: 374852

URL: http://llvm.org/viewvc/llvm-project?rev=374852&view=rev
Log:
Revert "fix"

This reverts commit d8af64c9a0228301f6fd0e1c841e4abe0b6f4801.

Modified:
lldb/trunk/source/Host/linux/Host.cpp

Modified: lldb/trunk/source/Host/linux/Host.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/Host.cpp?rev=374852&r1=374851&r2=374852&view=diff
==
--- lldb/trunk/source/Host/linux/Host.cpp (original)
+++ lldb/trunk/source/Host/linux/Host.cpp Mon Oct 14 16:56:54 2019
@@ -144,79 +144,68 @@ static ArchSpec GetELFProcessCPUType(llv
   }
 }
 
-static void GetProcessArgs(::pid_t pid, ProcessInstanceInfo &process_info) {
-  auto BufferOrError = getProcFile(pid, "cmdline");
-  if (!BufferOrError)
-return;
-  std::unique_ptr Cmdline = std::move(*BufferOrError);
-
-  llvm::StringRef Arg0, Rest;
-  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
-  process_info.SetArg0(Arg0);
-  while (!Rest.empty()) {
-llvm::StringRef Arg;
-std::tie(Arg, Rest) = Rest.split('\0');
-process_info.GetArguments().AppendArgument(Arg);
-  }
-}
+static bool GetProcessAndStatInfo(::pid_t pid,
+  ProcessInstanceInfo &process_info,
+  ProcessState &State, ::pid_t &tracerpid) {
+  tracerpid = 0;
+  process_info.Clear();
 
-static void GetExePathAndArch(::pid_t pid, ProcessInstanceInfo &process_info) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
-  std::string ExePath(PATH_MAX, '\0');
 
   // We can't use getProcFile here because proc/[pid]/exe is a symbolic link.
   llvm::SmallString<64> ProcExe;
   (llvm::Twine("/proc/") + llvm::Twine(pid) + "/exe").toVector(ProcExe);
+  std::string ExePath(PATH_MAX, '\0');
 
   ssize_t len = readlink(ProcExe.c_str(), &ExePath[0], PATH_MAX);
-  if (len > 0) {
-ExePath.resize(len);
-  } else {
+  if (len <= 0) {
 LLDB_LOG(log, "failed to read link exe link for {0}: {1}", pid,
  Status(errno, eErrorTypePOSIX));
-ExePath.resize(0);
+return false;
   }
+  ExePath.resize(len);
+
   // If the binary has been deleted, the link name has " (deleted)" appended.
   // Remove if there.
   llvm::StringRef PathRef = ExePath;
   PathRef.consume_back(" (deleted)");
 
-  if (!PathRef.empty()) {
-process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
-process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
-  }
-}
+  process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
 
-static void GetProcessEnviron(::pid_t pid, ProcessInstanceInfo &process_info) {
   // Get the process environment.
   auto BufferOrError = getProcFile(pid, "environ");
   if (!BufferOrError)
-return;
-
+return false;
   std::unique_ptr Environ = std::move(*BufferOrError);
+
+  // Get the command line used to start the process.
+  BufferOrError = getProcFile(pid, "cmdline");
+  if (!BufferOrError)
+return false;
+  std::unique_ptr Cmdline = std::move(*BufferOrError);
+
+  // Get User and Group IDs and get tracer pid.
+  if (!GetStatusInfo(pid, process_info, State, tracerpid))
+return false;
+
+  process_info.SetProcessID(pid);
+  process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
+
   llvm::StringRef Rest = Environ->getBuffer();
   while (!Rest.empty()) {
 llvm::StringRef Var;
 std::tie(Var, Rest) = Rest.split('\0');
 process_info.GetEnvironment().insert(Var);
   }
-}
-
-static bool GetProcessAndStatInfo(::pid_t pid,
-  ProcessInstanceInfo &process_info,
-  ProcessState &State, ::pid_t &tracerpid) {
-  tracerpid = 0;
-  process_info.Clear();
 
-  process_info.SetProcessID(pid);
-
-  GetExePathAndArch(pid, process_info);
-  GetProcessArgs(pid, process_info);
-  GetProcessEnviron(pid, process_info);
-
-  // Get User and Group IDs and get tracer pid.
-  if (!GetStatusInfo(pid, process_info, State, tracerpid))
-return false;
+  llvm::StringRef Arg0;
+  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
+  process_info.SetArg0(Arg0);
+  while (!Rest.empty()) {
+llvm::StringRef Arg;
+std::tie(Arg, Rest) = Rest.split('\0');
+process_info.GetArguments().AppendArgument(Arg);
+  }
 
   return true;
 }


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

[Lldb-commits] [lldb] r374853 - [lldb-server/android] Show more processes by relaxing some checks

[Walter Erquinigo via lldb-commits]

Author: wallace
Date: Mon Oct 14 17:00:05 2019
New Revision: 374853

URL: http://llvm.org/viewvc/llvm-project?rev=374853&view=rev
Log:
[lldb-server/android] Show more processes by relaxing some checks

By default `platform process list` only shows the processes of the current user 
that lldb-server can parse.
There are several problems:
- apk programs don't have an executable file. They instead use a package name 
as identifier. We should show them instead.
- each apk also runs under a different user. That's how android works
- because of the user permission, some files like /proc//{environ,exe} 
can't be read.

This results in a very small process list.

This is a local run on my machine
```
(lldb) platform process list
2 matching processes were found on "remote-android"
PIDPARENT USER   TRIPLE   NAME
== == ==  
23291  3177  aarch64-unknown-linux-android sh
23301  23291aarch64-unknown-linux-android lldb-server
```
However, I have 700 processes running at this time.

By implementing a few fallbacks for android, I've expanded this list to 202, 
filtering out kernel processes, which would presumably appear in this list if 
the device was rooted.

```
(lldb) platform process list
202 matching processes were found on "remote-android"
PIDPARENT USER   TRIPLE   NAME
== == ==  
...
12647  3208  aarch64-unknown-linux-android sh
12649  12647 aarch64-unknown-linux-android lldb-server
12653  982com.samsung.faceservice
13185  982com.samsung.vvm
15899  982com.samsung.android.spay
16220  982com.sec.spp.push
17126  982
com.sec.spp.push:RemoteDlcProcess
19772  983com.android.chrome
20209  982com.samsung.cmh:CMH
20380  982
com.google.android.inputmethod.latin
20879  982
com.samsung.android.oneconnect:Receiver
21212  983com.tencent.mm
24459  1 aarch64-unknown-linux-android wpa_supplicant
25974  982com.samsung.android.contacts
26293  982com.samsung.android.messaging
28714  982com.samsung.android.dialer
31605  982
com.samsung.android.MtpApplication
32256  982com.bezobidny
```

Something to notice is that the architecture is unkonwn for all apks. And 
that's fine, because run-as would be required to gather this information and 
that would make this entire functionality massively slow.

There are still several improvements to make here, like displaying actual user 
names, which I'll try to do in a following diff.

Note: Regarding overall apk debugging support from lldb. I'm planning on having 
lldb spawn lldb-server by itself with the correct user, so that everything 
works well. The initial lldb-server used for connecting to the remote platform 
can be reused for such purpose. Furthermore, eventually lldb could also launch 
that initial lldb-server on its own.

Differential Revision: D68289

Modified:
lldb/trunk/source/Host/linux/Host.cpp

Modified: lldb/trunk/source/Host/linux/Host.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/Host.cpp?rev=374853&r1=374852&r2=374853&view=diff
==
--- lldb/trunk/source/Host/linux/Host.cpp (original)
+++ lldb/trunk/source/Host/linux/Host.cpp Mon Oct 14 17:00:05 2019
@@ -144,68 +144,79 @@ static ArchSpec GetELFProcessCPUType(llv
   }
 }
 
-static bool GetProcessAndStatInfo(::pid_t pid,
-  ProcessInstanceInfo &process_info,
-  ProcessState &State, ::pid_t &tracerpid) {
-  tracerpid = 0;
-  process_info.Clear();
+static void GetProcessArgs(::pid_t pid, ProcessInstanceInfo &process_info) {
+  auto BufferOrError = getProcFile(pid, "cmdline");
+  if (!BufferOrError)
+return;
+  std::unique_ptr Cmdline = std::move(*BufferOrError);
+
+  llvm::StringRef Arg0, Rest;
+  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
+  process_info.SetArg0(Arg0);
+  while (!Rest.empty()) {
+llvm::StringRef Arg;
+std::tie(Arg, Rest) = Rest.split('\0');
+process_info.GetArguments().AppendArgument(Arg);
+  }
+}
 
+static void GetExePathAndArch(::pid_t pid, ProcessInstanceInfo &process_info) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+  std::string ExePath(PATH_MAX, '\0');
 
   // 

[Lldb-commits] [PATCH] D68289: [lldb-server/android] Show more processes by relaxing some checks

[walter erquinigo via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd04855f820c5: [lldb-server/android] Show more processes by 
relaxing some checks (authored by wallace).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68289

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
@@ -144,68 +144,79 @@
   }
 }
 
-static bool GetProcessAndStatInfo(::pid_t pid,
-  ProcessInstanceInfo &process_info,
-  ProcessState &State, ::pid_t &tracerpid) {
-  tracerpid = 0;
-  process_info.Clear();
+static void GetProcessArgs(::pid_t pid, ProcessInstanceInfo &process_info) {
+  auto BufferOrError = getProcFile(pid, "cmdline");
+  if (!BufferOrError)
+return;
+  std::unique_ptr Cmdline = std::move(*BufferOrError);
+
+  llvm::StringRef Arg0, Rest;
+  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
+  process_info.SetArg0(Arg0);
+  while (!Rest.empty()) {
+llvm::StringRef Arg;
+std::tie(Arg, Rest) = Rest.split('\0');
+process_info.GetArguments().AppendArgument(Arg);
+  }
+}
 
+static void GetExePathAndArch(::pid_t pid, ProcessInstanceInfo &process_info) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+  std::string ExePath(PATH_MAX, '\0');
 
   // We can't use getProcFile here because proc/[pid]/exe is a symbolic link.
   llvm::SmallString<64> ProcExe;
   (llvm::Twine("/proc/") + llvm::Twine(pid) + "/exe").toVector(ProcExe);
-  std::string ExePath(PATH_MAX, '\0');
 
   ssize_t len = readlink(ProcExe.c_str(), &ExePath[0], PATH_MAX);
-  if (len <= 0) {
+  if (len > 0) {
+ExePath.resize(len);
+  } else {
 LLDB_LOG(log, "failed to read link exe link for {0}: {1}", pid,
  Status(errno, eErrorTypePOSIX));
-return false;
+ExePath.resize(0);
   }
-  ExePath.resize(len);
-
   // If the binary has been deleted, the link name has " (deleted)" appended.
   // Remove if there.
   llvm::StringRef PathRef = ExePath;
   PathRef.consume_back(" (deleted)");
 
-  process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  if (!PathRef.empty()) {
+process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
+process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  }
+}
 
+static void GetProcessEnviron(::pid_t pid, ProcessInstanceInfo &process_info) {
   // Get the process environment.
   auto BufferOrError = getProcFile(pid, "environ");
   if (!BufferOrError)
-return false;
-  std::unique_ptr Environ = std::move(*BufferOrError);
-
-  // Get the command line used to start the process.
-  BufferOrError = getProcFile(pid, "cmdline");
-  if (!BufferOrError)
-return false;
-  std::unique_ptr Cmdline = std::move(*BufferOrError);
-
-  // Get User and Group IDs and get tracer pid.
-  if (!GetStatusInfo(pid, process_info, State, tracerpid))
-return false;
-
-  process_info.SetProcessID(pid);
-  process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
+return;
 
+  std::unique_ptr Environ = std::move(*BufferOrError);
   llvm::StringRef Rest = Environ->getBuffer();
   while (!Rest.empty()) {
 llvm::StringRef Var;
 std::tie(Var, Rest) = Rest.split('\0');
 process_info.GetEnvironment().insert(Var);
   }
+}
 
-  llvm::StringRef Arg0;
-  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
-  process_info.SetArg0(Arg0);
-  while (!Rest.empty()) {
-llvm::StringRef Arg;
-std::tie(Arg, Rest) = Rest.split('\0');
-process_info.GetArguments().AppendArgument(Arg);
-  }
+static bool GetProcessAndStatInfo(::pid_t pid,
+  ProcessInstanceInfo &process_info,
+  ProcessState &State, ::pid_t &tracerpid) {
+  tracerpid = 0;
+  process_info.Clear();
+
+  process_info.SetProcessID(pid);
+
+  GetExePathAndArch(pid, process_info);
+  GetProcessArgs(pid, process_info);
+  GetProcessEnviron(pid, process_info);
+
+  // Get User and Group IDs and get tracer pid.
+  if (!GetStatusInfo(pid, process_info, State, tracerpid))
+return false;
 
   return true;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

[Lawrence D'Anna via Phabricator via lldb-commits]

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



Comment at: lldb/include/lldb/Interpreter/ScriptInterpreter.h:469
+  
+  virtual int GetNumArgumentsForCallable(const char *callable_name) { 
+return -1;

labath wrote:
> In light of varargs functions (`*args, **kwargs`), which are fairly popular 
> in python, the concept of "number of arguments of a callable" does not seem 
> that well defined. The current implementation seems to return the number of 
> fixed arguments, which might be fine, but I think this behavior should be 
> documented. Also, it would be good to modernize this function signature -- 
> have it take a StringRef, and return a `Expected` -- ongoing 
> work by @lawrence_danna will make it possible to return errors from the 
> python interpreter, and this will make it possible to display those, instead 
> of just guessing that this is because the callable was not found (it could in 
> fact be because the named thing is not a callable, of because resolving the 
> name produced an exception, ...).
I just took a look at PythonCallable::GetNumArguments() and it's horribly 
broken.  

It doesn't even work for the simplest test case I could think of.

```  
auto builtins = PythonModule::Import("builtins");
ASSERT_THAT_EXPECTED(builtins, llvm::Succeeded());
auto hex = As(builtins.get().GetAttribute("hex"));
ASSERT_THAT_EXPECTED(hex, llvm::Succeeded());  
EXPECT_EQ(hex.get().GetNumArguments().count, 1u);
```

we should really re-write it to use inspect.signature.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68671



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

[Lldb-commits] [PATCH] D68293: [android/process list] support showing process arguments

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 224949.
wallace added a comment.
Herald added a subscriber: jfb.

address comments

Btw, @labath, could you point me to a example of a full end to end test like 
the attach one you mention?
I haven't been able to find it :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68293

Files:
  lldb/docs/lldb-gdb-remote.txt
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -1181,6 +1181,15 @@
   proc_info.GetEffectiveUserID(), proc_info.GetEffectiveGroupID());
   response.PutCString("name:");
   response.PutStringAsRawHex8(proc_info.GetExecutableFile().GetCString());
+
+  response.PutChar(';');
+  response.PutCString("args:");
+  response.PutStringAsRawHex8(proc_info.GetArg0());
+  for (auto &arg : proc_info.GetArguments()) {
+response.PutChar('-');
+response.PutStringAsRawHex8(arg.ref());
+  }
+
   response.PutChar(';');
   const ArchSpec &proc_arch = proc_info.GetArchitecture();
   if (proc_arch.IsValid()) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1927,6 +1927,22 @@
 std::string name;
 extractor.GetHexByteString(name);
 process_info.GetExecutableFile().SetFile(name, FileSpec::Style::native);
+  } else if (name.equals("args")) {
+llvm::StringRef encoded_args(value), hex_arg;
+
+while (!encoded_args.empty()) {
+  std::tie(hex_arg, encoded_args) = encoded_args.split('-');
+  std::string arg;
+  StringExtractor extractor(hex_arg);
+  if (extractor.GetHexByteString(arg) * 2 != hex_arg.size()) {
+// In case of wrong encoding, we discard all the arguments
+process_info.GetArguments().Clear();
+break;
+  }
+  process_info.GetArguments().AppendArgument(arg);
+}
+if (!process_info.GetArguments().empty())
+  process_info.SetArg0(process_info.GetArguments().GetArgumentAtIndex(0));
   } else if (name.equals("cputype")) {
 value.getAsInteger(0, cpu);
   } else if (name.equals("cpusubtype")) {
Index: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
@@ -5,6 +5,8 @@
 from lldbsuite.test.decorators import *
 from gdbclientutils import *
 
+def hexlify(string):
+return binascii.hexlify(string.encode()).decode()
 
 class TestPlatformClient(GDBRemoteTestBase):
 
@@ -12,22 +14,56 @@
 """Test connecting to a remote linux platform"""
 
 class MyResponder(MockGDBServerResponder):
+def __init__(self):
+MockGDBServerResponder.__init__(self)
+self.currentQsProc = 0
+self.all_users = False
+
 def qfProcessInfo(self, packet):
 if "all_users:1" in packet:
-return "pid:10;ppid:1;uid:1;gid:1;euid:1;egid:1;name:" + binascii.hexlify("/a/test_process".encode()).decode() + ";"
+self.all_users = True
+name = hexlify("/a/test_process")
+args = "-".join(map(hexlify,
+["/system/bin/sh", "-c", "/data/local/tmp/lldb-server"]))
+return "pid:10;ppid:1;uid:2;gid:3;euid:4;egid:5;name:" + name + ";args:" + args + ";"
 else:
+self.all_users = False
 return "E04"
 
-self.server.responder = MyResponder()
+def qsProcessInfo(self):
+if self.all_users:
+if self.currentQsProc == 0:
+self.currentQsProc = 1
+name = hexlify("/b/another_test_process")
+# This intentionally has a badly encoded argument
+args = "X".join(map(hexlify,
+["/system/bin/ls", "--help"]))
+return "pid:11;ppid:2;uid:3;gid

[Lldb-commits] [PATCH] D68968: [android/process list] use arg0 as fallback for process name

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: labath, clayborg, aadsm.
Herald added subscribers: lldb-commits, srhines.
Herald added a project: LLDB.

In systems like android, there are cases in which a process name can correspond 
to a package name (e.g. com.test.app), which is not an executable path. 
ProcessInfo has been assuming that the process name is an executable path, and 
as mentioned before, it's not always the case.
This package name is stored in Arg0, so we can use it as fallback.

After this change, I can see com.amazon.dee.app as process name. Before I 
couldn't

PIDPARENT USER   TRIPLE NAME




16876  1  shell /system/bin/adbd
19941  982u0_a239   com.amazon.dee.app

Another way to do it is to have a specific variable in ProcessInfo for 
app_bundle_name, which could be the actual fallback when the executable path is 
empty. However, I don't think there's a case in which Arg0 is not the correct 
value.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68968

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
  lldb/source/Utility/ProcessInfo.cpp

Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -38,11 +38,18 @@
   m_pid = LLDB_INVALID_PROCESS_ID;
 }
 
+// In systems like android, there are cases in which a process name can
+// correspond to a package name (e.g. com.test.app), which is not an executable
+// path. This package name is stored in Arg0, so we can use it as fallback.
 const char *ProcessInfo::GetName() const {
+  if (m_executable.GetFilename().IsEmpty() && !GetArg0().empty())
+return GetArg0().data();
   return m_executable.GetFilename().GetCString();
 }
 
 llvm::StringRef ProcessInfo::GetNameAsStringRef() const {
+  if (m_executable.GetFilename().IsEmpty() && !GetArg0().empty())
+return GetArg0();
   return m_executable.GetFilename().GetStringRef();
 }
 
Index: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
@@ -5,9 +5,11 @@
 from lldbsuite.test.decorators import *
 from gdbclientutils import *
 
+
 def hexlify(string):
 return binascii.hexlify(string.encode()).decode()
 
+
 class TestPlatformClient(GDBRemoteTestBase):
 
 def test_process_list_with_all_users(self):
@@ -16,31 +18,42 @@
 class MyResponder(MockGDBServerResponder):
 def __init__(self):
 MockGDBServerResponder.__init__(self)
-self.currentQsProc = 0
+self.currentProc = 0
 self.all_users = False
 
+def _processInfo0(self):
+name = hexlify("/a/test_process")
+args = "-".join(map(hexlify,
+["/system/bin/sh", "-c", "/data/local/tmp/lldb-server"]))
+return "pid:10;ppid:1;uid:2;gid:3;euid:4;egid:5;name:" + name + ";args:" + args + ";"
+
+def _processInfo1(self):
+name = hexlify("/b/another_test_process")
+# This intentionally has a badly encoded argument
+args = "X".join(map(hexlify,
+["/system/bin/ls", "--help"]))
+return "pid:11;ppid:2;uid:3;gid:4;euid:5;egid:6;name:" + name + ";args:" + args + ";"
+
+def _processInfo2(self):
+# a process with an empty name but an arg0, which can happen on android
+args = hexlify("com.test.app")
+return "pid:12;ppid:3;uid:4;gid:5;euid:6;egid:7;name:;args:" + args + ";"
+
 def qfProcessInfo(self, packet):
-if "all_users:1" in packet:
-self.all_users = True
-name = hexlify("/a/test_process")
-args = "-".join(map(hexlify,
-["/system/bin/sh", "-c", "/data/local/tmp/lldb-server"]))
-return "pid:10;ppid:1;uid:2;gid:3;euid:4;egid:5;name:" + name + ";args:" + args + ";"
-else:
-self.all_users = False
-return "E04"
+self.all_users = "all_users:1" in packet
+self.currentProc = 1
+return self._processInfo0()
 
 def qsProcessInfo(self):
 if self.all_users:
-if self.currentQsProc == 0:
-self.currentQsProc = 1
-name = hexlify("/b/another_test_process")
-# 

[Lldb-commits] [PATCH] D68858: [lldb] Creates _liblldb symlink from cmake

[Haibo Huang via Phabricator via lldb-commits]

hhb updated this revision to Diff 224954.
hhb added a comment.

Fix the build for multi-config generator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68858

Files:
  lldb/CMakeLists.txt
  lldb/scripts/Python/finishSwigPythonLLDB.py

Index: lldb/scripts/Python/finishSwigPythonLLDB.py
===
--- lldb/scripts/Python/finishSwigPythonLLDB.py
+++ lldb/scripts/Python/finishSwigPythonLLDB.py
@@ -245,284 +245,6 @@
 
 return (bOk, strMsg)
 
-#++---
-# Details:  Make the symbolic link on a Windows platform.
-# Args: vstrSrcFile - (R) Source file name.
-#   vstrTargetFile  - (R) Destination file name.
-# Returns:  Bool - True = function success, False = failure.
-#   Str - Error description on task failure.
-# Throws:   None.
-#--
-
-
-def make_symlink_windows(vstrSrcPath, vstrTargetPath):
-print(("Making symlink from %s to %s" % (vstrSrcPath, vstrTargetPath)))
-dbg = utilsDebug.CDebugFnVerbose("Python script make_symlink_windows()")
-bOk = True
-strErrMsg = ""
-# If the src file doesn't exist, this is an error and we should throw.
-src_stat = os.stat(vstrSrcPath)
-
-try:
-target_stat = os.stat(vstrTargetPath)
-# If the target file exists but refers to a different file, delete it so that we can
-# re-create the link.  This can happen if you run this script once (creating a link)
-# and then delete the source file (so that a brand new file gets created the next time
-# you compile and link), and then re-run this script, so that both the target hardlink
-# and the source file exist, but the target refers to an old copy of
-# the source.
-if (target_stat.st_ino == src_stat.st_ino) and (
-target_stat.st_dev == src_stat.st_dev):
-return (bOk, strErrMsg)
-
-os.remove(vstrTargetPath)
-except:
-# If the target file don't exist, ignore this exception, we will link
-# it shortly.
-pass
-
-try:
-csl = ctypes.windll.kernel32.CreateHardLinkW
-csl.argtypes = (ctypes.c_wchar_p, ctypes.c_wchar_p, ctypes.c_uint32)
-csl.restype = ctypes.c_ubyte
-if csl(vstrTargetPath, vstrSrcPath, 0) == 0:
-raise ctypes.WinError()
-except Exception as e:
-if e.errno != 17:
-bOk = False
-strErrMsg = "WinError(%d): %s %s" % (
-e.errno, e.strerror, strErrMsgMakeSymlink)
-strErrMsg += " Src:'%s' Target:'%s'" % (
-vstrSrcPath, vstrTargetPath)
-
-return (bOk, strErrMsg)
-
-#++---
-# Details:  Make the symbolic link on a UNIX style platform.
-# Args: vstrSrcFile - (R) Source file name.
-#   vstrTargetFile  - (R) Destination file name.
-# Returns:  Bool - True = function success, False = failure.
-#   Str - Error description on task failure.
-# Throws:   None.
-#--
-
-
-def make_symlink_other_platforms(vstrSrcPath, vstrTargetPath):
-dbg = utilsDebug.CDebugFnVerbose(
-"Python script make_symlink_other_platforms()")
-bOk = True
-strErrMsg = ""
-
-try:
-os.symlink(vstrSrcPath, vstrTargetPath)
-except OSError as e:
-bOk = False
-strErrMsg = "OSError(%d): %s %s" % (
-e.errno, e.strerror, strErrMsgMakeSymlink)
-strErrMsg += " Src:'%s' Target:'%s'" % (vstrSrcPath, vstrTargetPath)
-except:
-bOk = False
-strErrMsg = strErrMsgUnexpected % sys.exec_info()[0]
-
-return (bOk, strErrMsg)
-
-
-def make_symlink_native(vDictArgs, strSrc, strTarget):
-eOSType = utilsOsType.determine_os_type()
-bDbg = "-d" in vDictArgs
-bOk = True
-strErrMsg = ""
-
-target_filename = os.path.basename(strTarget)
-if eOSType == utilsOsType.EnumOsType.Unknown:
-bOk = False
-strErrMsg = strErrMsgOsTypeUnknown
-elif eOSType == utilsOsType.EnumOsType.Windows:
-if bDbg:
-print((strMsgSymlinkMk % (target_filename, strSrc, strTarget)))
-bOk, strErrMsg = make_symlink_windows(strSrc,
-  strTarget)
-else:
-if os.path.islink(strTarget):
-if bDbg:
-print((strMsgSymlinkExists % target_filename))
-return (bOk, strErrMsg)
-if bDbg:
-print((strMsgSymlinkMk % (target_filename, strSrc, strTarget)))
-bOk, strErrMsg = make_symlink_other_platforms(strSrc,
-  strTarget)
-
-return (bOk, strErrMsg)
-
-#++---
-# Details:  Make the symbolic link.
-# Args:   

[Lldb-commits] [PATCH] D68858: [lldb] Creates _liblldb symlink from cmake

[Haibo Huang via Phabricator via lldb-commits]

hhb updated this revision to Diff 224955.
hhb added a comment.

Oops fix typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68858

Files:
  lldb/CMakeLists.txt
  lldb/scripts/Python/finishSwigPythonLLDB.py

Index: lldb/scripts/Python/finishSwigPythonLLDB.py
===
--- lldb/scripts/Python/finishSwigPythonLLDB.py
+++ lldb/scripts/Python/finishSwigPythonLLDB.py
@@ -245,284 +245,6 @@
 
 return (bOk, strMsg)
 
-#++---
-# Details:  Make the symbolic link on a Windows platform.
-# Args: vstrSrcFile - (R) Source file name.
-#   vstrTargetFile  - (R) Destination file name.
-# Returns:  Bool - True = function success, False = failure.
-#   Str - Error description on task failure.
-# Throws:   None.
-#--
-
-
-def make_symlink_windows(vstrSrcPath, vstrTargetPath):
-print(("Making symlink from %s to %s" % (vstrSrcPath, vstrTargetPath)))
-dbg = utilsDebug.CDebugFnVerbose("Python script make_symlink_windows()")
-bOk = True
-strErrMsg = ""
-# If the src file doesn't exist, this is an error and we should throw.
-src_stat = os.stat(vstrSrcPath)
-
-try:
-target_stat = os.stat(vstrTargetPath)
-# If the target file exists but refers to a different file, delete it so that we can
-# re-create the link.  This can happen if you run this script once (creating a link)
-# and then delete the source file (so that a brand new file gets created the next time
-# you compile and link), and then re-run this script, so that both the target hardlink
-# and the source file exist, but the target refers to an old copy of
-# the source.
-if (target_stat.st_ino == src_stat.st_ino) and (
-target_stat.st_dev == src_stat.st_dev):
-return (bOk, strErrMsg)
-
-os.remove(vstrTargetPath)
-except:
-# If the target file don't exist, ignore this exception, we will link
-# it shortly.
-pass
-
-try:
-csl = ctypes.windll.kernel32.CreateHardLinkW
-csl.argtypes = (ctypes.c_wchar_p, ctypes.c_wchar_p, ctypes.c_uint32)
-csl.restype = ctypes.c_ubyte
-if csl(vstrTargetPath, vstrSrcPath, 0) == 0:
-raise ctypes.WinError()
-except Exception as e:
-if e.errno != 17:
-bOk = False
-strErrMsg = "WinError(%d): %s %s" % (
-e.errno, e.strerror, strErrMsgMakeSymlink)
-strErrMsg += " Src:'%s' Target:'%s'" % (
-vstrSrcPath, vstrTargetPath)
-
-return (bOk, strErrMsg)
-
-#++---
-# Details:  Make the symbolic link on a UNIX style platform.
-# Args: vstrSrcFile - (R) Source file name.
-#   vstrTargetFile  - (R) Destination file name.
-# Returns:  Bool - True = function success, False = failure.
-#   Str - Error description on task failure.
-# Throws:   None.
-#--
-
-
-def make_symlink_other_platforms(vstrSrcPath, vstrTargetPath):
-dbg = utilsDebug.CDebugFnVerbose(
-"Python script make_symlink_other_platforms()")
-bOk = True
-strErrMsg = ""
-
-try:
-os.symlink(vstrSrcPath, vstrTargetPath)
-except OSError as e:
-bOk = False
-strErrMsg = "OSError(%d): %s %s" % (
-e.errno, e.strerror, strErrMsgMakeSymlink)
-strErrMsg += " Src:'%s' Target:'%s'" % (vstrSrcPath, vstrTargetPath)
-except:
-bOk = False
-strErrMsg = strErrMsgUnexpected % sys.exec_info()[0]
-
-return (bOk, strErrMsg)
-
-
-def make_symlink_native(vDictArgs, strSrc, strTarget):
-eOSType = utilsOsType.determine_os_type()
-bDbg = "-d" in vDictArgs
-bOk = True
-strErrMsg = ""
-
-target_filename = os.path.basename(strTarget)
-if eOSType == utilsOsType.EnumOsType.Unknown:
-bOk = False
-strErrMsg = strErrMsgOsTypeUnknown
-elif eOSType == utilsOsType.EnumOsType.Windows:
-if bDbg:
-print((strMsgSymlinkMk % (target_filename, strSrc, strTarget)))
-bOk, strErrMsg = make_symlink_windows(strSrc,
-  strTarget)
-else:
-if os.path.islink(strTarget):
-if bDbg:
-print((strMsgSymlinkExists % target_filename))
-return (bOk, strErrMsg)
-if bDbg:
-print((strMsgSymlinkMk % (target_filename, strSrc, strTarget)))
-bOk, strErrMsg = make_symlink_other_platforms(strSrc,
-  strTarget)
-
-return (bOk, strErrMsg)
-
-#++---
-# Details:  Make the symbolic link.
-# Args: vDictArgs   -

[Lldb-commits] [PATCH] D68858: [lldb] Creates _liblldb symlink from cmake

[Haibo Huang via Phabricator via lldb-commits]

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



Comment at: lldb/CMakeLists.txt:244
+  else()
+set(LIBLLDB_SYMLINK_DEST 
"${liblldb_build_dir}/liblldb${CMAKE_SHARED_LIBRARY_SUFFIX}")
+  endif()

tatyana-krasnukha wrote:
> This command still produces a path without configuration name, Visual Studio 
> failed to execute the post-build step.
> 
Thanks for testing and the patch! I found that CMAKE_CFG_INTDIR can actually be 
passed into function and expanded to $(Configure). At least in VS2019 
community... Can you try this new version? Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68858



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