date:20190923

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

This is an interesting problem. As Jim suspects, breakpoint int3s come out as 
stop reason = breakpoint in lldb, and non-breakpoint int3 as SIGTRAP. So, in 
theory, it should be possible to change the SIGTRAP behavior so that it does 
not impact SIGTRAPs from lldb-generated breakpoints, though it will require 
some fiddling with the `QPassSignals` packet -- the way it's implemented now is 
that it would bypass the stop reason determination and forcefully reinject all 
signals (even the "debugger generated" ones into the process).

That said, I'm not sure this is actually a good idea. :) One of the uses of 
application-generated SIGTRAPs I've seen (and used) was to implement a poor 
man's "fast conditional breakpoint" -- you insert something like `if 
(do_i_want_to_stop) int3()` into the code, and let the application run under a 
debugger. The intention here is to be able to continue the program from this 
"breakpoint" and this will only work if the debugger does not reinject the 
SIGTRAP.

Now, this is not a super common pattern, so it might be fine to just ask to 
user to change the SIGTRAP disposition if he wants to do this. However, given 
that this is the default in gdb too, I am not sure if it is such a good idea to 
change it. As this problem is specifically about the behavior of process 
attach, maybe the problem should be solved at a higher level? Could we make 
"process attach" never set the "crashed" flag? Attaching to a process just at 
the moment it is about the crash is very unlikely, and I suspect it is actually 
impossible (on linux at least).

Alternatively, (and this is something that would be useful in in (lit) tests 
too), we could add a mechanism to selectively suppress the stop-on-error 
behavior. Something like `nohup process attach ` perhaps?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67776



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

[Lldb-commits] [lldb] r372549 - [lldb] Fix that importing decls in a TagDecl end up in wrong declaration context (partly reverts D61333)

2019-09-23 Thread Raphael Isemann via lldb-commits

Author: teemperor
Date: Mon Sep 23 00:27:14 2019
New Revision: 372549

URL: http://llvm.org/viewvc/llvm-project?rev=372549&view=rev
Log:
[lldb] Fix that importing decls in a TagDecl end up in wrong declaration 
context (partly reverts D61333)

Summary:
In D61333 we dropped some code from ClangASTSource that checks if imported 
declarations
ended up in the right DeclContext. While this code wasn't tested by the test 
suite (or better, it was hit
by the test suite but we didn't have any checks that were affected) and the 
code seems pointless
(as usually Decls should end up in the right DeclContext), it actually broke 
the data formatters in LLDB
and causes a bunch of obscure bugs where structs suddenly miss all their 
members. The first report we got about
this was that printing a std::map doesn't work anymore when simply doing "expr 
m" (m is the std::map).

This patch reverts D61333 partly and reintroduces the check in a more stricter 
way (we actually check now that
we *move* the Decl and it is in a single DeclContext). This should fix all the 
problems we currently have until
we figure out how to properly fix the underlying issues. I changed the order of 
some std::map formatter tests
which is currently the most reliable way to test this problem (it's a tricky 
setup, see description below).

Fixes rdar://55502701 and rdar://55129537

--

Some more explanation what is actually going on and what is going wrong:

The situation we have is that if we have a `std::map m` and do a `expr m`, we 
end up seeing an empty map
(even if `m` has elements). The reason for this is that our data formatter sees 
that std::pair has no
members. However, `frame var m` works just fine (and fixes all following `expr 
m` calls).

The reason for why `expr` breaks std::map is that we actually copy the std::map 
nodes in two steps in the
three ASTContexts that are involved: The debug information ASTContext (D-AST), 
the expression ASTContext
we created for the current expression (E-AST) and the persistent ASTContext we 
use for our $variables (P-AST).

When doing `expr m` we do a minimal import of `std::map` from D-AST to E-AST 
just do the type checking/codegen.
This copies std::map itself and does a minimal.import of `std::pair` 
(that is, we don't actually import
the `first` and `second` members as we don't need them for anything). After the 
expression is done, we take
the expression result and copy it from E-AST to P-AST. This imports the E-AST's 
`std::pair` into P-AST which still
has no `first` and `second` as they are still undeserialized. Once we are in 
P-AST, the data formatter tries to
inspect `std::map` (and also `std::pair` as that's what the elements are) and 
it asks for the `std::pair` members.
We see that `std::pair` has undeserialized members and go to the 
ExternalASTSource to ask for them. However,
P-ASTs ExternalASTSource points to D-AST (and not E-AST, which `std::pair` came 
from). It can't point to E-AST
as that is only temporary and already gone (and also doesn't actually contain 
all decls we have in P-AST).

So we go to D-AST to get the `std::pair` members. The ASTImporter is asked to 
copy over `std::pair` members
and first checks if `std::pair` is already in P-AST. However, it only finds the 
std::pair we got from E-AST, so it
can't use it's map of already imported declarations and does a comparison 
between the `std::pair` decls we have
Because the ASTImporter thinks they are different declarations, it creates a 
second `std::pair` and fills in the
members `first` and `second` into the second `std::pair`. However, the data 
formatter is looking at the first
`std::pair` which still has no members as they are in the other decl. Now we 
pretend we have no declarations
and just print an empty map as a fallback.

The hack we had before fixed this issue by moving `first` and `second` to the 
first declaration which makes
the formatters happy as they can now see the members in the DeclContext they 
are querying.

Obviously this is a temporary patch until we get a real fix but I'm not sure 
what's the best way to fix this.
Implementing that the ClassTemplateSpecializationDecl actually understands that 
the two std::pair's are the same
decl fixes the issue, but this doesn't fix the bug for all declarations. My 
preferred solution would be to
complete all declarations in E-AST before they get moved to P-AST (as we anyway 
have to do this from what I can
tell), but that might have unintended side-effects and not sure what's the best 
way to implement this.

Reviewers: friss, martong

Reviewed By: martong

Subscribers: aprantl, rnkovacs, christof, abidh, JDevlieghere, lldb-commits, 
shafik

Tags: #lldb

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

M

[Lldb-commits] [PATCH] D67803: [lldb] Fix that importing decls in a TagDecl end up in wrong declaration context (partly reverts D61333)

2019-09-23 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372549: [lldb] Fix that importing decls in a TagDecl end up 
in wrong declaration… (authored by teemperor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67803?vs=220956&id=221258#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67803

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp


Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -650,6 +650,20 @@
 
 m_ast_importer_sp->RequireCompleteType(copied_field_type);
   }
+  auto decl_context_non_const = const_cast(decl_context);
+
+  // The decl ended up in the wrong DeclContext. Let's fix that so
+  // the decl we copied will actually be found.
+  // FIXME: This is a horrible hack that shouldn't be necessary. However
+  // it seems our current setup sometimes fails to copy decls to the right
+  // place. See rdar://55129537.
+  if (copied_decl->getDeclContext() != decl_context) {
+assert(copied_decl->getDeclContext()->containsDecl(copied_decl));
+copied_decl->getDeclContext()->removeDecl(copied_decl);
+copied_decl->setDeclContext(decl_context_non_const);
+assert(!decl_context_non_const->containsDecl(copied_decl));
+decl_context_non_const->addDeclInternal(copied_decl);
+  }
 } else {
   SkippedDecls = true;
 }
Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
@@ -52,13 +52,26 @@
 self.addTearDownHook(cleanup)
 
 ns = self.namespace
-self.expect('frame variable ii',
+self.expect('p ii',
+substrs=['%s::map' % ns,
+ 'size=0',
+ '{}'])
+self.expect('frame var ii',
 substrs=['%s::map' % ns,
  'size=0',
  '{}'])
 
 lldbutil.continue_to_breakpoint(self.process(), bkpt)
 
+self.expect('p ii',
+substrs=['%s::map' % ns, 'size=2',
+ '[0] = ',
+ 'first = 0',
+ 'second = 0',
+ '[1] = ',
+ 'first = 1',
+ 'second = 1'])
+
 self.expect('frame variable ii',
 substrs=['%s::map' % ns, 'size=2',
  '[0] = ',
@@ -81,7 +94,7 @@
 
 lldbutil.continue_to_breakpoint(self.process(), bkpt)
 
-self.expect("frame variable ii",
+self.expect("p ii",
 substrs=['%s::map' % ns, 'size=8',
  '[5] = ',
  'first = 5',
@@ -90,7 +103,7 @@
  'first = 7',
  'second = 1'])
 
-self.expect("p ii",
+self.expect("frame var ii",
 substrs=['%s::map' % ns, 'size=8',
  '[5] = ',
  'first = 5',


Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -650,6 +650,20 @@
 
 m_ast_importer_sp->RequireCompleteType(copied_field_type);
   }
+  auto decl_context_non_const = const_cast(decl_context);
+
+  // The decl ended up in the wrong DeclContext. Let's fix that so
+  // the decl we copied will actually be found.
+  // FIXME: This is a horrible hack that shouldn't be necessary. However
+  // it seems our current setup sometimes fails to copy decls to the right
+  // place. See rdar://55129537.
+  if (copied_decl->getDeclContext() != decl_context) {
+assert(copied_decl->getDeclContext()->containsDecl(copied_decl));
+copied_decl->getDeclContext()->removeDecl(copied_decl);
+

[Lldb-commits] [PATCH] D65942: Disallow implicit conversion from pointers to bool in llvm::toStringRef

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.

Comment at: lldb/source/Symbol/TypeSystem.cpp:331
   "TypeSystem for language " +
-  llvm::toStringRef(Language::GetNameForLanguageType(language)) +
+  llvm::StringRef(Language::GetNameForLanguageType(language)) +
   " doesn't exist",

dblaikie wrote:
> teemperor wrote:
> > dblaikie wrote:
> > > Perhaps Language::GetNameForLanguageType should return StringRef to start 
> > > with?
> > > 
> > > (& guessing there should be an lldb test case demonstrating this fix?)
> > The problem is that this is used in LLDB's SB API which returns a const 
> > char* (and we can't change that). So we can't return a StringRef as that 
> > doesn't convert back to const char * (and we can't go over std::string or 
> > anything like that as the API doesn't transfer ownership back to the 
> > caller). I added an alternative method to save some typing (and maybe we 
> > can go fully StringRef in the future if we ever change the SB API strings).
> > 
> > And we don't test log message content in LLDB. Also the log message only 
> > happens on systems with unsupported type system (like MIPS or something 
> > like that). And we don't have any such bot that would even run the test.
> Those both seem a bit problematic... 
> 
> Is there no API migration strategy in the SB API? Introducing new versions of 
> functions and deprecating old ones?
> 
> And testing - clearly this code was buggy and worth fixing, so it ought to be 
> worth testing. Any chance of unit testing/API-level testing any of this?
> Is there no API migration strategy in the SB API? Introducing new versions of 
> functions and deprecating old ones?

Right now, there isn't. However, I am not very happy with SB layer restrictions 
constraining the internal APIs. One of the ways around that (which is already 
used in a some places) is to pass the value through a ConstString at the SB 
layer. This will guarantee that the string is null terminated and persistent 
(at the cost of some cpu and memory). Though I am not very fond of that 
solution either, it does not seem like too bad of an option for this case, as 
the set of language names is limited.

As for testing of logging, there definitely are ways to test that, but we 
(usually) don't do that. I think the best analogy here would be the LLVM_DEBUG 
output in llvm. This is also mainly a debugging aid, and we usually don't have 
dedicated tests for that (though it would certainly be possible to do that). I 
think both mechanisms would benefit from some testing, but the question is how 
much testing, and whether that time wouldn't be better spent improving test 
coverage elsewhere...

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

https://reviews.llvm.org/D65942

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

[Lldb-commits] [PATCH] D67583: Fix swig python package path

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D67583#1677141 , @hhb wrote:

> In D67583#1676531 , @labath wrote:
>
> > Since I'm already complaining about python paths, I'll add that the way we 
> > currently compute the python path is wrong for cross-compilation. (Because 
> > it will pick up the path from the host python, not the target one.) If 
> > we're going to be changing something here, it would be nice to fix that too 
> > (which, I hope, will also allow us avoid needing to keep multiple places in 
> > sync).
>
>
> I also realized that this is totally wrong for cross-compilation. We are 
> doing cross compilation to windows with MinGW so I need to fix that.
>
> Actually I don't know why we need get_python_lib(). From what I can see now, 
> the 3 paths should match:
>
> 1. finishSwigPythonLLDB.py write necessary files to 
> ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/distutils.sysconfig.get_python_lib()
> 2. lldb/scripts/CMakeLists.txt copies them to lib${LLVM_LIBDIR_SUFFIX} (if 
> not xcode)
> 3. ScriptInterpreterPython.cpp adds the right path into PYTHONPATH.
>
>   As long as these 3 paths matches, we are good. Then why not simply put 
> everything into ${liblldb path}/site-packages for all platforms?
>
>   (I'm gonna make a change to test that. But let me know if anything is 
> obviously wrong.


This will work for the python-in-lldb case. However, the idea is lldb can also 
be used as python package, from any python application (by just typing `import 
lldb`). For that to work, we need to install lldb python stuff to a location 
that will be searched for by python in its default configuration.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67583



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

[Lldb-commits] [PATCH] D67789: bugfix: File::GetWaitableHandle() should call fileno()

2019-09-23 Thread 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 adding the test. LGTM, modulo some nits inline.




Comment at: lldb/unittests/Host/CMakeLists.txt:14
   TaskPoolTest.cpp
+  FileTest.cpp
 )

Please keep the list sorted.



Comment at: lldb/unittests/Host/FileTest.cpp:29-32
+  EXPECT_GE(fd, 0);
+
+  FILE *stream = fdopen(fd, "r");
+  EXPECT_TRUE(stream);

nit: change EXPECT_XXX to ASSERT_XXX as it does not make sense to continue the 
test if the assertions are not true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67789



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

[Lldb-commits] [PATCH] D61240: Implement GetSystemIncludeDirectories for macOS

2019-09-23 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor abandoned this revision.
teemperor added a comment.
Herald added a subscriber: JDevlieghere.

This is no longer necessary as we abandon the "reimplement include directories" 
approach (we now look at the support files or require -gmodules which both 
provide the same information in a more reliable way).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61240



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

[Lldb-commits] [PATCH] D67583: Fix swig python package path

2019-09-23 Thread Michał Górny via Phabricator via lldb-commits

mgorny added a comment.

In D67583#1678505 , @labath wrote:

> This will work for the python-in-lldb case. However, the idea is lldb can 
> also be used as python package, from any python application (by just typing 
> `import lldb`). For that to work, we need to install lldb python stuff to a 
> location that will be searched for by python in its default configuration.


I agree. Hence, I sent D67890  yesterday.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67583



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

[Lldb-commits] [lldb] r372556 - [lldb] Reduce some dangerous boilerplate with CompletionRequest::ShiftArguments

2019-09-23 Thread Raphael Isemann via lldb-commits

Author: teemperor
Date: Mon Sep 23 01:16:19 2019
New Revision: 372556

URL: http://llvm.org/viewvc/llvm-project?rev=372556&view=rev
Log:
[lldb] Reduce some dangerous boilerplate with CompletionRequest::ShiftArguments

We should in general not allow external code to fiddle with the internals of
CompletionRequest, but until this is gone let's at least provide a utility
function that makes this less dangerous.

This also now correct updates the partially parsed argument list,
but it doesn't seem to be used by anything that is behind one of
the current shift/SetCursorIndex calls, so this doesn't seeem to
fix any currently used completion.

Modified:
lldb/trunk/include/lldb/Utility/CompletionRequest.h
lldb/trunk/source/Commands/CommandObjectHelp.cpp
lldb/trunk/source/Commands/CommandObjectMultiword.cpp
lldb/trunk/source/Interpreter/CommandInterpreter.cpp
lldb/trunk/unittests/Utility/CompletionRequestTest.cpp

Modified: lldb/trunk/include/lldb/Utility/CompletionRequest.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/CompletionRequest.h?rev=372556&r1=372555&r2=372556&view=diff
==
--- lldb/trunk/include/lldb/Utility/CompletionRequest.h (original)
+++ lldb/trunk/include/lldb/Utility/CompletionRequest.h Mon Sep 23 01:16:19 2019
@@ -119,6 +119,13 @@ public:
 return GetParsedLine()[GetCursorIndex()];
   }
 
+  /// Drops the first argument from the argument list.
+  void ShiftArguments() {
+m_cursor_index--;
+m_parsed_line.Shift();
+m_partial_parsed_line.Shift();
+  }
+
   void SetCursorIndex(int i) { m_cursor_index = i; }
   int GetCursorIndex() const { return m_cursor_index; }
 

Modified: lldb/trunk/source/Commands/CommandObjectHelp.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectHelp.cpp?rev=372556&r1=372555&r2=372556&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectHelp.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectHelp.cpp Mon Sep 23 01:16:19 2019
@@ -215,8 +215,7 @@ void CommandObjectHelp::HandleCompletion
   // user is getting help on...
 
   if (cmd_obj) {
-request.GetParsedLine().Shift();
-request.SetCursorIndex(request.GetCursorIndex() - 1);
+request.ShiftArguments();
 cmd_obj->HandleCompletion(request);
 return;
   }

Modified: lldb/trunk/source/Commands/CommandObjectMultiword.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectMultiword.cpp?rev=372556&r1=372555&r2=372556&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectMultiword.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectMultiword.cpp Mon Sep 23 01:16:19 
2019
@@ -214,8 +214,7 @@ void CommandObjectMultiword::HandleCompl
   // Remove the one match that we got from calling GetSubcommandObject.
   new_matches.DeleteStringAtIndex(0);
   request.AddCompletions(new_matches);
-  request.GetParsedLine().Shift();
-  request.SetCursorIndex(request.GetCursorIndex() - 1);
+  request.ShiftArguments();
   sub_command_object->HandleCompletion(request);
 }
 

Modified: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandInterpreter.cpp?rev=372556&r1=372555&r2=372556&view=diff
==
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp (original)
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp Mon Sep 23 01:16:19 
2019
@@ -1794,8 +1794,7 @@ void CommandInterpreter::HandleCompletio
 CommandObject *command_object =
 GetCommandObject(request.GetParsedLine().GetArgumentAtIndex(0));
 if (command_object) {
-  request.GetParsedLine().Shift();
-  request.SetCursorIndex(request.GetCursorIndex() - 1);
+  request.ShiftArguments();
   command_object->HandleCompletion(request);
 }
   }

Modified: lldb/trunk/unittests/Utility/CompletionRequestTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/CompletionRequestTest.cpp?rev=372556&r1=372555&r2=372556&view=diff
==
--- lldb/trunk/unittests/Utility/CompletionRequestTest.cpp (original)
+++ lldb/trunk/unittests/Utility/CompletionRequestTest.cpp Mon Sep 23 01:16:19 
2019
@@ -31,6 +31,41 @@ TEST(CompletionRequest, Constructor) {
   EXPECT_STREQ(request.GetPartialParsedLine().GetArgumentAtIndex(1), "b");
 }
 
+TEST(CompletionRequest, ShiftArguments) {
+  std::string command = "a bad c";
+  const unsigned cursor_pos = 3;
+  const int arg_index = 1;
+  const int arg_cursor_pos = 1;
+  StringList matches;
+  CompletionResult result;
+
+  CompletionRequest request(command, cursor_pos, result);
+  result.GetMatches(matches);
+
+

[Lldb-commits] [lldb] r372561 - [lldb][NFC] Remove argument prefix checking boilerplate when adding completions

2019-09-23 Thread Raphael Isemann via lldb-commits

Author: teemperor
Date: Mon Sep 23 01:59:21 2019
New Revision: 372561

URL: http://llvm.org/viewvc/llvm-project?rev=372561&view=rev
Log:
[lldb][NFC] Remove argument prefix checking boilerplate when adding completions

Modified:
lldb/trunk/include/lldb/Utility/CompletionRequest.h
lldb/trunk/source/Commands/CommandCompletions.cpp
lldb/trunk/source/Interpreter/OptionValueBoolean.cpp
lldb/trunk/source/Interpreter/OptionValueEnumeration.cpp
lldb/trunk/source/Interpreter/OptionValueUUID.cpp
lldb/trunk/source/Interpreter/Options.cpp
lldb/trunk/source/Utility/ArchSpec.cpp
lldb/trunk/unittests/Utility/CompletionRequestTest.cpp

Modified: lldb/trunk/include/lldb/Utility/CompletionRequest.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/CompletionRequest.h?rev=372561&r1=372560&r2=372561&view=diff
==
--- lldb/trunk/include/lldb/Utility/CompletionRequest.h (original)
+++ lldb/trunk/include/lldb/Utility/CompletionRequest.h Mon Sep 23 01:59:21 2019
@@ -146,6 +146,24 @@ public:
 m_result.AddResult(completion, description, mode);
   }
 
+  /// Adds a possible completion string if the completion would complete the
+  /// current argument.
+  ///
+  /// \param match The suggested completion.
+  /// \param description An optional description of the completion string. The
+  /// description will be displayed to the user alongside the completion.
+  template 
+  void TryCompleteCurrentArg(llvm::StringRef completion,
+ llvm::StringRef description = "") {
+// Trying to rewrite the whole line while checking for the current
+// argument never makes sense. Completion modes are always hardcoded, so
+// this can be a static_assert.
+static_assert(M != CompletionMode::RewriteLine,
+  "Shouldn't rewrite line with this function");
+if (completion.startswith(GetCursorArgumentPrefix()))
+  AddCompletion(completion, description, M);
+  }
+
   /// Adds multiple possible completion strings.
   ///
   /// \param completions The list of completions.

Modified: lldb/trunk/source/Commands/CommandCompletions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandCompletions.cpp?rev=372561&r1=372560&r2=372561&view=diff
==
--- lldb/trunk/source/Commands/CommandCompletions.cpp (original)
+++ lldb/trunk/source/Commands/CommandCompletions.cpp Mon Sep 23 01:59:21 2019
@@ -308,10 +308,8 @@ void CommandCompletions::SettingsNames(C
 }
   }
 
-  for (const std::string &s : g_property_names) {
-if (llvm::StringRef(s).startswith(request.GetCursorArgumentPrefix()))
-  request.AddCompletion(s);
-  }
+  for (const std::string &s : g_property_names)
+request.TryCompleteCurrentArg(s);
 }
 
 void CommandCompletions::PlatformPluginNames(CommandInterpreter &interpreter,

Modified: lldb/trunk/source/Interpreter/OptionValueBoolean.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValueBoolean.cpp?rev=372561&r1=372560&r2=372561&view=diff
==
--- lldb/trunk/source/Interpreter/OptionValueBoolean.cpp (original)
+++ lldb/trunk/source/Interpreter/OptionValueBoolean.cpp Mon Sep 23 01:59:21 
2019
@@ -82,8 +82,6 @@ void OptionValueBoolean::AutoComplete(Co
   if (request.GetCursorArgumentPrefix().empty())
 entries = entries.take_front(2);
 
-  for (auto entry : entries) {
-if (entry.startswith_lower(request.GetCursorArgumentPrefix()))
-  request.AddCompletion(entry);
-  }
+  for (auto entry : entries)
+request.TryCompleteCurrentArg(entry);
 }

Modified: lldb/trunk/source/Interpreter/OptionValueEnumeration.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValueEnumeration.cpp?rev=372561&r1=372560&r2=372561&view=diff
==
--- lldb/trunk/source/Interpreter/OptionValueEnumeration.cpp (original)
+++ lldb/trunk/source/Interpreter/OptionValueEnumeration.cpp Mon Sep 23 
01:59:21 2019
@@ -108,8 +108,7 @@ void OptionValueEnumeration::AutoComplet
   if (!request.GetCursorArgumentPrefix().empty()) {
 for (size_t i = 0; i < num_enumerators; ++i) {
   llvm::StringRef name = 
m_enumerations.GetCStringAtIndex(i).GetStringRef();
-  if (name.startswith(request.GetCursorArgumentPrefix()))
-request.AddCompletion(name);
+  request.TryCompleteCurrentArg(name);
 }
 return;
   }

Modified: lldb/trunk/source/Interpreter/OptionValueUUID.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionValueUUID.cpp?rev=372561&r1=372560&r2=372561&view=diff
==
--- lldb/trunk/source/Interpreter/OptionValueUUID.cpp (original)
+++ lldb/trunk

[Lldb-commits] [lldb] r372566 - [lldb] Make cursor index in CompletionRequest unsigned

2019-09-23 Thread Raphael Isemann via lldb-commits

Author: teemperor
Date: Mon Sep 23 02:46:17 2019
New Revision: 372566

URL: http://llvm.org/viewvc/llvm-project?rev=372566&view=rev
Log:
[lldb] Make cursor index in CompletionRequest unsigned

The fact that index==-1 means "no arguments" is not obvious and only
used in one place from what I can tell. Also fixes several warnings
about using the cursor index as if it was a size_t when comparing.

Not fully NFC as we now also correctly update the partial argument list
when injecting the fake empty argument in the CompletionRequest
constructor.

Modified:
lldb/trunk/include/lldb/Utility/CompletionRequest.h
lldb/trunk/source/Commands/CommandObjectSettings.cpp
lldb/trunk/source/Interpreter/CommandInterpreter.cpp
lldb/trunk/source/Interpreter/Options.cpp
lldb/trunk/source/Utility/CompletionRequest.cpp
lldb/trunk/unittests/Utility/CompletionRequestTest.cpp

Modified: lldb/trunk/include/lldb/Utility/CompletionRequest.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/CompletionRequest.h?rev=372566&r1=372565&r2=372566&view=diff
==
--- lldb/trunk/include/lldb/Utility/CompletionRequest.h (original)
+++ lldb/trunk/include/lldb/Utility/CompletionRequest.h Mon Sep 23 02:46:17 2019
@@ -126,8 +126,8 @@ public:
 m_partial_parsed_line.Shift();
   }
 
-  void SetCursorIndex(int i) { m_cursor_index = i; }
-  int GetCursorIndex() const { return m_cursor_index; }
+  void SetCursorIndex(size_t i) { m_cursor_index = i; }
+  size_t GetCursorIndex() const { return m_cursor_index; }
 
   void SetCursorCharPosition(int pos) { m_cursor_char_position = pos; }
   int GetCursorCharPosition() const { return m_cursor_char_position; }
@@ -208,7 +208,7 @@ private:
   /// The command line until the cursor position parsed as arguments.
   Args m_partial_parsed_line;
   /// The index of the argument in which the completion cursor is.
-  int m_cursor_index;
+  size_t m_cursor_index;
   /// The cursor position in the argument indexed by m_cursor_index.
   int m_cursor_char_position;
 

Modified: lldb/trunk/source/Commands/CommandObjectSettings.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectSettings.cpp?rev=372566&r1=372565&r2=372566&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectSettings.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectSettings.cpp Mon Sep 23 02:46:17 
2019
@@ -130,9 +130,8 @@ insert-before or insert-after.");
 
 const size_t argc = request.GetParsedLine().GetArgumentCount();
 const char *arg = nullptr;
-int setting_var_idx;
-for (setting_var_idx = 0; setting_var_idx < static_cast(argc);
- ++setting_var_idx) {
+size_t setting_var_idx;
+for (setting_var_idx = 0; setting_var_idx < argc; ++setting_var_idx) {
   arg = request.GetParsedLine().GetArgumentAtIndex(setting_var_idx);
   if (arg && arg[0] != '-')
 break; // We found our setting variable name index

Modified: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandInterpreter.cpp?rev=372566&r1=372565&r2=372566&view=diff
==
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp (original)
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp Mon Sep 23 02:46:17 
2019
@@ -1756,7 +1756,7 @@ void CommandInterpreter::HandleCompletio
 
   // For any of the command completions a unique match will be a complete word.
 
-  if (request.GetCursorIndex() == -1) {
+  if (request.GetPartialParsedLine().GetArgumentCount() == 0) {
 // We got nothing on the command line, so return the list of commands
 bool include_aliases = true;
 StringList new_matches, descriptions;
@@ -1780,7 +1780,7 @@ void CommandInterpreter::HandleCompletio
 new_matches.DeleteStringAtIndex(0);
 new_descriptions.DeleteStringAtIndex(0);
 request.GetParsedLine().AppendArgument(llvm::StringRef());
-request.SetCursorIndex(request.GetCursorIndex() + 1);
+request.SetCursorIndex(request.GetCursorIndex() + 1U);
 request.SetCursorCharPosition(0);
   }
 }

Modified: lldb/trunk/source/Interpreter/Options.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Options.cpp?rev=372566&r1=372565&r2=372566&view=diff
==
--- lldb/trunk/source/Interpreter/Options.cpp (original)
+++ lldb/trunk/source/Interpreter/Options.cpp Mon Sep 23 02:46:17 2019
@@ -655,8 +655,8 @@ bool Options::HandleOptionCompletion(Com
   llvm::StringRef cur_opt_str = request.GetCursorArgumentPrefix();
 
   for (size_t i = 0; i < opt_element_vector.size(); i++) {
-int opt_pos = opt_element_vector[i].opt_pos;
-int opt_arg_pos = opt_element

[Lldb-commits] [PATCH] D67861: [LLDB] Check for the MINGW32 define instead of __MINGW64

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added a reviewer: amccarth.
labath added a comment.

Judging by the microsoft docs 
,
 `vsnprintf` is avaliable at least since VS 2017 (the lowest version supported 
by llvm). Is it possible that we can just delete this `#ifdef` ?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67861



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

[Lldb-commits] [lldb] r372568 - [lldb][NFC] Make cursor char position unsigned in CompletionRequest

2019-09-23 Thread Raphael Isemann via lldb-commits

Author: teemperor
Date: Mon Sep 23 02:51:36 2019
New Revision: 372568

URL: http://llvm.org/viewvc/llvm-project?rev=372568&view=rev
Log:
[lldb][NFC] Make cursor char position unsigned in CompletionRequest

This was only an 'int' because to fit into the old API which is
gone by now.

Modified:
lldb/trunk/include/lldb/Utility/CompletionRequest.h
lldb/trunk/unittests/Utility/CompletionRequestTest.cpp

Modified: lldb/trunk/include/lldb/Utility/CompletionRequest.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/CompletionRequest.h?rev=372568&r1=372567&r2=372568&view=diff
==
--- lldb/trunk/include/lldb/Utility/CompletionRequest.h (original)
+++ lldb/trunk/include/lldb/Utility/CompletionRequest.h Mon Sep 23 02:51:36 2019
@@ -129,8 +129,8 @@ public:
   void SetCursorIndex(size_t i) { m_cursor_index = i; }
   size_t GetCursorIndex() const { return m_cursor_index; }
 
-  void SetCursorCharPosition(int pos) { m_cursor_char_position = pos; }
-  int GetCursorCharPosition() const { return m_cursor_char_position; }
+  void SetCursorCharPosition(size_t pos) { m_cursor_char_position = pos; }
+  size_t GetCursorCharPosition() const { return m_cursor_char_position; }
 
   /// Adds a possible completion string. If the completion was already
   /// suggested before, it will not be added to the list of results. A copy of
@@ -210,7 +210,7 @@ private:
   /// The index of the argument in which the completion cursor is.
   size_t m_cursor_index;
   /// The cursor position in the argument indexed by m_cursor_index.
-  int m_cursor_char_position;
+  size_t m_cursor_char_position;
 
   /// The result this request is supposed to fill out.
   /// We keep this object private to ensure that no backend can in any way

Modified: lldb/trunk/unittests/Utility/CompletionRequestTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/CompletionRequestTest.cpp?rev=372568&r1=372567&r2=372568&view=diff
==
--- lldb/trunk/unittests/Utility/CompletionRequestTest.cpp (original)
+++ lldb/trunk/unittests/Utility/CompletionRequestTest.cpp Mon Sep 23 02:51:36 
2019
@@ -15,7 +15,7 @@ TEST(CompletionRequest, Constructor) {
   std::string command = "a bad c";
   const unsigned cursor_pos = 3;
   const size_t arg_index = 1;
-  const int arg_cursor_pos = 1;
+  const size_t arg_cursor_pos = 1;
   StringList matches;
   CompletionResult result;
 
@@ -43,7 +43,7 @@ TEST(CompletionRequest, FakeLastArg) {
   EXPECT_STREQ(request.GetRawLine().str().c_str(), command.c_str());
   EXPECT_EQ(request.GetRawCursorPos(), cursor_pos);
   EXPECT_EQ(request.GetCursorIndex(), 3U);
-  EXPECT_EQ(request.GetCursorCharPosition(), 0);
+  EXPECT_EQ(request.GetCursorCharPosition(), 0U);
 
   EXPECT_EQ(request.GetPartialParsedLine().GetArgumentCount(), 4U);
   EXPECT_STREQ(request.GetPartialParsedLine().GetArgumentAtIndex(3), "");
@@ -90,7 +90,7 @@ TEST(CompletionRequest, ShiftArguments)
   std::string command = "a bad c";
   const unsigned cursor_pos = 3;
   const size_t arg_index = 1;
-  const int arg_cursor_pos = 1;
+  const size_t arg_cursor_pos = 1;
   StringList matches;
   CompletionResult result;
 


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

[Lldb-commits] [lldb] r372569 - [lldb][NFC] Fix documentation of CompletionRequest::AddCompletion

2019-09-23 Thread Raphael Isemann via lldb-commits

Author: teemperor
Date: Mon Sep 23 02:53:33 2019
New Revision: 372569

URL: http://llvm.org/viewvc/llvm-project?rev=372569&view=rev
Log:
[lldb][NFC] Fix documentation of CompletionRequest::AddCompletion

Modified:
lldb/trunk/include/lldb/Utility/CompletionRequest.h

Modified: lldb/trunk/include/lldb/Utility/CompletionRequest.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/CompletionRequest.h?rev=372569&r1=372568&r2=372569&view=diff
==
--- lldb/trunk/include/lldb/Utility/CompletionRequest.h (original)
+++ lldb/trunk/include/lldb/Utility/CompletionRequest.h Mon Sep 23 02:53:33 2019
@@ -138,8 +138,9 @@ public:
   /// afterwards.
   ///
   /// \param match The suggested completion.
-  /// \param match An optional description of the completion string. The
+  /// \param completion An optional description of the completion string. The
   /// description will be displayed to the user alongside the completion.
+  /// \param mode The CompletionMode for this completion.
   void AddCompletion(llvm::StringRef completion,
  llvm::StringRef description = "",
  CompletionMode mode = CompletionMode::Normal) {


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

[Lldb-commits] [lldb] r372572 - [lldb][NFC] Remove dead code in Options::HandleOptionArgumentCompletion

2019-09-23 Thread Raphael Isemann via lldb-commits

Author: teemperor
Date: Mon Sep 23 02:56:53 2019
New Revision: 372572

URL: http://llvm.org/viewvc/llvm-project?rev=372572&view=rev
Log:
[lldb][NFC] Remove dead code in Options::HandleOptionArgumentCompletion

Modified:
lldb/trunk/source/Interpreter/Options.cpp

Modified: lldb/trunk/source/Interpreter/Options.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Options.cpp?rev=372572&r1=372571&r2=372572&view=diff
==
--- lldb/trunk/source/Interpreter/Options.cpp (original)
+++ lldb/trunk/source/Interpreter/Options.cpp Mon Sep 23 02:56:53 2019
@@ -752,16 +752,9 @@ void Options::HandleOptionArgumentComple
   // See if this is an enumeration type option, and if so complete it here:
 
   const auto &enum_values = opt_defs[opt_defs_index].enum_values;
-  if (!enum_values.empty()) {
-std::string match_string(
-request.GetParsedLine().GetArgumentAtIndex(opt_arg_pos),
-request.GetParsedLine().GetArgumentAtIndex(opt_arg_pos) +
-request.GetCursorCharPosition());
-
-for (const auto &enum_value : enum_values) {
+  if (!enum_values.empty())
+for (const auto &enum_value : enum_values)
   request.TryCompleteCurrentArg(enum_value.string_value);
-}
-  }
 
   // If this is a source file or symbol type completion, and  there is a -shlib
   // option somewhere in the supplied arguments, then make a search filter for


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

[Lldb-commits] [PATCH] D67885: [LLDB] Add a missing specification of linking against dbghelp

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

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

LGTM. It might be nice to add a comment that this is for saving of minidump 
files (I assume that's the only reason). The reason for that is the object file 
plugins are generally expected to work on any host, and so linking os-specific 
libraries is a bit surprising. Given that lldb is slowly growing support for 
minidump files on non-windows platforms, it is a matter of time when we will 
create an os-independent minidump writer (a lot of the pieces are already 
there), and then this bit of code can be removed.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67885



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

[Lldb-commits] [lldb] r372574 - [lldb][NFC] Remove unused variable in Options::HandleOptionArgumentCompletion

2019-09-23 Thread Raphael Isemann via lldb-commits

Author: teemperor
Date: Mon Sep 23 03:02:26 2019
New Revision: 372574

URL: http://llvm.org/viewvc/llvm-project?rev=372574&view=rev
Log:
[lldb][NFC] Remove unused variable in Options::HandleOptionArgumentCompletion

Modified:
lldb/trunk/source/Interpreter/Options.cpp

Modified: lldb/trunk/source/Interpreter/Options.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Options.cpp?rev=372574&r1=372573&r2=372574&view=diff
==
--- lldb/trunk/source/Interpreter/Options.cpp (original)
+++ lldb/trunk/source/Interpreter/Options.cpp Mon Sep 23 03:02:26 2019
@@ -746,7 +746,6 @@ void Options::HandleOptionArgumentComple
   auto opt_defs = GetDefinitions();
   std::unique_ptr filter_up;
 
-  int opt_arg_pos = opt_element_vector[opt_element_index].opt_arg_pos;
   int opt_defs_index = opt_element_vector[opt_element_index].opt_defs_index;
 
   // See if this is an enumeration type option, and if so complete it here:


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

[Lldb-commits] [PATCH] D67861: [LLDB] Check for the MINGW32 define instead of __MINGW64

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo added a comment.

In D67861#1678618 , @labath wrote:

> Judging by the microsoft docs 
> ,
>  `vsnprintf` is avaliable at least since VS 2017 (the lowest version 
> supported by llvm). Is it possible that we can just delete this `#ifdef` ?

Yes, it would seem so.

With MSVC, you got an MSVC-specific version of the CRT, and with the versions 
of MSVC we require, it is a C99 compliant one.

With MinGW, you can link either against the UCRT (the same modern CRT as MSVC 
uses these days) or msvcrt.dll (the old legacy one, shipped as a part of the 
OS). But I checked that this use of `vsnprintf(NULL, 0, ...)` does seem to work 
with the old msvcrt.dll since at least XP (and llvm requires a much newer 
version of Windows anyway), so I would say that it should be safe.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67861

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

[Lldb-commits] [PATCH] D67861: [LLDB] Remove a now redundant windows specific workaround

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo updated this revision to Diff 221276.
mstorsjo retitled this revision from "[LLDB] Check for the __MINGW32__ define 
instead of __MINGW64" to "[LLDB] Remove a now redundant windows specific 
workaround".
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Updated the patch to simply remove the now redundant workaround.


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

https://reviews.llvm.org/D67861

Files:
  lldb/source/Host/windows/Windows.cpp


Index: lldb/source/Host/windows/Windows.cpp
===
--- lldb/source/Host/windows/Windows.cpp
+++ lldb/source/Host/windows/Windows.cpp
@@ -48,13 +48,8 @@
   size_t buflen;
   va_list ap2;
 
-#if defined(_MSC_VER) || defined(__MINGW64)
-  ap2 = ap;
-  len = _vscprintf(fmt, ap2);
-#else
   va_copy(ap2, ap);
   len = vsnprintf(NULL, 0, fmt, ap2);
-#endif
 
   if (len >= 0 &&
   (buf = (char *)malloc((buflen = (size_t)(len + 1 != NULL) {


Index: lldb/source/Host/windows/Windows.cpp
===
--- lldb/source/Host/windows/Windows.cpp
+++ lldb/source/Host/windows/Windows.cpp
@@ -48,13 +48,8 @@
   size_t buflen;
   va_list ap2;
 
-#if defined(_MSC_VER) || defined(__MINGW64)
-  ap2 = ap;
-  len = _vscprintf(fmt, ap2);
-#else
   va_copy(ap2, ap);
   len = vsnprintf(NULL, 0, fmt, ap2);
-#endif
 
   if (len >= 0 &&
   (buf = (char *)malloc((buflen = (size_t)(len + 1 != NULL) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67903: [lldb] Add completion support for log enable/disable/list

2019-09-23 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: LLDB.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67903

Files:
  lldb/include/lldb/Utility/Log.h
  
lldb/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Utility/Log.cpp

Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -9,7 +9,6 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/VASPrintf.h"
 
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator.h"
@@ -38,13 +37,21 @@
 
 llvm::ManagedStatic Log::g_channel_map;
 
-void Log::ListCategories(llvm::raw_ostream &stream, const ChannelMap::value_type &entry) {
-  stream << llvm::formatv("Logging categories for '{0}':\n", entry.first());
-  stream << "  all - all available logging categories\n";
-  stream << "  default - default set of logging categories\n";
+void Log::ForEachCategory(
+const Log::ChannelMap::value_type &entry,
+llvm::function_ref lambda) {
+  lambda("all", "all available logging categories");
+  lambda("default", "default set of logging categories");
   for (const auto &category : entry.second.m_channel.categories)
-stream << llvm::formatv("  {0} - {1}\n", category.name,
-category.description);
+lambda(category.name, category.description);
+}
+
+void Log::ListCategories(llvm::raw_ostream &stream,
+ const ChannelMap::value_type &entry) {
+  ForEachCategory(entry,
+  [&stream](llvm::StringRef name, llvm::StringRef description) {
+stream << llvm::formatv("  {0} - {1}\n", name, description);
+  });
 }
 
 uint32_t Log::GetFlags(llvm::raw_ostream &stream, const ChannelMap::value_type &entry,
@@ -237,6 +244,23 @@
 entry.second.Disable(UINT32_MAX);
 }
 
+void Log::ForEachChannelCategory(
+llvm::StringRef channel,
+llvm::function_ref lambda) {
+  auto ch = g_channel_map->find(channel);
+  if (ch == g_channel_map->end())
+return;
+
+  ForEachCategory(*ch, lambda);
+}
+
+std::vector Log::ListChannels() {
+  std::vector result;
+  for (const auto &channel : *g_channel_map)
+result.push_back(channel.first());
+  return result;
+}
+
 void Log::ListAllLogChannels(llvm::raw_ostream &stream) {
   if (g_channel_map->empty()) {
 stream << "No logging channels are currently registered.\n";
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -34,6 +34,21 @@
 #define LLDB_OPTIONS_log
 #include "CommandOptions.inc"
 
+/// Common completion logic for log enable/disable.
+static void CompleteEnableDisable(CompletionRequest &request) {
+  size_t arg_index = request.GetCursorIndex();
+  if (arg_index == 0) { // We got: log enable/disable x[tab]
+for (llvm::StringRef channel : Log::ListChannels())
+  request.TryCompleteCurrentArg(channel);
+  } else if (arg_index >= 1) { // We got: log enable/disable channel x[tab]
+llvm::StringRef channel = request.GetParsedLine().GetArgumentAtIndex(0);
+Log::ForEachChannelCategory(
+channel, [&request](llvm::StringRef name, llvm::StringRef desc) {
+  request.TryCompleteCurrentArg(name, desc);
+});
+  }
+}
+
 class CommandObjectLogEnable : public CommandObjectParsed {
 public:
   // Constructors and Destructors
@@ -134,6 +149,12 @@
 uint32_t log_options;
   };
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+CompleteEnableDisable(request);
+  }
+
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
 if (args.GetArgumentCount() < 2) {
@@ -202,6 +223,12 @@
 
   ~CommandObjectLogDisable() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+CompleteEnableDisable(request);
+  }
+
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
 if (args.empty()) {
@@ -254,6 +281,13 @@
 
   ~CommandObjectLogList() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+for (llvm::StringRef channel : Log::ListChannels())
+  request.TryCompleteCurrentArg(channel);
+  }
+
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
 std::string output;
Index: lldb/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
=

[Lldb-commits] [PATCH] D67861: [LLDB] Remove a now redundant windows specific workaround

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

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

Awesome. Let's give that a spin.


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

https://reviews.llvm.org/D67861



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

[Lldb-commits] [PATCH] D67896: [LLDB] Add a void* cast when passing object pointers to printf %p

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

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

This looks fine. When you see a pattern like `if(log) { LLDB_LOGF(...) }`, feel 
free to delete the surrounding `if(log)`. It is no longer necessary, and the 
reason it is still there is simply because Jonas's vim script wasn't smart 
enough to delete it in some cases.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67896



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

[Lldb-commits] [PATCH] D67894: [LLDB] Rework a MinGW build fix from D65691

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Let's wait for @hhb to give this a spin to avoid twiddling this back and forth.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67894



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

[Lldb-commits] [PATCH] D67885: [LLDB] Add a missing specification of linking against dbghelp

2019-09-23 Thread MyDeveloperDay via Phabricator via lldb-commits

MyDeveloperDay added a comment.

Just as a drive by comment, there is code in  DebugInfo/Symbolize/Symbolize.cpp 
that loads dbghelp via #pragma comment(lib... trick

There is also code to load dbghelp dynamically in 
lib\Support\Windows\Signal.inc including loading MiniDumpWriteDump


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67885



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

[Lldb-commits] [PATCH] D67885: [LLDB] Add a missing specification of linking against dbghelp

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo added a comment.

In D67885#1678701 , @MyDeveloperDay 
wrote:

> Just as a drive by comment, there is code in  
> DebugInfo/Symbolize/Symbolize.cpp that loads dbghelp via #pragma 
> comment(lib... trick


Yes, but as I mentioned in the patch description here, #pragma comment(lib) 
only works in MSVC mode, it isn't supported when targeting MinGW unless 
-fms-extensions is specified (and if building with GCC and/or linking with GNU 
ld, it's not supported at all).

> There is also code to load dbghelp dynamically in 
> lib\Support\Windows\Signal.inc including loading MiniDumpWriteDump

I guess that could be an option as well, for a slightly larger change instead.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67885



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

[Lldb-commits] [PATCH] D67906: [lldb] Provide tab completion for target select/delete

2019-09-23 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: LLDB.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.
Herald added a project: LLDB.

Select and delete are similar enough to implement/test this in one batch.

Completion looks like this:

  (lldb) target select 
  Available completions:
0 -- /Users/me/test/foo ( arch=x86_64-apple-macosx10.14.0, 
platform=host, pid=65449, state=stopped )
1 -- /Users/me/test/basic ( arch=x86_64-apple-macosx10.14.0, 
platform=host )


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67906

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
  lldb/source/Commands/CommandObjectTarget.cpp

Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -60,7 +60,8 @@
 
 static void DumpTargetInfo(uint32_t target_idx, Target *target,
const char *prefix_cstr,
-   bool show_stopped_process_status, Stream &strm) {
+   bool show_stopped_process_status,
+   bool print_target_index, Stream &strm) {
   const ArchSpec &target_arch = target->GetArchitecture();
 
   Module *exe_module = target->GetExecutableModulePointer();
@@ -72,8 +73,13 @@
   if (!exe_valid)
 ::strcpy(exe_path, "");
 
-  strm.Printf("%starget #%u: %s", prefix_cstr ? prefix_cstr : "", target_idx,
-  exe_path);
+  if (prefix_cstr)
+strm << prefix_cstr;
+
+  if (print_target_index)
+strm.Printf("target #%u: ", target_idx);
+
+  strm << exe_path;
 
   uint32_t properties = 0;
   if (target_arch.IsValid()) {
@@ -125,14 +131,36 @@
   TargetSP target_sp(target_list.GetTargetAtIndex(i));
   if (target_sp) {
 bool is_selected = target_sp.get() == selected_target_sp.get();
+bool print_target_index = true;
 DumpTargetInfo(i, target_sp.get(), is_selected ? "* " : "  ",
-   show_stopped_process_status, strm);
+   show_stopped_process_status, print_target_index, strm);
   }
 }
   }
   return num_targets;
 }
 
+static void CompleteTargetIndex(CompletionRequest &request,
+Debugger &debugger) {
+  TargetList &target_list = debugger.GetTargetList();
+  size_t number_of_targets = target_list.GetNumTargets();
+  for (size_t target_i = 0; target_i < number_of_targets; ++target_i) {
+// The completion is only the target index.
+std::string completion = std::to_string(target_i);
+
+// Generate the description.
+TargetSP target_sp(target_list.GetTargetAtIndex(target_i));
+StreamString stream;
+bool show_stopped_process_status = false;
+bool print_thread_index = false;
+DumpTargetInfo(target_i, target_sp.get(), "", show_stopped_process_status,
+   print_thread_index, stream);
+llvm::StringRef description = stream.GetString().rtrim();
+
+request.TryCompleteCurrentArg(completion, description);
+  }
+}
+
 // Note that the negation in the argument name causes a slightly confusing
 // mapping of the enum values.
 static constexpr OptionEnumValueElement g_dependents_enumaration[] = {
@@ -524,6 +552,14 @@
 
   ~CommandObjectTargetSelect() override = default;
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+if (request.GetCursorIndex() != 0)
+  return;
+CompleteTargetIndex(request, GetDebugger());
+  }
+
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
 if (args.GetArgumentCount() == 1) {
@@ -604,6 +640,12 @@
 
   Options *GetOptions() override { return &m_option_group; }
 
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+CompleteTargetIndex(request, GetDebugger());
+  }
+
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
 const size_t argc = args.GetArgumentCount();
Index: lldb/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
@@ -409,3 +409,27 @@
 # No completion for Qu because the candidate is
 # (anonymous namespace)::Quux().
 self.complete_from_to('breakpoint set -n Qu', '')
+
+def test_target_select_and_delete(self):
+self.build()
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+self.complete_from_to('tar

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

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

This looks like a good idea to me, but given how fiddly this code seems to be, 
lets give some time for @hhb and @ZeGentzy to try this out.
The logical next step seems to be to pass this information into 
`prepare_binding_Python.py` to avoid recomputing the same thing twice. After 
that, we could try to support the cross-compile scenario by detecting these 
paths based on the target python (with the worst case being, making this a 
cache variable and having the user override in a cross-scenario).




Comment at: lldb/scripts/CMakeLists.txt:45-50
+  execute_process(
+COMMAND ${PYTHON_EXECUTABLE}
+-c "import distutils.sysconfig, sys; 
print(distutils.sysconfig.get_python_lib(True, False, sys.argv[1]))"
+${CMAKE_BINARY_DIR}
+OUTPUT_VARIABLE SWIG_PYTHON_DIR
+OUTPUT_STRIP_TRAILING_WHITESPACE)

For my own education, is it possible that the result of the `get_python_lib` 
call will fundamentally differ depending on the value of the third argument. 
I.e., is there any case where `${SWIG_PYTHON_DIR}` will be different from 
`${CMAKE_BINARY_DIR}/${SWIG_INSTALL_DIR}` ?


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Michał Górny via Phabricator via lldb-commits

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

I think it will be also nice to eliminate get_relative_lib_dir.py long term.




Comment at: lldb/scripts/CMakeLists.txt:45-50
+  execute_process(
+COMMAND ${PYTHON_EXECUTABLE}
+-c "import distutils.sysconfig, sys; 
print(distutils.sysconfig.get_python_lib(True, False, sys.argv[1]))"
+${CMAKE_BINARY_DIR}
+OUTPUT_VARIABLE SWIG_PYTHON_DIR
+OUTPUT_STRIP_TRAILING_WHITESPACE)

labath wrote:
> For my own education, is it possible that the result of the `get_python_lib` 
> call will fundamentally differ depending on the value of the third argument. 
> I.e., is there any case where `${SWIG_PYTHON_DIR}` will be different from 
> `${CMAKE_BINARY_DIR}/${SWIG_INSTALL_DIR}` ?
The former will be an absolute path while the latter is just the relative path. 
But if you mean whether they could have a different tail: I don't think so, at 
least with CPython. PyPy had some nasty logic, so I'd have to check if we ever 
decided to support that.


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [lldb] r372587 - [LLDB] Add a missing specification of linking against dbghelp

2019-09-23 Thread Martin Storsjo via lldb-commits

Author: mstorsjo
Date: Mon Sep 23 05:03:08 2019
New Revision: 372587

URL: http://llvm.org/viewvc/llvm-project?rev=372587&view=rev
Log:
[LLDB] Add a missing specification of linking against dbghelp

The PECOFF object file plugin uses the dbghelp API, but doesn't
specify that it has to be linked in anywhere.

Current MSVC based builds have probably succeeded, as other parts
in LLDB have had a "#pragma comment(lib, "dbghelp.lib")", but there's
currently no such pragma in the PECOFF plugin.

The "#pragma comment(lib, ...)" approach doesn't work in MinGW mode
(unless the compiler is given the -fms-extensions option, and even
then, it's only supported by clang/lld, not by GCC/binutils), thus
add it to be linked via CMake. (The other parts of LLDB that use
dbghelp are within _MSC_VER ifdefs.)

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

Modified:
lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt

Modified: lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt?rev=372587&r1=372586&r2=372587&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt Mon Sep 23 
05:03:08 2019
@@ -1,3 +1,10 @@
+# Dbghelp is used on windows for writing minidump files.
+if(WIN32)
+  set(DBGHELP_LINK_FILES dbghelp)
+else()
+  set(DBGHELP_LINK_FILES "")
+endif()
+
 add_lldb_library(lldbPluginObjectFilePECOFF PLUGIN
   ObjectFilePECOFF.cpp
   WindowsMiniDump.cpp
@@ -7,6 +14,7 @@ add_lldb_library(lldbPluginObjectFilePEC
 lldbHost
 lldbSymbol
 lldbTarget
+${DBGHELP_LINK_FILES}
   LINK_COMPONENTS
 BinaryFormat
 Support


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

[Lldb-commits] [lldb] r372586 - [LLDB] Use the Windows SOCKET type on all windows targets, not only MSVC

2019-09-23 Thread Martin Storsjo via lldb-commits

Author: mstorsjo
Date: Mon Sep 23 05:02:59 2019
New Revision: 372586

URL: http://llvm.org/viewvc/llvm-project?rev=372586&view=rev
Log:
[LLDB] Use the Windows SOCKET type on all windows targets, not only MSVC

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

Modified:
lldb/trunk/include/lldb/Host/Socket.h

Modified: lldb/trunk/include/lldb/Host/Socket.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/Socket.h?rev=372586&r1=372585&r2=372586&view=diff
==
--- lldb/trunk/include/lldb/Host/Socket.h (original)
+++ lldb/trunk/include/lldb/Host/Socket.h Mon Sep 23 05:02:59 2019
@@ -31,7 +31,7 @@ class StringRef;
 
 namespace lldb_private {
 
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 typedef SOCKET NativeSocket;
 #else
 typedef int NativeSocket;


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

[Lldb-commits] [lldb] r372589 - [LLDB] Avoid a warning about an unused static variable

2019-09-23 Thread Martin Storsjo via lldb-commits

Author: mstorsjo
Date: Mon Sep 23 05:03:21 2019
New Revision: 372589

URL: http://llvm.org/viewvc/llvm-project?rev=372589&view=rev
Log:
[LLDB] Avoid a warning about an unused static variable

The variable is unused on windows.

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

Modified:
lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp

Modified: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp?rev=372589&r1=372588&r2=372589&view=diff
==
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp (original)
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp Mon Sep 23 05:03:21 2019
@@ -108,10 +108,10 @@ static struct option g_long_options[] =
 {"fd", required_argument, nullptr, 'F'},
 {nullptr, 0, nullptr, 0}};
 
+#ifndef _WIN32
 // Watch for signals
 static int g_sighup_received_count = 0;
 
-#ifndef _WIN32
 static void sighup_handler(MainLoopBase &mainloop) {
   ++g_sighup_received_count;
 


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

[Lldb-commits] [lldb] r372590 - [LLDB] Add a void* cast when passing object pointers to printf %p

2019-09-23 Thread Martin Storsjo via lldb-commits

Author: mstorsjo
Date: Mon Sep 23 05:03:28 2019
New Revision: 372590

URL: http://llvm.org/viewvc/llvm-project?rev=372590&view=rev
Log:
[LLDB] Add a void* cast when passing object pointers to printf %p

This fixes build warnings in MinGW mode.

Also remove leftover if (log) {} around the log macro.

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

Modified:
lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp

Modified: lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp?rev=372590&r1=372589&r2=372590&view=diff
==
--- lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp (original)
+++ lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp Mon Sep 23 
05:03:28 2019
@@ -245,13 +245,11 @@ finish:
 
   IncrementFilePointer(return_info.GetBytes());
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
-  if (log) {
-LLDB_LOGF(log,
-  "%p ConnectionGenericFile::Read()  handle = %p, dst = %p, "
-  "dst_len = %zu) => %zu, error = %s",
-  this, m_file, dst, dst_len, return_info.GetBytes(),
-  return_info.GetError().AsCString());
-  }
+  LLDB_LOGF(log,
+"%p ConnectionGenericFile::Read()  handle = %p, dst = %p, "
+"dst_len = %zu) => %zu, error = %s",
+static_cast(this), m_file, dst, dst_len,
+return_info.GetBytes(), return_info.GetError().AsCString());
 
   return return_info.GetBytes();
 }
@@ -296,13 +294,11 @@ finish:
 
   IncrementFilePointer(return_info.GetBytes());
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
-  if (log) {
-LLDB_LOGF(log,
-  "%p ConnectionGenericFile::Write()  handle = %p, src = %p, "
-  "src_len = %zu) => %zu, error = %s",
-  this, m_file, src, src_len, return_info.GetBytes(),
-  return_info.GetError().AsCString());
-  }
+  LLDB_LOGF(log,
+"%p ConnectionGenericFile::Write()  handle = %p, src = %p, "
+"src_len = %zu) => %zu, error = %s",
+static_cast(this), m_file, src, src_len,
+return_info.GetBytes(), return_info.GetError().AsCString());
   return return_info.GetBytes();
 }
 


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

[Lldb-commits] [lldb] r372588 - [LLDB] Remove a stray semicolon. NFC.

2019-09-23 Thread Martin Storsjo via lldb-commits

Author: mstorsjo
Date: Mon Sep 23 05:03:14 2019
New Revision: 372588

URL: http://llvm.org/viewvc/llvm-project?rev=372588&view=rev
Log:
[LLDB] Remove a stray semicolon. NFC.

This fixes build warnings with at least GCC.

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

Modified: lldb/trunk/source/Utility/Scalar.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Scalar.cpp?rev=372588&r1=372587&r2=372588&view=diff
==
--- lldb/trunk/source/Utility/Scalar.cpp (original)
+++ lldb/trunk/source/Utility/Scalar.cpp Mon Sep 23 05:03:14 2019
@@ -434,7 +434,7 @@ Scalar::Type Scalar::GetBestTypeForBitSi
 if (bit_size <= 512) return Scalar::e_uint512;
   }
   return Scalar::e_void;
-};
+}
 
 void Scalar::TruncOrExtendTo(Scalar::Type type, uint16_t bits) {
   switch (type) {


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

[Lldb-commits] [lldb] r372591 - [LLDB] Remove a now redundant windows specific workaround

2019-09-23 Thread Martin Storsjo via lldb-commits

Author: mstorsjo
Date: Mon Sep 23 05:03:33 2019
New Revision: 372591

URL: http://llvm.org/viewvc/llvm-project?rev=372591&view=rev
Log:
[LLDB] Remove a now redundant windows specific workaround

vsnprintf(NULL, 0, ...) works for measuring the needed string
size on all supported Windows variants; it's supported since
at least MSVC 2015 (and LLVM requires a newer version than that),
and works on both msvcrt.dll (since at least XP) and UCRT with MinGW.

The previous use of ifdefs was wrong as well, as __MINGW64__ only is
defined for 64 bit targets, and the define without trailing
underscores is never defined automatically (neither by clang nor
by gcc).

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

Modified:
lldb/trunk/source/Host/windows/Windows.cpp

Modified: lldb/trunk/source/Host/windows/Windows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/Windows.cpp?rev=372591&r1=372590&r2=372591&view=diff
==
--- lldb/trunk/source/Host/windows/Windows.cpp (original)
+++ lldb/trunk/source/Host/windows/Windows.cpp Mon Sep 23 05:03:33 2019
@@ -48,13 +48,8 @@ int vasprintf(char **ret, const char *fm
   size_t buflen;
   va_list ap2;
 
-#if defined(_MSC_VER) || defined(__MINGW64)
-  ap2 = ap;
-  len = _vscprintf(fmt, ap2);
-#else
   va_copy(ap2, ap);
   len = vsnprintf(NULL, 0, fmt, ap2);
-#endif
 
   if (len >= 0 &&
   (buf = (char *)malloc((buflen = (size_t)(len + 1 != NULL) {


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

[Lldb-commits] [lldb] r372592 - [LLDB] Check for _WIN32 instead of _MSC_VER for code specific to windows in general

2019-09-23 Thread Martin Storsjo via lldb-commits

Author: mstorsjo
Date: Mon Sep 23 05:03:56 2019
New Revision: 372592

URL: http://llvm.org/viewvc/llvm-project?rev=372592&view=rev
Log:
[LLDB] Check for _WIN32 instead of _MSC_VER for code specific to windows in 
general

These ifdefs contain code that isn't specific to MSVC but useful for
any windows target, like MinGW.

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

Modified:
lldb/trunk/source/Core/IOHandler.cpp
lldb/trunk/source/Host/common/UDPSocket.cpp
lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/trunk/source/Utility/SelectHelper.cpp
lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Modified: lldb/trunk/source/Core/IOHandler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/IOHandler.cpp?rev=372592&r1=372591&r2=372592&view=diff
==
--- lldb/trunk/source/Core/IOHandler.cpp (original)
+++ lldb/trunk/source/Core/IOHandler.cpp Mon Sep 23 05:03:56 2019
@@ -579,7 +579,7 @@ void IOHandlerEditline::PrintAsync(Strea
   else
 #endif
   {
-#ifdef _MSC_VER
+#ifdef _WIN32
 const char *prompt = GetPrompt();
 if (prompt) {
   // Back up over previous prompt using Windows API
@@ -594,7 +594,7 @@ void IOHandlerEditline::PrintAsync(Strea
 }
 #endif
 IOHandler::PrintAsync(stream, s, len);
-#ifdef _MSC_VER
+#ifdef _WIN32
 if (prompt)
   IOHandler::PrintAsync(GetOutputStreamFile().get(), prompt,
 strlen(prompt));

Modified: lldb/trunk/source/Host/common/UDPSocket.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/UDPSocket.cpp?rev=372592&r1=372591&r2=372592&view=diff
==
--- lldb/trunk/source/Host/common/UDPSocket.cpp (original)
+++ lldb/trunk/source/Host/common/UDPSocket.cpp Mon Sep 23 05:03:56 2019
@@ -80,7 +80,7 @@ Status UDPSocket::Connect(llvm::StringRe
   &service_info_list);
   if (err != 0) {
 error.SetErrorStringWithFormat(
-#if defined(_MSC_VER) && defined(UNICODE)
+#if defined(_WIN32) && defined(UNICODE)
 "getaddrinfo(%s, %s, &hints, &info) returned error %i (%S)",
 #else
 "getaddrinfo(%s, %s, &hints, &info) returned error %i (%s)",

Modified: lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp?rev=372592&r1=372591&r2=372592&view=diff
==
--- lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp (original)
+++ lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp Mon Sep 23 
05:03:56 2019
@@ -564,7 +564,7 @@ ConnectionFileDescriptor::BytesAvailable
   select_helper.SetTimeout(*timeout);
 
 select_helper.FDSetRead(handle);
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 // select() won't accept pipes on Windows.  The entire Windows codepath
 // needs to be converted over to using WaitForMultipleObjects and event
 // HANDLEs, but for now at least this will allow ::select() to not return

Modified: lldb/trunk/source/Utility/SelectHelper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/SelectHelper.cpp?rev=372592&r1=372591&r2=372592&view=diff
==
--- lldb/trunk/source/Utility/SelectHelper.cpp (original)
+++ lldb/trunk/source/Utility/SelectHelper.cpp Mon Sep 23 05:03:56 2019
@@ -92,7 +92,7 @@ static void updateMaxFd(llvm::Optional(FD_SETSIZE));
 if (fd >= static_cast(FD_SETSIZE)) {
   error.SetErrorStringWithFormat("%i is too large for select()", fd);

Modified: lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp?rev=372592&r1=372591&r2=372592&view=diff
==
--- lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp (original)
+++ lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp Mon Sep 23 
05:03:56 2019
@@ -31,7 +31,7 @@
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/FileSpec.h"
 
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 #include "lldb/Host/windows/windows.h"
 #include 
 #endif
@@ -46,7 +46,7 @@ public:
 // Initialize and TearDown the plugin every time, so we get a brand new
 // AST every time so that modifications to the AST from each test don't
 // leak into the next test.
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 ::CoInitializeEx(nullptr, COINIT_MULTITHREADED);
 #endif
 
@@ -69,7 +69,7 @@ public:
 HostInfo::Terminate();
 FileSystem::Terminate();
 
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 ::CoUninitialize();
 #endif
   }


___
lldb-commits mailing list
lldb-commit

[Lldb-commits] [PATCH] D67859: [LLDB] Use the Windows SOCKET type on all windows targets, not only MSVC

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372586: [LLDB] Use the Windows SOCKET type on all windows 
targets, not only MSVC (authored by mstorsjo, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67859?vs=221104&id=221292#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67859

Files:
  lldb/trunk/include/lldb/Host/Socket.h


Index: lldb/trunk/include/lldb/Host/Socket.h
===
--- lldb/trunk/include/lldb/Host/Socket.h
+++ lldb/trunk/include/lldb/Host/Socket.h
@@ -31,7 +31,7 @@
 
 namespace lldb_private {
 
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 typedef SOCKET NativeSocket;
 #else
 typedef int NativeSocket;


Index: lldb/trunk/include/lldb/Host/Socket.h
===
--- lldb/trunk/include/lldb/Host/Socket.h
+++ lldb/trunk/include/lldb/Host/Socket.h
@@ -31,7 +31,7 @@
 
 namespace lldb_private {
 
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 typedef SOCKET NativeSocket;
 #else
 typedef int NativeSocket;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67895: [LLDB] Avoid a warning about an unused static variable

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372589: [LLDB] Avoid a warning about an unused static 
variable (authored by mstorsjo, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67895?vs=221226&id=221294#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67895

Files:
  lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp


Index: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
@@ -108,10 +108,10 @@
 {"fd", required_argument, nullptr, 'F'},
 {nullptr, 0, nullptr, 0}};
 
+#ifndef _WIN32
 // Watch for signals
 static int g_sighup_received_count = 0;
 
-#ifndef _WIN32
 static void sighup_handler(MainLoopBase &mainloop) {
   ++g_sighup_received_count;
 


Index: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
@@ -108,10 +108,10 @@
 {"fd", required_argument, nullptr, 'F'},
 {nullptr, 0, nullptr, 0}};
 
+#ifndef _WIN32
 // Watch for signals
 static int g_sighup_received_count = 0;
 
-#ifndef _WIN32
 static void sighup_handler(MainLoopBase &mainloop) {
   ++g_sighup_received_count;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67885: [LLDB] Add a missing specification of linking against dbghelp

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372587: [LLDB] Add a missing specification of linking 
against dbghelp (authored by mstorsjo, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67885?vs=221199&id=221293#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67885

Files:
  lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt


Index: lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt
===
--- lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt
+++ lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt
@@ -1,3 +1,10 @@
+# Dbghelp is used on windows for writing minidump files.
+if(WIN32)
+  set(DBGHELP_LINK_FILES dbghelp)
+else()
+  set(DBGHELP_LINK_FILES "")
+endif()
+
 add_lldb_library(lldbPluginObjectFilePECOFF PLUGIN
   ObjectFilePECOFF.cpp
   WindowsMiniDump.cpp
@@ -7,6 +14,7 @@
 lldbHost
 lldbSymbol
 lldbTarget
+${DBGHELP_LINK_FILES}
   LINK_COMPONENTS
 BinaryFormat
 Support


Index: lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt
===
--- lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt
+++ lldb/trunk/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt
@@ -1,3 +1,10 @@
+# Dbghelp is used on windows for writing minidump files.
+if(WIN32)
+  set(DBGHELP_LINK_FILES dbghelp)
+else()
+  set(DBGHELP_LINK_FILES "")
+endif()
+
 add_lldb_library(lldbPluginObjectFilePECOFF PLUGIN
   ObjectFilePECOFF.cpp
   WindowsMiniDump.cpp
@@ -7,6 +14,7 @@
 lldbHost
 lldbSymbol
 lldbTarget
+${DBGHELP_LINK_FILES}
   LINK_COMPONENTS
 BinaryFormat
 Support
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67861: [LLDB] Remove a now redundant windows specific workaround

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372591: [LLDB] Remove a now redundant windows specific 
workaround (authored by mstorsjo, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67861?vs=221276&id=221296#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67861

Files:
  lldb/trunk/source/Host/windows/Windows.cpp


Index: lldb/trunk/source/Host/windows/Windows.cpp
===
--- lldb/trunk/source/Host/windows/Windows.cpp
+++ lldb/trunk/source/Host/windows/Windows.cpp
@@ -48,13 +48,8 @@
   size_t buflen;
   va_list ap2;
 
-#if defined(_MSC_VER) || defined(__MINGW64)
-  ap2 = ap;
-  len = _vscprintf(fmt, ap2);
-#else
   va_copy(ap2, ap);
   len = vsnprintf(NULL, 0, fmt, ap2);
-#endif
 
   if (len >= 0 &&
   (buf = (char *)malloc((buflen = (size_t)(len + 1 != NULL) {


Index: lldb/trunk/source/Host/windows/Windows.cpp
===
--- lldb/trunk/source/Host/windows/Windows.cpp
+++ lldb/trunk/source/Host/windows/Windows.cpp
@@ -48,13 +48,8 @@
   size_t buflen;
   va_list ap2;
 
-#if defined(_MSC_VER) || defined(__MINGW64)
-  ap2 = ap;
-  len = _vscprintf(fmt, ap2);
-#else
   va_copy(ap2, ap);
   len = vsnprintf(NULL, 0, fmt, ap2);
-#endif
 
   if (len >= 0 &&
   (buf = (char *)malloc((buflen = (size_t)(len + 1 != NULL) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67896: [LLDB] Add a void* cast when passing object pointers to printf %p

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372590: [LLDB] Add a void* cast when passing object pointers 
to printf %p (authored by mstorsjo, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67896?vs=221230&id=221295#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67896

Files:
  lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp


Index: lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp
===
--- lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp
+++ lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp
@@ -245,13 +245,11 @@
 
   IncrementFilePointer(return_info.GetBytes());
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
-  if (log) {
-LLDB_LOGF(log,
-  "%p ConnectionGenericFile::Read()  handle = %p, dst = %p, "
-  "dst_len = %zu) => %zu, error = %s",
-  this, m_file, dst, dst_len, return_info.GetBytes(),
-  return_info.GetError().AsCString());
-  }
+  LLDB_LOGF(log,
+"%p ConnectionGenericFile::Read()  handle = %p, dst = %p, "
+"dst_len = %zu) => %zu, error = %s",
+static_cast(this), m_file, dst, dst_len,
+return_info.GetBytes(), return_info.GetError().AsCString());
 
   return return_info.GetBytes();
 }
@@ -296,13 +294,11 @@
 
   IncrementFilePointer(return_info.GetBytes());
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
-  if (log) {
-LLDB_LOGF(log,
-  "%p ConnectionGenericFile::Write()  handle = %p, src = %p, "
-  "src_len = %zu) => %zu, error = %s",
-  this, m_file, src, src_len, return_info.GetBytes(),
-  return_info.GetError().AsCString());
-  }
+  LLDB_LOGF(log,
+"%p ConnectionGenericFile::Write()  handle = %p, src = %p, "
+"src_len = %zu) => %zu, error = %s",
+static_cast(this), m_file, src, src_len,
+return_info.GetBytes(), return_info.GetError().AsCString());
   return return_info.GetBytes();
 }
 


Index: lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp
===
--- lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp
+++ lldb/trunk/source/Host/windows/ConnectionGenericFileWindows.cpp
@@ -245,13 +245,11 @@
 
   IncrementFilePointer(return_info.GetBytes());
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
-  if (log) {
-LLDB_LOGF(log,
-  "%p ConnectionGenericFile::Read()  handle = %p, dst = %p, "
-  "dst_len = %zu) => %zu, error = %s",
-  this, m_file, dst, dst_len, return_info.GetBytes(),
-  return_info.GetError().AsCString());
-  }
+  LLDB_LOGF(log,
+"%p ConnectionGenericFile::Read()  handle = %p, dst = %p, "
+"dst_len = %zu) => %zu, error = %s",
+static_cast(this), m_file, dst, dst_len,
+return_info.GetBytes(), return_info.GetError().AsCString());
 
   return return_info.GetBytes();
 }
@@ -296,13 +294,11 @@
 
   IncrementFilePointer(return_info.GetBytes());
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
-  if (log) {
-LLDB_LOGF(log,
-  "%p ConnectionGenericFile::Write()  handle = %p, src = %p, "
-  "src_len = %zu) => %zu, error = %s",
-  this, m_file, src, src_len, return_info.GetBytes(),
-  return_info.GetError().AsCString());
-  }
+  LLDB_LOGF(log,
+"%p ConnectionGenericFile::Write()  handle = %p, src = %p, "
+"src_len = %zu) => %zu, error = %s",
+static_cast(this), m_file, src, src_len,
+return_info.GetBytes(), return_info.GetError().AsCString());
   return return_info.GetBytes();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67893: [LLDB] Check for _WIN32 instead of _MSC_VER for code specific to windows in general

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372592: [LLDB] Check for _WIN32 instead of _MSC_VER for code 
specific to windows in… (authored by mstorsjo, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67893?vs=221224&id=221297#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67893

Files:
  lldb/trunk/source/Core/IOHandler.cpp
  lldb/trunk/source/Host/common/UDPSocket.cpp
  lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/trunk/source/Utility/SelectHelper.cpp
  lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Index: lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -31,7 +31,7 @@
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/FileSpec.h"
 
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 #include "lldb/Host/windows/windows.h"
 #include 
 #endif
@@ -46,7 +46,7 @@
 // Initialize and TearDown the plugin every time, so we get a brand new
 // AST every time so that modifications to the AST from each test don't
 // leak into the next test.
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 ::CoInitializeEx(nullptr, COINIT_MULTITHREADED);
 #endif
 
@@ -69,7 +69,7 @@
 HostInfo::Terminate();
 FileSystem::Terminate();
 
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 ::CoUninitialize();
 #endif
   }
Index: lldb/trunk/source/Host/common/UDPSocket.cpp
===
--- lldb/trunk/source/Host/common/UDPSocket.cpp
+++ lldb/trunk/source/Host/common/UDPSocket.cpp
@@ -80,7 +80,7 @@
   &service_info_list);
   if (err != 0) {
 error.SetErrorStringWithFormat(
-#if defined(_MSC_VER) && defined(UNICODE)
+#if defined(_WIN32) && defined(UNICODE)
 "getaddrinfo(%s, %s, &hints, &info) returned error %i (%S)",
 #else
 "getaddrinfo(%s, %s, &hints, &info) returned error %i (%s)",
Index: lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -564,7 +564,7 @@
   select_helper.SetTimeout(*timeout);
 
 select_helper.FDSetRead(handle);
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 // select() won't accept pipes on Windows.  The entire Windows codepath
 // needs to be converted over to using WaitForMultipleObjects and event
 // HANDLEs, but for now at least this will allow ::select() to not return
Index: lldb/trunk/source/Core/IOHandler.cpp
===
--- lldb/trunk/source/Core/IOHandler.cpp
+++ lldb/trunk/source/Core/IOHandler.cpp
@@ -579,7 +579,7 @@
   else
 #endif
   {
-#ifdef _MSC_VER
+#ifdef _WIN32
 const char *prompt = GetPrompt();
 if (prompt) {
   // Back up over previous prompt using Windows API
@@ -594,7 +594,7 @@
 }
 #endif
 IOHandler::PrintAsync(stream, s, len);
-#ifdef _MSC_VER
+#ifdef _WIN32
 if (prompt)
   IOHandler::PrintAsync(GetOutputStreamFile().get(), prompt,
 strlen(prompt));
Index: lldb/trunk/source/Utility/SelectHelper.cpp
===
--- lldb/trunk/source/Utility/SelectHelper.cpp
+++ lldb/trunk/source/Utility/SelectHelper.cpp
@@ -92,7 +92,7 @@
 
 lldb_private::Status SelectHelper::Select() {
   lldb_private::Status error;
-#ifdef _MSC_VER
+#ifdef _WIN32
   // On windows FD_SETSIZE limits the number of file descriptors, not their
   // numeric value.
   lldbassert(m_fd_map.size() <= FD_SETSIZE);
@@ -107,7 +107,7 @@
   for (auto &pair : m_fd_map) {
 pair.second.PrepareForSelect();
 const lldb::socket_t fd = pair.first;
-#if !defined(__APPLE__) && !defined(_MSC_VER)
+#if !defined(__APPLE__) && !defined(_WIN32)
 lldbassert(fd < static_cast(FD_SETSIZE));
 if (fd >= static_cast(FD_SETSIZE)) {
   error.SetErrorStringWithFormat("%i is too large for select()", fd);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67792: File::SetDescriptor() should require options

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

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

This looks much better to me, but maybe you could add some unit tests for the 
use cases this fixes (I guess that's the use case of creating a `File` object 
with an fd, and the writing(?) to it).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67792



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

[Lldb-commits] [PATCH] D67910: [LLDB] Avoid warnings about redefining posix mode defines on MinGW

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo created this revision.
mstorsjo added reviewers: hhb, amccarth, labath.
Herald added subscribers: JDevlieghere, abidh.
Herald added a project: LLDB.

Since these defines were added in LLVM SVN r189364 in 2013, mingw-w64 got 
defines for S_I?GRP, S_IRWXG, S_I?OTH and S_IRWXO in 2015.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67910

Files:
  lldb/include/lldb/Host/windows/PosixApi.h


Index: lldb/include/lldb/Host/windows/PosixApi.h
===
--- lldb/include/lldb/Host/windows/PosixApi.h
+++ lldb/include/lldb/Host/windows/PosixApi.h
@@ -36,15 +36,25 @@
 #define S_IWUSR S_IWRITE /* write, user */
 #define S_IXUSR 0/* execute, user */
 #endif
+#ifndef S_IRGRP
 #define S_IRGRP 0 /* read, group */
 #define S_IWGRP 0 /* write, group */
 #define S_IXGRP 0 /* execute, group */
+#endif
+#ifndef S_IROTH
 #define S_IROTH 0 /* read, others */
 #define S_IWOTH 0 /* write, others */
 #define S_IXOTH 0 /* execute, others */
+#endif
+#ifndef S_IRWXU
 #define S_IRWXU 0
+#endif
+#ifndef S_IRWXG
 #define S_IRWXG 0
+#endif
+#ifndef S_IRWXO
 #define S_IRWXO 0
+#endif
 
 #if HAVE_SYS_TYPES_H
 // pyconfig.h typedefs this.  We require python headers to be included before


Index: lldb/include/lldb/Host/windows/PosixApi.h
===
--- lldb/include/lldb/Host/windows/PosixApi.h
+++ lldb/include/lldb/Host/windows/PosixApi.h
@@ -36,15 +36,25 @@
 #define S_IWUSR S_IWRITE /* write, user */
 #define S_IXUSR 0/* execute, user */
 #endif
+#ifndef S_IRGRP
 #define S_IRGRP 0 /* read, group */
 #define S_IWGRP 0 /* write, group */
 #define S_IXGRP 0 /* execute, group */
+#endif
+#ifndef S_IROTH
 #define S_IROTH 0 /* read, others */
 #define S_IWOTH 0 /* write, others */
 #define S_IXOTH 0 /* execute, others */
+#endif
+#ifndef S_IRWXU
 #define S_IRWXU 0
+#endif
+#ifndef S_IRWXG
 #define S_IRWXG 0
+#endif
+#ifndef S_IRWXO
 #define S_IRWXO 0
+#endif
 
 #if HAVE_SYS_TYPES_H
 // pyconfig.h typedefs this.  We require python headers to be included before
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67911: [LLDB] [Windows] Add missing ifdefs to fix building for non-x86 architectures

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo created this revision.
mstorsjo added reviewers: amccarth, labath, hhb, compnerd, asmith.
Herald added subscribers: JDevlieghere, abidh.
Herald added a project: LLDB.

While debugging on those architectures might not be supported yet, the generic 
code should still be buildable. This file accesses x86 specific fields in the 
CONTEXT struct.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67911

Files:
  lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp


Index: lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
@@ -95,6 +95,7 @@
   if (!CacheAllRegisterValues())
 return false;
 
+#if defined(__i386__) || defined(_M_IX86) || defined(__x86_64__) || 
defined(_M_AMD64)
   unsigned shift = 2 * slot;
   m_context.Dr7 |= 1ULL << shift;
 
@@ -109,6 +110,10 @@
   m_context.Dr7 |= (read ? 3ULL : (write ? 1ULL : 0)) << shift;
 
   return ApplyAllRegisterValues();
+
+#else
+  return false;
+#endif
 }
 
 bool RegisterContextWindows::RemoveHardwareBreakpoint(uint32_t slot) {
@@ -118,19 +123,25 @@
   if (!CacheAllRegisterValues())
 return false;
 
+#if defined(__i386__) || defined(_M_IX86) || defined(__x86_64__) || 
defined(_M_AMD64)
   unsigned shift = 2 * slot;
   m_context.Dr7 &= ~(1ULL << shift);
 
   return ApplyAllRegisterValues();
+#else
+  return false;
+#endif
 }
 
 uint32_t RegisterContextWindows::GetTriggeredHardwareBreakpointSlotId() {
   if (!CacheAllRegisterValues())
 return LLDB_INVALID_INDEX32;
 
+#if defined(__i386__) || defined(_M_IX86) || defined(__x86_64__) || 
defined(_M_AMD64)
   for (unsigned i = 0UL; i < NUM_HARDWARE_BREAKPOINT_SLOTS; i++)
 if (m_context.Dr6 & (1ULL << i))
   return i;
+#endif
 
   return LLDB_INVALID_INDEX32;
 }


Index: lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp
@@ -95,6 +95,7 @@
   if (!CacheAllRegisterValues())
 return false;
 
+#if defined(__i386__) || defined(_M_IX86) || defined(__x86_64__) || defined(_M_AMD64)
   unsigned shift = 2 * slot;
   m_context.Dr7 |= 1ULL << shift;
 
@@ -109,6 +110,10 @@
   m_context.Dr7 |= (read ? 3ULL : (write ? 1ULL : 0)) << shift;
 
   return ApplyAllRegisterValues();
+
+#else
+  return false;
+#endif
 }
 
 bool RegisterContextWindows::RemoveHardwareBreakpoint(uint32_t slot) {
@@ -118,19 +123,25 @@
   if (!CacheAllRegisterValues())
 return false;
 
+#if defined(__i386__) || defined(_M_IX86) || defined(__x86_64__) || defined(_M_AMD64)
   unsigned shift = 2 * slot;
   m_context.Dr7 &= ~(1ULL << shift);
 
   return ApplyAllRegisterValues();
+#else
+  return false;
+#endif
 }
 
 uint32_t RegisterContextWindows::GetTriggeredHardwareBreakpointSlotId() {
   if (!CacheAllRegisterValues())
 return LLDB_INVALID_INDEX32;
 
+#if defined(__i386__) || defined(_M_IX86) || defined(__x86_64__) || defined(_M_AMD64)
   for (unsigned i = 0UL; i < NUM_HARDWARE_BREAKPOINT_SLOTS; i++)
 if (m_context.Dr6 & (1ULL << i))
   return i;
+#endif
 
   return LLDB_INVALID_INDEX32;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67912: [LLDB] [PECOFF] Recognize arm64 executables

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo created this revision.
mstorsjo added reviewers: amccarth, compnerd, hhb, labath, asmith.
Herald added subscribers: JDevlieghere, kristof.beyls.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67912

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -198,6 +198,10 @@
 spec.SetTriple("arm-pc-windows");
 specs.Append(module_spec);
 break;
+  case MachineArm64:
+spec.SetTriple("aarch64-pc-windows");
+specs.Append(module_spec);
+break;
   default:
 break;
   }
@@ -1200,6 +1204,7 @@
   case llvm::COFF::IMAGE_FILE_MACHINE_ARM:
   case llvm::COFF::IMAGE_FILE_MACHINE_ARMNT:
   case llvm::COFF::IMAGE_FILE_MACHINE_THUMB:
+  case llvm::COFF::IMAGE_FILE_MACHINE_ARM64:
 ArchSpec arch;
 arch.SetArchitecture(eArchTypeCOFF, machine, LLDB_INVALID_CPUTYPE,
  IsWindowsSubsystem() ? llvm::Triple::Win32


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -198,6 +198,10 @@
 spec.SetTriple("arm-pc-windows");
 specs.Append(module_spec);
 break;
+  case MachineArm64:
+spec.SetTriple("aarch64-pc-windows");
+specs.Append(module_spec);
+break;
   default:
 break;
   }
@@ -1200,6 +1204,7 @@
   case llvm::COFF::IMAGE_FILE_MACHINE_ARM:
   case llvm::COFF::IMAGE_FILE_MACHINE_ARMNT:
   case llvm::COFF::IMAGE_FILE_MACHINE_THUMB:
+  case llvm::COFF::IMAGE_FILE_MACHINE_ARM64:
 ArchSpec arch;
 arch.SetArchitecture(eArchTypeCOFF, machine, LLDB_INVALID_CPUTYPE,
  IsWindowsSubsystem() ? llvm::Triple::Win32
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67913: [LLDB] [Windows] Map COFF ARM machine ids to the right triplet architectures

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo created this revision.
mstorsjo added reviewers: amccarth, compnerd, hhb, labath, asmith.
Herald added subscribers: JDevlieghere, kristof.beyls.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D67913

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


Index: lldb/source/Host/windows/Host.cpp
===
--- lldb/source/Host/windows/Host.cpp
+++ lldb/source/Host/windows/Host.cpp
@@ -56,6 +56,10 @@
 triple.setArch(llvm::Triple::x86_64);
   else if (machineType == 0x14c)
 triple.setArch(llvm::Triple::x86);
+  else if (machineType == 0x1c4)
+triple.setArch(llvm::Triple::arm);
+  else if (machineType == 0xaa64)
+triple.setArch(llvm::Triple::aarch64);
 
   return true;
 }


Index: lldb/source/Host/windows/Host.cpp
===
--- lldb/source/Host/windows/Host.cpp
+++ lldb/source/Host/windows/Host.cpp
@@ -56,6 +56,10 @@
 triple.setArch(llvm::Triple::x86_64);
   else if (machineType == 0x14c)
 triple.setArch(llvm::Triple::x86);
+  else if (machineType == 0x1c4)
+triple.setArch(llvm::Triple::arm);
+  else if (machineType == 0xaa64)
+triple.setArch(llvm::Triple::aarch64);
 
   return true;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67760: [lldb] Decouple importing the std C++ module from the way the program is compiled

2019-09-23 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor updated this revision to Diff 221307.
teemperor added a comment.

- Addressed feedback.


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

https://reviews.llvm.org/D67760

Files:
  lldb/include/lldb/Symbol/CompileUnit.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Target/Platform.h
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/basic/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/basic/TestImportStdModule.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/conflicts/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/conflicts/TestStdModuleWithConflicts.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/deque-basic/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/deque-basic/TestBasicDeque.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/deque-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDeque.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/forward_list-basic/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/forward_list-basic/TestBasicForwardList.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/forward_list-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardList.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/list-basic/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/list-basic/TestBasicList.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/list-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentList.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/no-std-module/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/no-std-module/TestMissingStdModule.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/queue/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/queue/TestQueue.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/shared_ptr-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/shared_ptr-dbg-info-content/TestSharedPtrDbgInfoContent.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/shared_ptr/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/shared_ptr/TestSharedPtr.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/stack/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/stack/TestStack.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/sysroot/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/sysroot/TestStdModuleSysroot.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/unique_ptr-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/unique_ptr/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/unique_ptr/TestUniquePtr.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-basic/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-basic/TestBasicVector.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-bool/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-bool/TestBoolVector.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVector.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-of-vectors/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectors.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/weak_ptr-dbg-info-content/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtr.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/weak_ptr/Make

[Lldb-commits] [PATCH] D67760: [lldb] Decouple importing the std C++ module from the way the program is compiled

2019-09-23 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor marked 6 inline comments as done.
teemperor added a comment.

Thanks for the quick review!




Comment at: lldb/include/lldb/Symbol/CompileUnit.h:228
 
+  /// Apply a lambda to each external module referenced by this compilation
+  /// unit. Recursively also descends into the referenced external modules

aprantl wrote:
> What does "external module" mean in this context? Can we be specific about 
> lldb::Module vs clang::Module?
Rather lldb::Module. Well, it refers to the external type modules we get with 
-gmodules (i.e. what we store in SymbolFileDWRF::m_external_type_modules), so 
it is kind of both in this case.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:46
+  // we should remove that '/bits' suffix to get the actual include directory.
+  if (dir.endswith("/usr/include/bits"))
+dir.consume_back("/bits");

aprantl wrote:
> llvm::sys::path::append to avoid hardcoding path separators?
I would rather have this converted to POSIX style and work with that. Otherwise 
this whole function needs to be path-sep independent (including escaping the 
regex and what not). And that ends up being a lot of boilerplate to make this 
OS-specific code OS-agnostic.


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

https://reviews.llvm.org/D67760



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

[Lldb-commits] [PATCH] D67891: remove File::SetStream(), make new files instead.

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

While I am fully in favour of removing the SetXXX methods, and it does seem 
like some amount of shared_ptrs will be needed at the (though I don't fully 
understand your vission yet), I don't really like this proliferation of 
shared_ptrs everywhere (there's way too many of shared_ptrs in lldb already). 
I'd like it to be possible to use these APIs in the simplest cases without any 
shared_ptrs involved. For instance, instead of `Status FileSystem::Open(FileSP 
&, FileSpec, uint32_t, uint32_t, bool)`, we could have `Expected 
FileSystem::Open(FileSpec, uint32_t, uint32_t, bool)` or 
`Expected> FileSystem::Open(FileSpec, uint32_t, uint32_t, 
bool)`. The first option would require making the File class movable (so you 
could write `file = File(...)` instead of `file.SetXXX(...)`, while the second 
option would enable fully immutable File objects. Given that we already have 
`File::Close` (and ergo, a natural representation for the moved-from state), 
I'd try to go for the first option.

Could we make changes like that first, and later decide how much shared_ptring 
needs to be done? One can always convert a non-shared pointer into a shared 
one, but the other direction is more tricky. In the mean time, I'll respond to 
the lldb-dev thread you started (thanks for doing that!) to get a better idea 
of what you're trying to do here.




Comment at: lldb/include/lldb/Core/Debugger.h:123-131
+  lldb_private::File &GetInputFile() { return *m_input_file_sp; }
+
+  lldb_private::File &GetOutputFile() { return m_output_stream_sp->GetFile(); }
+
+  lldb_private::File &GetErrorFile() { return m_error_stream_sp->GetFile(); }
+
+  lldb_private::StreamFile &GetOutputStream() { return *m_output_stream_sp; }

The lldb_private qualification is superfluous here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67891



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

[Lldb-commits] [PATCH] D67793: new api class: SBFile

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D67793#1678259 , @lawrence_danna 
wrote:

> @labath
>
> I wrote a patch for the shared_ptr approach.   It's simple, but it touches a 
> lot of lines.
>
> https://reviews.llvm.org/D67891
>
> Now that I've gone and done it, I kind of like it better that way.   If you 
> approve the other patch, I'll update this one accordingly.

Thanks for the quick turnaround. /I think/ the approach in the other patch 
looks better (I am super happy about the removal of the SetXXX methods), but 
I'd still like to understand whether we need to introduce that many 
shared_ptrs, so I'll need to think about this some more (I just got back from 
vacation and I am still working through my backlog).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793

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

[Lldb-commits] [PATCH] D67866: [lldb] Unify python site-packages path

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

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

Like I said in the other patch, I think we're stuck with having to match how 
the python layouts differ between operating systems (though there is definitely 
a lot of room for improvement in the way that it is done now).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67866



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/scripts/CMakeLists.txt:45-50
+  execute_process(
+COMMAND ${PYTHON_EXECUTABLE}
+-c "import distutils.sysconfig, sys; 
print(distutils.sysconfig.get_python_lib(True, False, sys.argv[1]))"
+${CMAKE_BINARY_DIR}
+OUTPUT_VARIABLE SWIG_PYTHON_DIR
+OUTPUT_STRIP_TRAILING_WHITESPACE)

mgorny wrote:
> labath wrote:
> > For my own education, is it possible that the result of the 
> > `get_python_lib` call will fundamentally differ depending on the value of 
> > the third argument. I.e., is there any case where `${SWIG_PYTHON_DIR}` will 
> > be different from `${CMAKE_BINARY_DIR}/${SWIG_INSTALL_DIR}` ?
> The former will be an absolute path while the latter is just the relative 
> path. But if you mean whether they could have a different tail: I don't think 
> so, at least with CPython. PyPy had some nasty logic, so I'd have to check if 
> we ever decided to support that.
Right now that doesn't matter, but I am thinking ahead about the 
cross-compilation case. If we turn out to need a cache variable for this, it 
would be nice if it would be a single variable that the user could set (instead 
of two of them). For that to work, we'd need to be able to compute the result 
of one of these calls using the value of the other one.


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Michał Górny via Phabricator via lldb-commits

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



Comment at: lldb/scripts/CMakeLists.txt:45-50
+  execute_process(
+COMMAND ${PYTHON_EXECUTABLE}
+-c "import distutils.sysconfig, sys; 
print(distutils.sysconfig.get_python_lib(True, False, sys.argv[1]))"
+${CMAKE_BINARY_DIR}
+OUTPUT_VARIABLE SWIG_PYTHON_DIR
+OUTPUT_STRIP_TRAILING_WHITESPACE)

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > For my own education, is it possible that the result of the 
> > > `get_python_lib` call will fundamentally differ depending on the value of 
> > > the third argument. I.e., is there any case where `${SWIG_PYTHON_DIR}` 
> > > will be different from `${CMAKE_BINARY_DIR}/${SWIG_INSTALL_DIR}` ?
> > The former will be an absolute path while the latter is just the relative 
> > path. But if you mean whether they could have a different tail: I don't 
> > think so, at least with CPython. PyPy had some nasty logic, so I'd have to 
> > check if we ever decided to support that.
> Right now that doesn't matter, but I am thinking ahead about the 
> cross-compilation case. If we turn out to need a cache variable for this, it 
> would be nice if it would be a single variable that the user could set 
> (instead of two of them). For that to work, we'd need to be able to compute 
> the result of one of these calls using the value of the other one.
I suppose you mean having a variable specifying path relative to prefix/build 
dir, I presume? I suppose we could build the whole thing around a cache var 
defaulting to the sysconfig call as proposed here for `SWIG_INSTALL_DIR` (note 
it's passing empty prefix to `get_python_lib()` in order to get relative path).

Thinking about it, maybe it'd indeed be better for me to prepare a more 
complete patch built around that, and focus on testing that instead. I was 
mostly worried/confused by Linux-specific code hanging around.


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67915: [LLDB] Fix logically dead code

2019-09-23 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

Personally I would check GIT history for some reasons it was written this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67915



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

[Lldb-commits] [PATCH] D67915: [LLDB] Fix logically dead code

2019-09-23 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

kwk created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
kwk added a reviewer: jankratochvil.
kwk edited the summary of this revision.
jankratochvil added a comment.

Personally I would check GIT history for some reasons it was written this way.


The indicated dead code may have performed some action; that action will never 
occur.

In lldb_private::LoadedModuleInfoList::LoadedModuleInfo::operator 
==(lldb_private::LoadedModuleInfoList::LoadedModuleInfo const &): Code can 
never be reached because of a logical contradiction (CWE-561)

Coverity Scan: 
https://scan3.coverity.com/reports.htm#v39507/p12195/fileInstanceId=27614986&defectInstanceId=8037484&mergedDefectId=221581

CID 221581


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67915

Files:
  lldb/include/lldb/Core/LoadedModuleInfoList.h


Index: lldb/include/lldb/Core/LoadedModuleInfoList.h
===
--- lldb/include/lldb/Core/LoadedModuleInfoList.h
+++ lldb/include/lldb/Core/LoadedModuleInfoList.h
@@ -84,9 +84,6 @@
 }
 
 bool operator==(LoadedModuleInfo const &rhs) const {
-  if (e_num != rhs.e_num)
-return false;
-
   for (size_t i = 0; i < e_num; ++i) {
 if (m_has[i] != rhs.m_has[i])
   return false;


Index: lldb/include/lldb/Core/LoadedModuleInfoList.h
===
--- lldb/include/lldb/Core/LoadedModuleInfoList.h
+++ lldb/include/lldb/Core/LoadedModuleInfoList.h
@@ -84,9 +84,6 @@
 }
 
 bool operator==(LoadedModuleInfo const &rhs) const {
-  if (e_num != rhs.e_num)
-return false;
-
   for (size_t i = 0; i < e_num; ++i) {
 if (m_has[i] != rhs.m_has[i])
   return false;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67915: [LLDB] Fix logically dead code

2019-09-23 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

FYI the link does not work for me: 
https://scan3.coverity.com/reports.htm#v39507/p12195/fileInstanceId=27614986&defectInstanceId=8037484&mergedDefectId=221581


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67915



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/scripts/CMakeLists.txt:45-50
+  execute_process(
+COMMAND ${PYTHON_EXECUTABLE}
+-c "import distutils.sysconfig, sys; 
print(distutils.sysconfig.get_python_lib(True, False, sys.argv[1]))"
+${CMAKE_BINARY_DIR}
+OUTPUT_VARIABLE SWIG_PYTHON_DIR
+OUTPUT_STRIP_TRAILING_WHITESPACE)

mgorny wrote:
> labath wrote:
> > mgorny wrote:
> > > labath wrote:
> > > > For my own education, is it possible that the result of the 
> > > > `get_python_lib` call will fundamentally differ depending on the value 
> > > > of the third argument. I.e., is there any case where 
> > > > `${SWIG_PYTHON_DIR}` will be different from 
> > > > `${CMAKE_BINARY_DIR}/${SWIG_INSTALL_DIR}` ?
> > > The former will be an absolute path while the latter is just the relative 
> > > path. But if you mean whether they could have a different tail: I don't 
> > > think so, at least with CPython. PyPy had some nasty logic, so I'd have 
> > > to check if we ever decided to support that.
> > Right now that doesn't matter, but I am thinking ahead about the 
> > cross-compilation case. If we turn out to need a cache variable for this, 
> > it would be nice if it would be a single variable that the user could set 
> > (instead of two of them). For that to work, we'd need to be able to compute 
> > the result of one of these calls using the value of the other one.
> I suppose you mean having a variable specifying path relative to prefix/build 
> dir, I presume? I suppose we could build the whole thing around a cache var 
> defaulting to the sysconfig call as proposed here for `SWIG_INSTALL_DIR` 
> (note it's passing empty prefix to `get_python_lib()` in order to get 
> relative path).
> 
> Thinking about it, maybe it'd indeed be better for me to prepare a more 
> complete patch built around that, and focus on testing that instead. I was 
> mostly worried/confused by Linux-specific code hanging around.
> I suppose you mean having a variable specifying path relative to prefix/build 
> dir, I presume?
Probably something like that, though I am still kind of hoping that there will 
be some way to detect this from cmake. However, that is looking less and less 
likely the more time I spend looking for a way to do it.

> Thinking about it, maybe it'd indeed be better for me to prepare a more 
> complete patch built around that, and focus on testing that instead.
That could work too, though I think the current version is an improvement in 
itself, and I agree we should do this slowly.


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67915: [LLDB] Fix logically dead code

2019-09-23 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

kwk added a comment.

@jankratochvil the file only has two revisions: 
https://github.com/llvm/llvm-project/commits/b9c1b51e45b845debb76d8658edabca70ca56079/lldb/include/lldb/Core/LoadedModuleInfoList.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67915



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

[Lldb-commits] [lldb] r372608 - [LLDB] Fix logically dead code

2019-09-23 Thread Konrad Kleine via lldb-commits

Author: kwk
Date: Mon Sep 23 07:05:51 2019
New Revision: 372608

URL: http://llvm.org/viewvc/llvm-project?rev=372608&view=rev
Log:
[LLDB] Fix logically dead code

Summary:
The indicated dead code may have performed some action; that action will never 
occur.

In lldb_private::LoadedModuleInfoList::LoadedModuleInfo::operator 
==(lldb_private::LoadedModuleInfoList::LoadedModuleInfo const &): Code can 
never be reached because of a logical contradiction (CWE-561)

Coverity Scan: https://scan.coverity.com/projects/kwk-llvm-project?tab=overview

CID 221581

Subscribers: lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h

Modified: lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h?rev=372608&r1=372607&r2=372608&view=diff
==
--- lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h (original)
+++ lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h Mon Sep 23 07:05:51 2019
@@ -84,9 +84,6 @@ public:
 }
 
 bool operator==(LoadedModuleInfo const &rhs) const {
-  if (e_num != rhs.e_num)
-return false;
-
   for (size_t i = 0; i < e_num; ++i) {
 if (m_has[i] != rhs.m_has[i])
   return false;


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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Michał Górny via Phabricator via lldb-commits

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



Comment at: lldb/scripts/CMakeLists.txt:45-50
+  execute_process(
+COMMAND ${PYTHON_EXECUTABLE}
+-c "import distutils.sysconfig, sys; 
print(distutils.sysconfig.get_python_lib(True, False, sys.argv[1]))"
+${CMAKE_BINARY_DIR}
+OUTPUT_VARIABLE SWIG_PYTHON_DIR
+OUTPUT_STRIP_TRAILING_WHITESPACE)

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > mgorny wrote:
> > > > labath wrote:
> > > > > For my own education, is it possible that the result of the 
> > > > > `get_python_lib` call will fundamentally differ depending on the 
> > > > > value of the third argument. I.e., is there any case where 
> > > > > `${SWIG_PYTHON_DIR}` will be different from 
> > > > > `${CMAKE_BINARY_DIR}/${SWIG_INSTALL_DIR}` ?
> > > > The former will be an absolute path while the latter is just the 
> > > > relative path. But if you mean whether they could have a different 
> > > > tail: I don't think so, at least with CPython. PyPy had some nasty 
> > > > logic, so I'd have to check if we ever decided to support that.
> > > Right now that doesn't matter, but I am thinking ahead about the 
> > > cross-compilation case. If we turn out to need a cache variable for this, 
> > > it would be nice if it would be a single variable that the user could set 
> > > (instead of two of them). For that to work, we'd need to be able to 
> > > compute the result of one of these calls using the value of the other one.
> > I suppose you mean having a variable specifying path relative to 
> > prefix/build dir, I presume? I suppose we could build the whole thing 
> > around a cache var defaulting to the sysconfig call as proposed here for 
> > `SWIG_INSTALL_DIR` (note it's passing empty prefix to `get_python_lib()` in 
> > order to get relative path).
> > 
> > Thinking about it, maybe it'd indeed be better for me to prepare a more 
> > complete patch built around that, and focus on testing that instead. I was 
> > mostly worried/confused by Linux-specific code hanging around.
> > I suppose you mean having a variable specifying path relative to 
> > prefix/build dir, I presume?
> Probably something like that, though I am still kind of hoping that there 
> will be some way to detect this from cmake. However, that is looking less and 
> less likely the more time I spend looking for a way to do it.
> 
> > Thinking about it, maybe it'd indeed be better for me to prepare a more 
> > complete patch built around that, and focus on testing that instead.
> That could work too, though I think the current version is an improvement in 
> itself, and I agree we should do this slowly.
I've grepped the `FindPython` modules and couldn't think anything useful. I 
suppose the external call to Python is as close as we can get. This is roughly 
what autotools is doing, except they go for full path and I tried to use 
relative path. The former is probably more correct (as Python won't look for 
the module in non-standard `CMAKE_INSTALL_PREFIX`) but I went for preserving 
existing behavior.


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67915: [LLDB] Fix logically dead code

2019-09-23 Thread Jan Kratochvil via Phabricator via lldb-commits

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

In D67915#1679030 , @kwk wrote:

> @jankratochvil the file only has two revisions: 
> https://github.com/llvm/llvm-project/commits/b9c1b51e45b845debb76d8658edabca70ca56079/lldb/include/lldb/Core/LoadedModuleInfoList.h

I expected some archeology like: D16004  added 
`operator==()` where `e_num` was already a constant. `e_num` was added in D9471 
 and it was also a constant there.
So yes, the fix is OK.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67915

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

[Lldb-commits] [PATCH] D67915: [LLDB] Fix logically dead code

2019-09-23 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372608: [LLDB] Fix logically dead code (authored by kwk, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67915?vs=221318&id=221323#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67915

Files:
  lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h


Index: lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h
===
--- lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h
+++ lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h
@@ -84,9 +84,6 @@
 }
 
 bool operator==(LoadedModuleInfo const &rhs) const {
-  if (e_num != rhs.e_num)
-return false;
-
   for (size_t i = 0; i < e_num; ++i) {
 if (m_has[i] != rhs.m_has[i])
   return false;


Index: lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h
===
--- lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h
+++ lldb/trunk/include/lldb/Core/LoadedModuleInfoList.h
@@ -84,9 +84,6 @@
 }
 
 bool operator==(LoadedModuleInfo const &rhs) const {
-  if (e_num != rhs.e_num)
-return false;
-
   for (size_t i = 0; i < e_num; ++i) {
 if (m_has[i] != rhs.m_has[i])
   return false;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D66398: 2/2: Fix `TestDataFormatterStdList` regression

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D66398#1677414 , @jankratochvil 
wrote:

> "jankratochvil added a reverting change" - that is not true, Differential got 
> somehow confused by the commit text there talking about this patch.


Good to know, but assuming that the above patch sticks, the plan *is* to revert 
this, right?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66398



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

[Lldb-commits] [PATCH] D66398: 2/2: Fix `TestDataFormatterStdList` regression

2019-09-23 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

In D66398#1679071 , @labath wrote:

> Good to know, but assuming that the above patch sticks, the plan *is* to 
> revert this, right?


D66654  is already checked-in, I have 
considered it as done as is. If you want to revert this patch then:

- One needs to reverse the registration order as a simple revert fails for 
`TestDataFormatterStdList.py`
- One should put a big fat warning there.

Should I do that?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66398



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

[Lldb-commits] [PATCH] D66638: Unwind: Add a stack scanning mechanism to support win32 unwinding

2019-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Thanks for the review and sorry for the delay (I was OOO). The idea to use 
`Process::GetLoadAddressPermissions` makes sense, both from consistency and 
correctness perspectives. Unfortunately it does not work "out of the box" 
because minidump core files (my primary use case for this) do not currently 
provide enough information through the memory region info api. It tries pretty 
hard to achieve that, but in the case of regular (not "full memory dumps") 
windows minidump, all we have is the "MemoryList" stream, which only contains a 
couple of memory regions, and it does not include the areas covered by loaded 
modules (which is where PCs should point). If this is to work, we'd need to 
extend this memory parsing code to also take into account the module memory 
ranges (from the ModuleList stream).

@clayborg, you're the one who wrote the memory region parsing code IIRC. Does 
that sound OK to you ?

The disassembling idea also sounds interesting, but I am afraid it's going to 
be very useful for my main use case. The main use case for breakpad symbol 
files is for the cases where one does not have the original object file (and 
when one has the original object file, it probably also has proper debug and 
unwind info). It _might_ be more interesting once we get around to PECOFF 
unwinding, as it also uses raSearch (on i386), and one is more likely to have 
the original file to disassemble there, but even then, I'd first check what 
other windows debuggers do, as might be best to just follow their lead. So, for 
now, I'd like to skip the disassembling logic.


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

https://reviews.llvm.org/D66638



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

[Lldb-commits] [PATCH] D67912: [LLDB] [PECOFF] Recognize arm64 executables

2019-09-23 Thread Saleem Abdulrasool via Phabricator via lldb-commits

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



Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:202
+  case MachineArm64:
+spec.SetTriple("aarch64-pc-windows");
+specs.Append(module_spec);

Please use `aarch64-unknown-windows`.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67912



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

[Lldb-commits] [PATCH] D67911: [LLDB] [Windows] Add missing ifdefs to fix building for non-x86 architectures

2019-09-23 Thread Saleem Abdulrasool via Phabricator via lldb-commits

compnerd added a comment.

What do you think of adding some sort of notification that hardware breakpoints 
are currently unsupported and that we are falling back to software breakpoints 
for the `#else` case?




Comment at: 
lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp:87
   case 4:
 #if defined(__x86_64__) || defined(_M_AMD64)
   case 8:

Should this line not also include `|| defined(__aarch64__) || 
defined(_M_ARM64)`?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67911



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

[Lldb-commits] [PATCH] D67913: [LLDB] [Windows] Map COFF ARM machine ids to the right triplet architectures

2019-09-23 Thread Saleem Abdulrasool via Phabricator via lldb-commits

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

Nit: `triple`, not `triplet`.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67913



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

[Lldb-commits] [PATCH] D65942: Disallow implicit conversion from pointers to bool in llvm::toStringRef

2019-09-23 Thread David Blaikie via Phabricator via lldb-commits

dblaikie added inline comments.



Comment at: lldb/source/Symbol/TypeSystem.cpp:331
   "TypeSystem for language " +
-  llvm::toStringRef(Language::GetNameForLanguageType(language)) +
+  llvm::StringRef(Language::GetNameForLanguageType(language)) +
   " doesn't exist",

labath wrote:
> dblaikie wrote:
> > teemperor wrote:
> > > dblaikie wrote:
> > > > Perhaps Language::GetNameForLanguageType should return StringRef to 
> > > > start with?
> > > > 
> > > > (& guessing there should be an lldb test case demonstrating this fix?)
> > > The problem is that this is used in LLDB's SB API which returns a const 
> > > char* (and we can't change that). So we can't return a StringRef as that 
> > > doesn't convert back to const char * (and we can't go over std::string or 
> > > anything like that as the API doesn't transfer ownership back to the 
> > > caller). I added an alternative method to save some typing (and maybe we 
> > > can go fully StringRef in the future if we ever change the SB API 
> > > strings).
> > > 
> > > And we don't test log message content in LLDB. Also the log message only 
> > > happens on systems with unsupported type system (like MIPS or something 
> > > like that). And we don't have any such bot that would even run the test.
> > Those both seem a bit problematic... 
> > 
> > Is there no API migration strategy in the SB API? Introducing new versions 
> > of functions and deprecating old ones?
> > 
> > And testing - clearly this code was buggy and worth fixing, so it ought to 
> > be worth testing. Any chance of unit testing/API-level testing any of this?
> > Is there no API migration strategy in the SB API? Introducing new versions 
> > of functions and deprecating old ones?
> 
> Right now, there isn't. However, I am not very happy with SB layer 
> restrictions constraining the internal APIs. One of the ways around that 
> (which is already used in a some places) is to pass the value through a 
> ConstString at the SB layer. This will guarantee that the string is null 
> terminated and persistent (at the cost of some cpu and memory). Though I am 
> not very fond of that solution either, it does not seem like too bad of an 
> option for this case, as the set of language names is limited.
> 
> As for testing of logging, there definitely are ways to test that, but we 
> (usually) don't do that. I think the best analogy here would be the 
> LLVM_DEBUG output in llvm. This is also mainly a debugging aid, and we 
> usually don't have dedicated tests for that (though it would certainly be 
> possible to do that). I think both mechanisms would benefit from some 
> testing, but the question is how much testing, and whether that time wouldn't 
> be better spent improving test coverage elsewhere...
Fair enough. Pity, but ah well.

Might be possible to add a unit test that demonstrates the SFINAE at work (I 
don't know for sure, though - you'd need to introduce an overload set that 
would've preferred toStringRef(bool) before the change, but now prefers some 
overload in the unit test with different behavior you could use to 
differentiate the two & show the SFINAE at work)


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

https://reviews.llvm.org/D65942



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

[Lldb-commits] [PATCH] D67903: [lldb] Add completion support for log enable/disable/list

2019-09-23 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision as: JDevlieghere.
JDevlieghere added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Utility/Log.cpp:49
+
+void Log::ListCategories(llvm::raw_ostream &stream,
+ const ChannelMap::value_type &entry) {

Is this used outside of CommandObjectLog? Otherwise this should probably go 
there.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67903



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

[Lldb-commits] [PATCH] D67906: [lldb] Provide tab completion for target select/delete

2019-09-23 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:63
const char *prefix_cstr,
-   bool show_stopped_process_status, Stream &strm) {
+   bool show_stopped_process_status,
+   bool print_target_index, Stream &strm) {

Can we convert these boolean flags with a new enum or struct 
`TargetInfoDumpOptions`? 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67906



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

+1 on passing this information to the Python script. Personally I'd like to 
move more of that logic into CMake so we can properly track dependencies and 
don't have to have dummy targets that are always out of date. This would be a 
step in the right direction.


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Haibo Huang via Phabricator via lldb-commits

hhb added inline comments.



Comment at: lldb/scripts/CMakeLists.txt:45-50
+  execute_process(
+COMMAND ${PYTHON_EXECUTABLE}
+-c "import distutils.sysconfig, sys; 
print(distutils.sysconfig.get_python_lib(True, False, sys.argv[1]))"
+${CMAKE_BINARY_DIR}
+OUTPUT_VARIABLE SWIG_PYTHON_DIR
+OUTPUT_STRIP_TRAILING_WHITESPACE)

mgorny wrote:
> labath wrote:
> > mgorny wrote:
> > > labath wrote:
> > > > mgorny wrote:
> > > > > labath wrote:
> > > > > > For my own education, is it possible that the result of the 
> > > > > > `get_python_lib` call will fundamentally differ depending on the 
> > > > > > value of the third argument. I.e., is there any case where 
> > > > > > `${SWIG_PYTHON_DIR}` will be different from 
> > > > > > `${CMAKE_BINARY_DIR}/${SWIG_INSTALL_DIR}` ?
> > > > > The former will be an absolute path while the latter is just the 
> > > > > relative path. But if you mean whether they could have a different 
> > > > > tail: I don't think so, at least with CPython. PyPy had some nasty 
> > > > > logic, so I'd have to check if we ever decided to support that.
> > > > Right now that doesn't matter, but I am thinking ahead about the 
> > > > cross-compilation case. If we turn out to need a cache variable for 
> > > > this, it would be nice if it would be a single variable that the user 
> > > > could set (instead of two of them). For that to work, we'd need to be 
> > > > able to compute the result of one of these calls using the value of the 
> > > > other one.
> > > I suppose you mean having a variable specifying path relative to 
> > > prefix/build dir, I presume? I suppose we could build the whole thing 
> > > around a cache var defaulting to the sysconfig call as proposed here for 
> > > `SWIG_INSTALL_DIR` (note it's passing empty prefix to `get_python_lib()` 
> > > in order to get relative path).
> > > 
> > > Thinking about it, maybe it'd indeed be better for me to prepare a more 
> > > complete patch built around that, and focus on testing that instead. I 
> > > was mostly worried/confused by Linux-specific code hanging around.
> > > I suppose you mean having a variable specifying path relative to 
> > > prefix/build dir, I presume?
> > Probably something like that, though I am still kind of hoping that there 
> > will be some way to detect this from cmake. However, that is looking less 
> > and less likely the more time I spend looking for a way to do it.
> > 
> > > Thinking about it, maybe it'd indeed be better for me to prepare a more 
> > > complete patch built around that, and focus on testing that instead.
> > That could work too, though I think the current version is an improvement 
> > in itself, and I agree we should do this slowly.
> I've grepped the `FindPython` modules and couldn't think anything useful. I 
> suppose the external call to Python is as close as we can get. This is 
> roughly what autotools is doing, except they go for full path and I tried to 
> use relative path. The former is probably more correct (as Python won't look 
> for the module in non-standard `CMAKE_INSTALL_PREFIX`) but I went for 
> preserving existing behavior.
> I suppose you mean having a variable specifying path relative to prefix/build 
> dir, I presume?

Sounds good to me and we do need cross compiling from Linux to Windows.



Comment at: lldb/scripts/get_relative_lib_dir.py:26
 split_libdir = arch_specific_libdir.split(os.sep)
-lib_re = re.compile(r"^lib.+$")
+lib_re = re.compile(r"^lib.*$")
 

If we go this way, should we always use LLDB_PYTHON_RELATIVE_LIBDIR in 
ScriptInterpreterPython.cpp, and add some code to make sure it is defined? 
Because all assumption of the path can be wrong.

After the change here, I think POSIX will always use 
LLDB_PYTHON_RELATIVE_LIBDIR. But for windows, the path is still hard coded to 
lib/site-packages.

(maybe finishSwigPythonLLDB.py / make_symlink() can also be updated to use 
os.path.relpath? )


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Haibo Huang via Phabricator via lldb-commits

hhb added a comment.

And thanks for this and it is definitely an improvement!


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67869: [ABISysV] Fix regression for Simulator and MacABI

2019-09-23 Thread Adrian Prantl via Phabricator via lldb-commits

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



Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:233
+  case llvm::Triple::EnvironmentType::Simulator:
+  case llvm::Triple::EnvironmentType::UnknownEnvironment:
+  case llvm::Triple::EnvironmentType::MacABI:

Can you add a comment here that this is to support older compilers that don't 
support the -simulator environment yet?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67869



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Michał Górny via Phabricator via lldb-commits

mgorny added a reviewer: ZeGentzy.
mgorny marked an inline comment as done.
mgorny added inline comments.



Comment at: lldb/scripts/get_relative_lib_dir.py:26
 split_libdir = arch_specific_libdir.split(os.sep)
-lib_re = re.compile(r"^lib.+$")
+lib_re = re.compile(r"^lib.*$")
 

hhb wrote:
> If we go this way, should we always use LLDB_PYTHON_RELATIVE_LIBDIR in 
> ScriptInterpreterPython.cpp, and add some code to make sure it is defined? 
> Because all assumption of the path can be wrong.
> 
> After the change here, I think POSIX will always use 
> LLDB_PYTHON_RELATIVE_LIBDIR. But for windows, the path is still hard coded to 
> lib/site-packages.
> 
> (maybe finishSwigPythonLLDB.py / make_symlink() can also be updated to use 
> os.path.relpath? )
Actually, I think we can kill all this logic by simply passing `''` as prefix, 
as I did in the CMake part.


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67869: [ABISysV] Fix regression for Simulator and MacABI

2019-09-23 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added a comment.

What does `x86_64-apple-darwin` canonicalize to? Do we need an extra case for 
that?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67869



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

[Lldb-commits] [PATCH] D67869: [ABISysV] Fix regression for Simulator and MacABI

2019-09-23 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added inline comments.



Comment at: lldb/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp:231
+  case llvm::Triple::OSType::WatchOS:
+switch(os_env) {
+  case llvm::Triple::EnvironmentType::Simulator:

clang-format?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67869



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Haibo Huang via Phabricator via lldb-commits

hhb added inline comments.

Comment at: lldb/scripts/get_relative_lib_dir.py:26
 split_libdir = arch_specific_libdir.split(os.sep)
-lib_re = re.compile(r"^lib.+$")
+lib_re = re.compile(r"^lib.*$")

mgorny wrote:
> hhb wrote:
> > If we go this way, should we always use LLDB_PYTHON_RELATIVE_LIBDIR in 
> > ScriptInterpreterPython.cpp, and add some code to make sure it is defined? 
> > Because all assumption of the path can be wrong.
> > 
> > After the change here, I think POSIX will always use 
> > LLDB_PYTHON_RELATIVE_LIBDIR. But for windows, the path is still hard coded 
> > to lib/site-packages.
> > 
> > (maybe finishSwigPythonLLDB.py / make_symlink() can also be updated to use 
> > os.path.relpath? )
> Actually, I think we can kill all this logic by simply passing `''` as 
> prefix, as I did in the CMake part.
I'm not sure. On my machine:

$ python3
Python 3.6.8 (default, Jan  3 2019, 03:42:36)
[GCC 8.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
\>>> import distutils.sysconfig
\>>> distutils.sysconfig.get_python_lib(True, False)
'/usr/lib/python3/dist-packages'
\>>> distutils.sysconfig.get_python_lib(True, False, '')
'lib/python3/dist-packages'
\>>> distutils.sysconfig.get_python_lib(True, False, '/src/lib')
'/src/lib/lib/python3.6/site-packages'

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

https://reviews.llvm.org/D67890

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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Michał Górny via Phabricator via lldb-commits

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



Comment at: lldb/scripts/get_relative_lib_dir.py:26
 split_libdir = arch_specific_libdir.split(os.sep)
-lib_re = re.compile(r"^lib.+$")
+lib_re = re.compile(r"^lib.*$")
 

hhb wrote:
> mgorny wrote:
> > hhb wrote:
> > > If we go this way, should we always use LLDB_PYTHON_RELATIVE_LIBDIR in 
> > > ScriptInterpreterPython.cpp, and add some code to make sure it is 
> > > defined? Because all assumption of the path can be wrong.
> > > 
> > > After the change here, I think POSIX will always use 
> > > LLDB_PYTHON_RELATIVE_LIBDIR. But for windows, the path is still hard 
> > > coded to lib/site-packages.
> > > 
> > > (maybe finishSwigPythonLLDB.py / make_symlink() can also be updated to 
> > > use os.path.relpath? )
> > Actually, I think we can kill all this logic by simply passing `''` as 
> > prefix, as I did in the CMake part.
> I'm not sure. On my machine:
> 
> $ python3
> Python 3.6.8 (default, Jan  3 2019, 03:42:36)
> [GCC 8.2.0] on linux
> Type "help", "copyright", "credits" or "license" for more information.
> \>>> import distutils.sysconfig
> \>>> distutils.sysconfig.get_python_lib(True, False)
> '/usr/lib/python3/dist-packages'
> \>>> distutils.sysconfig.get_python_lib(True, False, '')
> 'lib/python3/dist-packages'
> \>>> distutils.sysconfig.get_python_lib(True, False, '/src/lib')
> '/src/lib/lib/python3.6/site-packages'
> 
Hm, that's interesting. The documentation says:

> If 'prefix' is supplied, use it instead of sys.base_prefix or 
> sys.base_exec_prefix -- i.e., ignore 'plat_specific'.

So apparently first arg being true is meaningless then. Maybe we should go for 
`get_python_lib(False, False, '')`?


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Haibo Huang via Phabricator via lldb-commits

hhb added inline comments.



Comment at: lldb/scripts/get_relative_lib_dir.py:26
 split_libdir = arch_specific_libdir.split(os.sep)
-lib_re = re.compile(r"^lib.+$")
+lib_re = re.compile(r"^lib.*$")
 

mgorny wrote:
> hhb wrote:
> > mgorny wrote:
> > > hhb wrote:
> > > > If we go this way, should we always use LLDB_PYTHON_RELATIVE_LIBDIR in 
> > > > ScriptInterpreterPython.cpp, and add some code to make sure it is 
> > > > defined? Because all assumption of the path can be wrong.
> > > > 
> > > > After the change here, I think POSIX will always use 
> > > > LLDB_PYTHON_RELATIVE_LIBDIR. But for windows, the path is still hard 
> > > > coded to lib/site-packages.
> > > > 
> > > > (maybe finishSwigPythonLLDB.py / make_symlink() can also be updated to 
> > > > use os.path.relpath? )
> > > Actually, I think we can kill all this logic by simply passing `''` as 
> > > prefix, as I did in the CMake part.
> > I'm not sure. On my machine:
> > 
> > $ python3
> > Python 3.6.8 (default, Jan  3 2019, 03:42:36)
> > [GCC 8.2.0] on linux
> > Type "help", "copyright", "credits" or "license" for more information.
> > \>>> import distutils.sysconfig
> > \>>> distutils.sysconfig.get_python_lib(True, False)
> > '/usr/lib/python3/dist-packages'
> > \>>> distutils.sysconfig.get_python_lib(True, False, '')
> > 'lib/python3/dist-packages'
> > \>>> distutils.sysconfig.get_python_lib(True, False, '/src/lib')
> > '/src/lib/lib/python3.6/site-packages'
> > 
> Hm, that's interesting. The documentation says:
> 
> > If 'prefix' is supplied, use it instead of sys.base_prefix or 
> > sys.base_exec_prefix -- i.e., ignore 'plat_specific'.
> 
> So apparently first arg being true is meaningless then. Maybe we should go 
> for `get_python_lib(False, False, '')`?
Well I checked the code. plat_specific IS ignored. But it will test whether 
prefix "is default".

```
is_default_prefix = not prefix or os.path.normpath(prefix) in ('/usr', 
'/usr/local')
```

And do things differently based on that. Sigh..


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [lldb] r372634 - [lldb-suite] TestCallOverriddenMethod.py is now passing on Windows

2019-09-23 Thread Stella Stamenova via lldb-commits

Author: stella.stamenova
Date: Mon Sep 23 10:51:27 2019
New Revision: 372634

URL: http://llvm.org/viewvc/llvm-project?rev=372634&view=rev
Log:
[lldb-suite] TestCallOverriddenMethod.py is now passing on Windows

The test is now passing, so remove the expected failure. No other tests 
associated with the bug are passing, though, so only remove expected failure 
from this one test

Modified:

lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py?rev=372634&r1=372633&r2=372634&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
 Mon Sep 23 10:51:27 2019
@@ -26,7 +26,6 @@ class ExprCommandCallOverriddenMethod(Te
 # Find the line number to break for main.c.
 self.line = line_number('main.cpp', '// Set breakpoint here')
 
-@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr21765")
 def test(self):
 """Test calls to overridden methods in derived classes."""
 self.build()


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

[Lldb-commits] [PATCH] D66638: Unwind: Add a stack scanning mechanism to support win32 unwinding

2019-09-23 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D66638#1679195 , @labath wrote:

> Thanks for the review and sorry for the delay (I was OOO). The idea to use 
> `Process::GetLoadAddressPermissions` makes sense, both from consistency and 
> correctness perspectives. Unfortunately it does not work "out of the box" 
> because minidump core files (my primary use case for this) do not currently 
> provide enough information through the memory region info api. It tries 
> pretty hard to achieve that, but in the case of regular (not "full memory 
> dumps") windows minidump, all we have is the "MemoryList" stream, which only 
> contains a couple of memory regions, and it does not include the areas 
> covered by loaded modules (which is where PCs should point). If this is to 
> work, we'd need to extend this memory parsing code to also take into account 
> the module memory ranges (from the ModuleList stream).
>
> @clayborg, you're the one who wrote the memory region parsing code IIRC. Does 
> that sound OK to you ?


I am not sure if we can infer anything about permissions from the module 
ranges. I believe this range contains everything (.text, .data, .bss, etc), so 
it has a mixture of permissions?

If we can find a way to use the module base of image, we should only parse the 
module ranges if the /proc//maps is not in a minidump file as that is the 
best source for mapped ranges.

> The disassembling idea also sounds interesting, but I am afraid it's going to 
> be very useful for my main use case. The main use case for breakpad symbol 
> files is for the cases where one does not have the original object file (and 
> when one has the original object file, it probably also has proper debug and 
> unwind info). It _might_ be more interesting once we get around to PECOFF 
> unwinding, as it also uses raSearch (on i386), and one is more likely to have 
> the original file to disassemble there, but even then, I'd first check what 
> other windows debuggers do, as might be best to just follow their lead. So, 
> for now, I'd like to skip the disassembling logic.




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

https://reviews.llvm.org/D66638



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Michał Górny via Phabricator via lldb-commits

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



Comment at: lldb/scripts/get_relative_lib_dir.py:26
 split_libdir = arch_specific_libdir.split(os.sep)
-lib_re = re.compile(r"^lib.+$")
+lib_re = re.compile(r"^lib.*$")
 

hhb wrote:
> mgorny wrote:
> > hhb wrote:
> > > mgorny wrote:
> > > > hhb wrote:
> > > > > If we go this way, should we always use LLDB_PYTHON_RELATIVE_LIBDIR 
> > > > > in ScriptInterpreterPython.cpp, and add some code to make sure it is 
> > > > > defined? Because all assumption of the path can be wrong.
> > > > > 
> > > > > After the change here, I think POSIX will always use 
> > > > > LLDB_PYTHON_RELATIVE_LIBDIR. But for windows, the path is still hard 
> > > > > coded to lib/site-packages.
> > > > > 
> > > > > (maybe finishSwigPythonLLDB.py / make_symlink() can also be updated 
> > > > > to use os.path.relpath? )
> > > > Actually, I think we can kill all this logic by simply passing `''` as 
> > > > prefix, as I did in the CMake part.
> > > I'm not sure. On my machine:
> > > 
> > > $ python3
> > > Python 3.6.8 (default, Jan  3 2019, 03:42:36)
> > > [GCC 8.2.0] on linux
> > > Type "help", "copyright", "credits" or "license" for more information.
> > > \>>> import distutils.sysconfig
> > > \>>> distutils.sysconfig.get_python_lib(True, False)
> > > '/usr/lib/python3/dist-packages'
> > > \>>> distutils.sysconfig.get_python_lib(True, False, '')
> > > 'lib/python3/dist-packages'
> > > \>>> distutils.sysconfig.get_python_lib(True, False, '/src/lib')
> > > '/src/lib/lib/python3.6/site-packages'
> > > 
> > Hm, that's interesting. The documentation says:
> > 
> > > If 'prefix' is supplied, use it instead of sys.base_prefix or 
> > > sys.base_exec_prefix -- i.e., ignore 'plat_specific'.
> > 
> > So apparently first arg being true is meaningless then. Maybe we should go 
> > for `get_python_lib(False, False, '')`?
> Well I checked the code. plat_specific IS ignored. But it will test whether 
> prefix "is default".
> 
> ```
> is_default_prefix = not prefix or os.path.normpath(prefix) in ('/usr', 
> '/usr/local')
> ```
> 
> And do things differently based on that. Sigh..
I'm looking through the code of CPython and I don't see `dist-packages` 
anywhere. Is this some local distro patching or something?

What I'm really wondering is whether we need to split `.so` and `.py` modules. 
Technically distutils does that but it seems to use the same path for both on 
all platforms I see in `INSTALL_SCHEMES`.


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Haibo Huang via Phabricator via lldb-commits

hhb added inline comments.



Comment at: lldb/scripts/get_relative_lib_dir.py:26
 split_libdir = arch_specific_libdir.split(os.sep)
-lib_re = re.compile(r"^lib.+$")
+lib_re = re.compile(r"^lib.*$")
 

mgorny wrote:
> hhb wrote:
> > mgorny wrote:
> > > hhb wrote:
> > > > mgorny wrote:
> > > > > hhb wrote:
> > > > > > If we go this way, should we always use LLDB_PYTHON_RELATIVE_LIBDIR 
> > > > > > in ScriptInterpreterPython.cpp, and add some code to make sure it 
> > > > > > is defined? Because all assumption of the path can be wrong.
> > > > > > 
> > > > > > After the change here, I think POSIX will always use 
> > > > > > LLDB_PYTHON_RELATIVE_LIBDIR. But for windows, the path is still 
> > > > > > hard coded to lib/site-packages.
> > > > > > 
> > > > > > (maybe finishSwigPythonLLDB.py / make_symlink() can also be updated 
> > > > > > to use os.path.relpath? )
> > > > > Actually, I think we can kill all this logic by simply passing `''` 
> > > > > as prefix, as I did in the CMake part.
> > > > I'm not sure. On my machine:
> > > > 
> > > > $ python3
> > > > Python 3.6.8 (default, Jan  3 2019, 03:42:36)
> > > > [GCC 8.2.0] on linux
> > > > Type "help", "copyright", "credits" or "license" for more information.
> > > > \>>> import distutils.sysconfig
> > > > \>>> distutils.sysconfig.get_python_lib(True, False)
> > > > '/usr/lib/python3/dist-packages'
> > > > \>>> distutils.sysconfig.get_python_lib(True, False, '')
> > > > 'lib/python3/dist-packages'
> > > > \>>> distutils.sysconfig.get_python_lib(True, False, '/src/lib')
> > > > '/src/lib/lib/python3.6/site-packages'
> > > > 
> > > Hm, that's interesting. The documentation says:
> > > 
> > > > If 'prefix' is supplied, use it instead of sys.base_prefix or 
> > > > sys.base_exec_prefix -- i.e., ignore 'plat_specific'.
> > > 
> > > So apparently first arg being true is meaningless then. Maybe we should 
> > > go for `get_python_lib(False, False, '')`?
> > Well I checked the code. plat_specific IS ignored. But it will test whether 
> > prefix "is default".
> > 
> > ```
> > is_default_prefix = not prefix or os.path.normpath(prefix) in ('/usr', 
> > '/usr/local')
> > ```
> > 
> > And do things differently based on that. Sigh..
> I'm looking through the code of CPython and I don't see `dist-packages` 
> anywhere. Is this some local distro patching or something?
> 
> What I'm really wondering is whether we need to split `.so` and `.py` 
> modules. Technically distutils does that but it seems to use the same path 
> for both on all platforms I see in `INSTALL_SCHEMES`.
It is an Debian patch. 
https://bugs.launchpad.net/ubuntu/+source/python3-defaults/+bug/1814653

What make things worse, the path returned by get_python_lib() may not be in 
sys.path(). But I guess that's their issue.
https://bugs.launchpad.net/ubuntu/+source/python3-defaults/+bug/1814653


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Haibo Huang via Phabricator via lldb-commits

hhb added inline comments.



Comment at: lldb/scripts/get_relative_lib_dir.py:26
 split_libdir = arch_specific_libdir.split(os.sep)
-lib_re = re.compile(r"^lib.+$")
+lib_re = re.compile(r"^lib.*$")
 

hhb wrote:
> mgorny wrote:
> > hhb wrote:
> > > mgorny wrote:
> > > > hhb wrote:
> > > > > mgorny wrote:
> > > > > > hhb wrote:
> > > > > > > If we go this way, should we always use 
> > > > > > > LLDB_PYTHON_RELATIVE_LIBDIR in ScriptInterpreterPython.cpp, and 
> > > > > > > add some code to make sure it is defined? Because all assumption 
> > > > > > > of the path can be wrong.
> > > > > > > 
> > > > > > > After the change here, I think POSIX will always use 
> > > > > > > LLDB_PYTHON_RELATIVE_LIBDIR. But for windows, the path is still 
> > > > > > > hard coded to lib/site-packages.
> > > > > > > 
> > > > > > > (maybe finishSwigPythonLLDB.py / make_symlink() can also be 
> > > > > > > updated to use os.path.relpath? )
> > > > > > Actually, I think we can kill all this logic by simply passing `''` 
> > > > > > as prefix, as I did in the CMake part.
> > > > > I'm not sure. On my machine:
> > > > > 
> > > > > $ python3
> > > > > Python 3.6.8 (default, Jan  3 2019, 03:42:36)
> > > > > [GCC 8.2.0] on linux
> > > > > Type "help", "copyright", "credits" or "license" for more information.
> > > > > \>>> import distutils.sysconfig
> > > > > \>>> distutils.sysconfig.get_python_lib(True, False)
> > > > > '/usr/lib/python3/dist-packages'
> > > > > \>>> distutils.sysconfig.get_python_lib(True, False, '')
> > > > > 'lib/python3/dist-packages'
> > > > > \>>> distutils.sysconfig.get_python_lib(True, False, '/src/lib')
> > > > > '/src/lib/lib/python3.6/site-packages'
> > > > > 
> > > > Hm, that's interesting. The documentation says:
> > > > 
> > > > > If 'prefix' is supplied, use it instead of sys.base_prefix or 
> > > > > sys.base_exec_prefix -- i.e., ignore 'plat_specific'.
> > > > 
> > > > So apparently first arg being true is meaningless then. Maybe we should 
> > > > go for `get_python_lib(False, False, '')`?
> > > Well I checked the code. plat_specific IS ignored. But it will test 
> > > whether prefix "is default".
> > > 
> > > ```
> > > is_default_prefix = not prefix or os.path.normpath(prefix) in ('/usr', 
> > > '/usr/local')
> > > ```
> > > 
> > > And do things differently based on that. Sigh..
> > I'm looking through the code of CPython and I don't see `dist-packages` 
> > anywhere. Is this some local distro patching or something?
> > 
> > What I'm really wondering is whether we need to split `.so` and `.py` 
> > modules. Technically distutils does that but it seems to use the same path 
> > for both on all platforms I see in `INSTALL_SCHEMES`.
> It is an Debian patch. 
> https://bugs.launchpad.net/ubuntu/+source/python3-defaults/+bug/1814653
> 
> What make things worse, the path returned by get_python_lib() may not be in 
> sys.path(). But I guess that's their issue.
> https://bugs.launchpad.net/ubuntu/+source/python3-defaults/+bug/1814653
Sorry first link is wrong. Debian patch:
https://salsa.debian.org/cpython-team/python3-stdlib/blob/master/debian/patches/3.6/distutils-install-layout.diff


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [PATCH] D67894: [LLDB] Rework a MinGW build fix from D65691

2019-09-23 Thread Haibo Huang via Phabricator via lldb-commits

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

Thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67894



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

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Fix installing Python modules on systems using /usr/lib

2019-09-23 Thread Michał Górny via Phabricator via lldb-commits

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



Comment at: lldb/scripts/get_relative_lib_dir.py:26
 split_libdir = arch_specific_libdir.split(os.sep)
-lib_re = re.compile(r"^lib.+$")
+lib_re = re.compile(r"^lib.*$")
 

hhb wrote:
> hhb wrote:
> > mgorny wrote:
> > > hhb wrote:
> > > > mgorny wrote:
> > > > > hhb wrote:
> > > > > > mgorny wrote:
> > > > > > > hhb wrote:
> > > > > > > > If we go this way, should we always use 
> > > > > > > > LLDB_PYTHON_RELATIVE_LIBDIR in ScriptInterpreterPython.cpp, and 
> > > > > > > > add some code to make sure it is defined? Because all 
> > > > > > > > assumption of the path can be wrong.
> > > > > > > > 
> > > > > > > > After the change here, I think POSIX will always use 
> > > > > > > > LLDB_PYTHON_RELATIVE_LIBDIR. But for windows, the path is still 
> > > > > > > > hard coded to lib/site-packages.
> > > > > > > > 
> > > > > > > > (maybe finishSwigPythonLLDB.py / make_symlink() can also be 
> > > > > > > > updated to use os.path.relpath? )
> > > > > > > Actually, I think we can kill all this logic by simply passing 
> > > > > > > `''` as prefix, as I did in the CMake part.
> > > > > > I'm not sure. On my machine:
> > > > > > 
> > > > > > $ python3
> > > > > > Python 3.6.8 (default, Jan  3 2019, 03:42:36)
> > > > > > [GCC 8.2.0] on linux
> > > > > > Type "help", "copyright", "credits" or "license" for more 
> > > > > > information.
> > > > > > \>>> import distutils.sysconfig
> > > > > > \>>> distutils.sysconfig.get_python_lib(True, False)
> > > > > > '/usr/lib/python3/dist-packages'
> > > > > > \>>> distutils.sysconfig.get_python_lib(True, False, '')
> > > > > > 'lib/python3/dist-packages'
> > > > > > \>>> distutils.sysconfig.get_python_lib(True, False, '/src/lib')
> > > > > > '/src/lib/lib/python3.6/site-packages'
> > > > > > 
> > > > > Hm, that's interesting. The documentation says:
> > > > > 
> > > > > > If 'prefix' is supplied, use it instead of sys.base_prefix or 
> > > > > > sys.base_exec_prefix -- i.e., ignore 'plat_specific'.
> > > > > 
> > > > > So apparently first arg being true is meaningless then. Maybe we 
> > > > > should go for `get_python_lib(False, False, '')`?
> > > > Well I checked the code. plat_specific IS ignored. But it will test 
> > > > whether prefix "is default".
> > > > 
> > > > ```
> > > > is_default_prefix = not prefix or os.path.normpath(prefix) in ('/usr', 
> > > > '/usr/local')
> > > > ```
> > > > 
> > > > And do things differently based on that. Sigh..
> > > I'm looking through the code of CPython and I don't see `dist-packages` 
> > > anywhere. Is this some local distro patching or something?
> > > 
> > > What I'm really wondering is whether we need to split `.so` and `.py` 
> > > modules. Technically distutils does that but it seems to use the same 
> > > path for both on all platforms I see in `INSTALL_SCHEMES`.
> > It is an Debian patch. 
> > https://bugs.launchpad.net/ubuntu/+source/python3-defaults/+bug/1814653
> > 
> > What make things worse, the path returned by get_python_lib() may not be in 
> > sys.path(). But I guess that's their issue.
> > https://bugs.launchpad.net/ubuntu/+source/python3-defaults/+bug/1814653
> Sorry first link is wrong. Debian patch:
> https://salsa.debian.org/cpython-team/python3-stdlib/blob/master/debian/patches/3.6/distutils-install-layout.diff
Looking at it, it still uses the same location for `.py` and `.so`, just uses 
`dist-packages` instead of `site-packages` for stuff that's supposed to be 
installed by .deb packages. Do you expect LLDB to use dist-packages or 
site-packages by default? I think their intent is the former.


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

https://reviews.llvm.org/D67890



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

[Lldb-commits] [lldb] r372642 - [ABISysV] Fix regression for Simulator and MacABI

2019-09-23 Thread Jonas Devlieghere via lldb-commits

Author: jdevlieghere
Date: Mon Sep 23 12:06:00 2019
New Revision: 372642

URL: http://llvm.org/viewvc/llvm-project?rev=372642&view=rev
Log:
[ABISysV] Fix regression for Simulator and MacABI

The ABISysV ABI was refactored in r364216 to support the Windows ABI for
x86_64. In particular it changed ABISysV_x86_64::CreateInstance to
switch on the OS type. This breaks debugging MacABI apps as well as apps
in the simulator. This adds back the necessary cases.

We have a test on Github that exercises this code path and which I'd
like to upstream once the remaining MacABI parts become available in
clang.

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

Modified:
lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp

Modified: lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp?rev=372642&r1=372641&r2=372642&view=diff
==
--- lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp (original)
+++ lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp Mon Sep 23 
12:06:00 2019
@@ -222,17 +222,33 @@ ABISP
 ABISysV_x86_64::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec 
&arch) {
   const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
   const llvm::Triple::OSType os_type = arch.GetTriple().getOS();
+  const llvm::Triple::EnvironmentType os_env =
+  arch.GetTriple().getEnvironment();
   if (arch_type == llvm::Triple::x86_64) {
 switch(os_type) {
-  case llvm::Triple::OSType::MacOSX:
-  case llvm::Triple::OSType::Linux:
-  case llvm::Triple::OSType::FreeBSD:
-  case llvm::Triple::OSType::NetBSD:
-  case llvm::Triple::OSType::Solaris:
-  case llvm::Triple::OSType::UnknownOS:
+case llvm::Triple::OSType::IOS:
+case llvm::Triple::OSType::TvOS:
+case llvm::Triple::OSType::WatchOS:
+  switch (os_env) {
+  case llvm::Triple::EnvironmentType::MacABI:
+  case llvm::Triple::EnvironmentType::Simulator:
+  case llvm::Triple::EnvironmentType::UnknownEnvironment:
+// UnknownEnvironment is needed for older compilers that don't
+// support the simulator environment.
 return ABISP(new ABISysV_x86_64(process_sp));
-  default: 
+  default:
 return ABISP();
+  }
+case llvm::Triple::OSType::Darwin:
+case llvm::Triple::OSType::FreeBSD:
+case llvm::Triple::OSType::Linux:
+case llvm::Triple::OSType::MacOSX:
+case llvm::Triple::OSType::NetBSD:
+case llvm::Triple::OSType::Solaris:
+case llvm::Triple::OSType::UnknownOS:
+  return ABISP(new ABISysV_x86_64(process_sp));
+default:
+  return ABISP();
 }
   }
   return ABISP();


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

[Lldb-commits] [PATCH] D67869: [ABISysV] Fix regression for Simulator and MacABI

2019-09-23 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372642: [ABISysV] Fix regression for Simulator and MacABI 
(authored by JDevlieghere, committed by ).
Herald added subscribers: llvm-commits, fedor.sergeev.
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D67869?vs=221134&id=221380#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67869

Files:
  lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp


Index: lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
===
--- lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
+++ lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
@@ -222,17 +222,33 @@
 ABISysV_x86_64::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec 
&arch) {
   const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
   const llvm::Triple::OSType os_type = arch.GetTriple().getOS();
+  const llvm::Triple::EnvironmentType os_env =
+  arch.GetTriple().getEnvironment();
   if (arch_type == llvm::Triple::x86_64) {
 switch(os_type) {
-  case llvm::Triple::OSType::MacOSX:
-  case llvm::Triple::OSType::Linux:
-  case llvm::Triple::OSType::FreeBSD:
-  case llvm::Triple::OSType::NetBSD:
-  case llvm::Triple::OSType::Solaris:
-  case llvm::Triple::OSType::UnknownOS:
+case llvm::Triple::OSType::IOS:
+case llvm::Triple::OSType::TvOS:
+case llvm::Triple::OSType::WatchOS:
+  switch (os_env) {
+  case llvm::Triple::EnvironmentType::MacABI:
+  case llvm::Triple::EnvironmentType::Simulator:
+  case llvm::Triple::EnvironmentType::UnknownEnvironment:
+// UnknownEnvironment is needed for older compilers that don't
+// support the simulator environment.
 return ABISP(new ABISysV_x86_64(process_sp));
-  default: 
+  default:
 return ABISP();
+  }
+case llvm::Triple::OSType::Darwin:
+case llvm::Triple::OSType::FreeBSD:
+case llvm::Triple::OSType::Linux:
+case llvm::Triple::OSType::MacOSX:
+case llvm::Triple::OSType::NetBSD:
+case llvm::Triple::OSType::Solaris:
+case llvm::Triple::OSType::UnknownOS:
+  return ABISP(new ABISysV_x86_64(process_sp));
+default:
+  return ABISP();
 }
   }
   return ABISP();


Index: lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
===
--- lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
+++ lldb/trunk/source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
@@ -222,17 +222,33 @@
 ABISysV_x86_64::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
   const llvm::Triple::ArchType arch_type = arch.GetTriple().getArch();
   const llvm::Triple::OSType os_type = arch.GetTriple().getOS();
+  const llvm::Triple::EnvironmentType os_env =
+  arch.GetTriple().getEnvironment();
   if (arch_type == llvm::Triple::x86_64) {
 switch(os_type) {
-  case llvm::Triple::OSType::MacOSX:
-  case llvm::Triple::OSType::Linux:
-  case llvm::Triple::OSType::FreeBSD:
-  case llvm::Triple::OSType::NetBSD:
-  case llvm::Triple::OSType::Solaris:
-  case llvm::Triple::OSType::UnknownOS:
+case llvm::Triple::OSType::IOS:
+case llvm::Triple::OSType::TvOS:
+case llvm::Triple::OSType::WatchOS:
+  switch (os_env) {
+  case llvm::Triple::EnvironmentType::MacABI:
+  case llvm::Triple::EnvironmentType::Simulator:
+  case llvm::Triple::EnvironmentType::UnknownEnvironment:
+// UnknownEnvironment is needed for older compilers that don't
+// support the simulator environment.
 return ABISP(new ABISysV_x86_64(process_sp));
-  default: 
+  default:
 return ABISP();
+  }
+case llvm::Triple::OSType::Darwin:
+case llvm::Triple::OSType::FreeBSD:
+case llvm::Triple::OSType::Linux:
+case llvm::Triple::OSType::MacOSX:
+case llvm::Triple::OSType::NetBSD:
+case llvm::Triple::OSType::Solaris:
+case llvm::Triple::OSType::UnknownOS:
+  return ABISP(new ABISysV_x86_64(process_sp));
+default:
+  return ABISP();
 }
   }
   return ABISP();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67891: remove File::SetStream(), make new files instead.

2019-09-23 Thread Lawrence D'Anna via Phabricator via lldb-commits

lawrence_danna added a comment.

In D67891#1678933 , @labath wrote:

> The first option would require making the File class movable


I don't think making File movable will work.   I'll be wanting a PythonFile 
subclass, which overrides virtual methods in File.   It won't be possible to 
move one of those into a File variable. I do think the second option, with 
unique_ptr does make sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67891



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

[Lldb-commits] [PATCH] D67911: [LLDB] [Windows] Add missing ifdefs to fix building for non-x86 architectures

2019-09-23 Thread Martin Storsjö via Phabricator via lldb-commits

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

In D67911#1679333 , @compnerd wrote:

> What do you think of adding some sort of notification that hardware 
> breakpoints are currently unsupported and that we are falling back to 
> software breakpoints for the `#else` case?


I guess it could make sense. What kind of notification do you have in mind?




Comment at: 
lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp:87
   case 4:
 #if defined(__x86_64__) || defined(_M_AMD64)
   case 8:

compnerd wrote:
> Should this line not also include `|| defined(__aarch64__) || 
> defined(_M_ARM64)`?
I guess it should. Or this might be one of the few places where checking for 
just `_WIN64` might be easiest?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67911



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

[Lldb-commits] [PATCH] D67789: bugfix: File::GetWaitableHandle() should call fileno()

2019-09-23 Thread Lawrence D'Anna via Phabricator via lldb-commits

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

nits fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67789

Files:
  lldb/source/Host/common/File.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/FileTest.cpp


Index: lldb/unittests/Host/FileTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/FileTest.cpp
@@ -0,0 +1,36 @@
+//===-- FileTest.cpp *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/File.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(File, GetWaitableHandleFileno) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+
+  llvm::SmallString<128> name;
+  int fd;
+  llvm::sys::fs::createTemporaryFile(llvm::Twine(Info->test_case_name()) + "-" 
+
+ Info->name(),
+ "test", fd, name);
+  llvm::FileRemover remover(name);
+  ASSERT_GE(fd, 0);
+
+  FILE *stream = fdopen(fd, "r");
+  ASSERT_TRUE(stream);
+
+  File file(stream, true);
+  EXPECT_EQ(file.GetWaitableHandle(), fd);
+}
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -2,6 +2,7 @@
   ConnectionFileDescriptorTest.cpp
   FileActionTest.cpp
   FileSystemTest.cpp
+  FileTest.cpp
   HostInfoTest.cpp
   HostTest.cpp
   MainLoopTest.cpp
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -91,7 +91,7 @@
   return kInvalidDescriptor;
 }
 
-IOObject::WaitableHandle File::GetWaitableHandle() { return m_descriptor; }
+IOObject::WaitableHandle File::GetWaitableHandle() { return GetDescriptor(); }
 
 void File::SetDescriptor(int fd, bool transfer_ownership) {
   if (IsValid())


Index: lldb/unittests/Host/FileTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/FileTest.cpp
@@ -0,0 +1,36 @@
+//===-- FileTest.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/File.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(File, GetWaitableHandleFileno) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+
+  llvm::SmallString<128> name;
+  int fd;
+  llvm::sys::fs::createTemporaryFile(llvm::Twine(Info->test_case_name()) + "-" +
+ Info->name(),
+ "test", fd, name);
+  llvm::FileRemover remover(name);
+  ASSERT_GE(fd, 0);
+
+  FILE *stream = fdopen(fd, "r");
+  ASSERT_TRUE(stream);
+
+  File file(stream, true);
+  EXPECT_EQ(file.GetWaitableHandle(), fd);
+}
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -2,6 +2,7 @@
   ConnectionFileDescriptorTest.cpp
   FileActionTest.cpp
   FileSystemTest.cpp
+  FileTest.cpp
   HostInfoTest.cpp
   HostTest.cpp
   MainLoopTest.cpp
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -91,7 +91,7 @@
   return kInvalidDescriptor;
 }
 
-IOObject::WaitableHandle File::GetWaitableHandle() { return m_descriptor; }
+IOObject::WaitableHandle File::GetWaitableHandle() { return GetDescriptor(); }
 
 void File::SetDescriptor(int fd, bool transfer_ownership) {
   if (IsValid())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67789: bugfix: File::GetWaitableHandle() should call fileno()

2019-09-23 Thread Lawrence D'Anna via Phabricator via lldb-commits

lawrence_danna marked 2 inline comments as done.
lawrence_danna added a comment.

nits fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67789



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

[Lldb-commits] [lldb] r372644 - [Host] File::GetWaitableHandle() should call fileno()

2019-09-23 Thread Jonas Devlieghere via lldb-commits

Author: jdevlieghere
Date: Mon Sep 23 12:34:26 2019
New Revision: 372644

URL: http://llvm.org/viewvc/llvm-project?rev=372644&view=rev
Log:
[Host] File::GetWaitableHandle() should call fileno()

If the file has m_stream, it may not have a m_descriptor.
GetWaitableHandle() should call GetDescriptor(), which will call
fileno(), so it will get waitable descriptor whenever one is available.

Patch by: Lawrence D'Anna

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

Added:
lldb/trunk/unittests/Host/FileTest.cpp
Modified:
lldb/trunk/source/Host/common/File.cpp
lldb/trunk/unittests/Host/CMakeLists.txt

Modified: lldb/trunk/source/Host/common/File.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/File.cpp?rev=372644&r1=372643&r2=372644&view=diff
==
--- lldb/trunk/source/Host/common/File.cpp (original)
+++ lldb/trunk/source/Host/common/File.cpp Mon Sep 23 12:34:26 2019
@@ -91,7 +91,7 @@ int File::GetDescriptor() const {
   return kInvalidDescriptor;
 }
 
-IOObject::WaitableHandle File::GetWaitableHandle() { return m_descriptor; }
+IOObject::WaitableHandle File::GetWaitableHandle() { return GetDescriptor(); }
 
 void File::SetDescriptor(int fd, bool transfer_ownership) {
   if (IsValid())

Modified: lldb/trunk/unittests/Host/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/CMakeLists.txt?rev=372644&r1=372643&r2=372644&view=diff
==
--- lldb/trunk/unittests/Host/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Host/CMakeLists.txt Mon Sep 23 12:34:26 2019
@@ -2,6 +2,7 @@ set (FILES
   ConnectionFileDescriptorTest.cpp
   FileActionTest.cpp
   FileSystemTest.cpp
+  FileTest.cpp
   HostInfoTest.cpp
   HostTest.cpp
   MainLoopTest.cpp

Added: lldb/trunk/unittests/Host/FileTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/FileTest.cpp?rev=372644&view=auto
==
--- lldb/trunk/unittests/Host/FileTest.cpp (added)
+++ lldb/trunk/unittests/Host/FileTest.cpp Mon Sep 23 12:34:26 2019
@@ -0,0 +1,36 @@
+//===-- FileTest.cpp *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/File.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(File, GetWaitableHandleFileno) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+
+  llvm::SmallString<128> name;
+  int fd;
+  llvm::sys::fs::createTemporaryFile(llvm::Twine(Info->test_case_name()) + "-" 
+
+ Info->name(),
+ "test", fd, name);
+  llvm::FileRemover remover(name);
+  ASSERT_GE(fd, 0);
+
+  FILE *stream = fdopen(fd, "r");
+  ASSERT_TRUE(stream);
+
+  File file(stream, true);
+  EXPECT_EQ(file.GetWaitableHandle(), fd);
+}


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

[Lldb-commits] [PATCH] D67789: bugfix: File::GetWaitableHandle() should call fileno()

2019-09-23 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL372644: [Host] File::GetWaitableHandle() should call 
fileno() (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67789?vs=221383&id=221387#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67789

Files:
  lldb/trunk/source/Host/common/File.cpp
  lldb/trunk/unittests/Host/CMakeLists.txt
  lldb/trunk/unittests/Host/FileTest.cpp


Index: lldb/trunk/unittests/Host/FileTest.cpp
===
--- lldb/trunk/unittests/Host/FileTest.cpp
+++ lldb/trunk/unittests/Host/FileTest.cpp
@@ -0,0 +1,36 @@
+//===-- FileTest.cpp *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/File.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(File, GetWaitableHandleFileno) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+
+  llvm::SmallString<128> name;
+  int fd;
+  llvm::sys::fs::createTemporaryFile(llvm::Twine(Info->test_case_name()) + "-" 
+
+ Info->name(),
+ "test", fd, name);
+  llvm::FileRemover remover(name);
+  ASSERT_GE(fd, 0);
+
+  FILE *stream = fdopen(fd, "r");
+  ASSERT_TRUE(stream);
+
+  File file(stream, true);
+  EXPECT_EQ(file.GetWaitableHandle(), fd);
+}
Index: lldb/trunk/unittests/Host/CMakeLists.txt
===
--- lldb/trunk/unittests/Host/CMakeLists.txt
+++ lldb/trunk/unittests/Host/CMakeLists.txt
@@ -2,6 +2,7 @@
   ConnectionFileDescriptorTest.cpp
   FileActionTest.cpp
   FileSystemTest.cpp
+  FileTest.cpp
   HostInfoTest.cpp
   HostTest.cpp
   MainLoopTest.cpp
Index: lldb/trunk/source/Host/common/File.cpp
===
--- lldb/trunk/source/Host/common/File.cpp
+++ lldb/trunk/source/Host/common/File.cpp
@@ -91,7 +91,7 @@
   return kInvalidDescriptor;
 }
 
-IOObject::WaitableHandle File::GetWaitableHandle() { return m_descriptor; }
+IOObject::WaitableHandle File::GetWaitableHandle() { return GetDescriptor(); }
 
 void File::SetDescriptor(int fd, bool transfer_ownership) {
   if (IsValid())


Index: lldb/trunk/unittests/Host/FileTest.cpp
===
--- lldb/trunk/unittests/Host/FileTest.cpp
+++ lldb/trunk/unittests/Host/FileTest.cpp
@@ -0,0 +1,36 @@
+//===-- FileTest.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/File.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(File, GetWaitableHandleFileno) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+
+  llvm::SmallString<128> name;
+  int fd;
+  llvm::sys::fs::createTemporaryFile(llvm::Twine(Info->test_case_name()) + "-" +
+ Info->name(),
+ "test", fd, name);
+  llvm::FileRemover remover(name);
+  ASSERT_GE(fd, 0);
+
+  FILE *stream = fdopen(fd, "r");
+  ASSERT_TRUE(stream);
+
+  File file(stream, true);
+  EXPECT_EQ(file.GetWaitableHandle(), fd);
+}
Index: lldb/trunk/unittests/Host/CMakeLists.txt
===
--- lldb/trunk/unittests/Host/CMakeLists.txt
+++ lldb/trunk/unittests/Host/CMakeLists.txt
@@ -2,6 +2,7 @@
   ConnectionFileDescriptorTest.cpp
   FileActionTest.cpp
   FileSystemTest.cpp
+  FileTest.cpp
   HostInfoTest.cpp
   HostTest.cpp
   MainLoopTest.cpp
Index: lldb/trunk/source/Host/common/File.cpp
===
--- lldb/trunk/source/Host/common/File.cpp
+++ lldb/trunk/source/Host/common/File.cpp
@@ -91,7 +91,7 @@
   return kInvalidDescriptor;
 }
 
-IOObject::

[Lldb-commits] [PATCH] D67890: [lldb] [cmake] Unify and correct Python module installation paths

2019-09-23 Thread Michał Górny via Phabricator via lldb-commits

mgorny updated this revision to Diff 221388.
mgorny retitled this revision from "[lldb] [cmake] Fix installing Python 
modules on systems using /usr/lib" to "[lldb] [cmake] Unify and correct Python 
module installation paths".
mgorny edited the summary of this revision.
mgorny added a comment.

O-kay, please try this patch instead. I think it should fix all the issues and 
avoid inconsistencies between various places this is used. However, I'm not 
sure if it doesn't break exotic platforms.


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

https://reviews.llvm.org/D67890

Files:
  lldb/scripts/CMakeLists.txt
  lldb/scripts/Python/prepare_binding_Python.py
  lldb/scripts/get_relative_lib_dir.py
  lldb/scripts/prepare_bindings.py
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -312,18 +312,11 @@
 void ScriptInterpreterPython::ComputePythonDirForPosix(
 llvm::SmallVectorImpl &path) {
   auto style = llvm::sys::path::Style::posix;
-#if defined(LLDB_PYTHON_RELATIVE_LIBDIR)
   // Build the path by backing out of the lib dir, then building with whatever
   // the real python interpreter uses.  (e.g. lib for most, lib64 on RHEL
   // x86_64).
   llvm::sys::path::remove_filename(path, style);
   llvm::sys::path::append(path, style, LLDB_PYTHON_RELATIVE_LIBDIR);
-#else
-  llvm::sys::path::append(path, style,
-  "python" + llvm::Twine(PY_MAJOR_VERSION) + "." +
-  llvm::Twine(PY_MINOR_VERSION),
-  "site-packages");
-#endif
 }
 
 void ScriptInterpreterPython::ComputePythonDirForWindows(
Index: lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
===
--- lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
+++ lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
@@ -1,15 +1,4 @@
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
-  # Call a python script to gather the arch-specific libdir for
-  # modules like the lldb module.
-  execute_process(
-COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/../../../../scripts/get_relative_lib_dir.py
-RESULT_VARIABLE get_libdir_status
-OUTPUT_VARIABLE relative_libdir
-)
-  if (get_libdir_status EQUAL 0)
-add_definitions(-DLLDB_PYTHON_RELATIVE_LIBDIR="${relative_libdir}")
-  endif()
-endif()
+add_definitions(-DLLDB_PYTHON_RELATIVE_LIBDIR="${LLDB_PYTHON_RELATIVE_PATH}")
 
 add_lldb_library(lldbPluginScriptInterpreterPython PLUGIN
   PythonDataObjects.cpp
Index: lldb/scripts/prepare_bindings.py
===
--- lldb/scripts/prepare_bindings.py
+++ lldb/scripts/prepare_bindings.py
@@ -129,6 +129,10 @@
 parser.add_argument(
 "--prefix",
 help="Override path where the LLDB module is placed.")
+parser.add_argument(
+"--python-relative-path",
+"--pythonRelativePath",
+help="Path to store Python modules in, relative to targetDir.")
 parser.add_argument(
 "--src-root",
 "--srcRoot",
Index: lldb/scripts/get_relative_lib_dir.py
===
--- lldb/scripts/get_relative_lib_dir.py
+++ /dev/null
@@ -1,44 +0,0 @@
-import distutils.sysconfig
-import os
-import platform
-import re
-import sys
-
-
-def get_python_relative_libdir():
-"""Returns the appropropriate python libdir relative to the build directory.
-
-@param exe_path the path to the lldb executable
-
-@return the python path that needs to be added to sys.path (PYTHONPATH)
-in order to find the lldb python module.
-"""
-if platform.system() != 'Linux':
-return None
-
-# We currently have a bug in lldb -P that does not account for
-# architecture variants in python paths for
-# architecture-specific modules.  Handle the lookup here.
-# When that bug is fixed, we should just ask lldb for the
-# right answer always.
-arch_specific_libdir = distutils.sysconfig.get_python_lib(True, False)
-split_libdir = arch_specific_libdir.split(os.sep)
-lib_re = re.compile(r"^lib.+$")
-
-for i in range(len(split_libdir)):
-match = lib_re.match(split_libdir[i])
-if match is not None:
-# We'll call this the relative root of the lib dir.
-# Things like RHEL will have an arch-specific python
-# lib dir, which isn't 'lib' on x86_64.
-return os.sep.join(split_libdir[i:])
-# Didn't resolve it.
-return None
-
-if __name__ == '__main__':
-lib_dir = get_python_re

[Lldb-commits] [PATCH] D67792: File::SetDescriptor() should require options

2019-09-23 Thread Lawrence D'Anna via Phabricator via lldb-commits

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

added unit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67792

Files:
  lldb/include/lldb/Host/File.h
  lldb/source/Core/StreamFile.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Target/Process.cpp
  lldb/unittests/Host/FileTest.cpp

Index: lldb/unittests/Host/FileTest.cpp
===
--- lldb/unittests/Host/FileTest.cpp
+++ lldb/unittests/Host/FileTest.cpp
@@ -34,3 +34,24 @@
   File file(stream, true);
   EXPECT_EQ(file.GetWaitableHandle(), fd);
 }
+
+TEST(File, GetStreamFromDescriptor) {
+  const auto *Info = testing::UnitTest::GetInstance()->current_test_info();
+  llvm::SmallString<128> name;
+  int fd;
+  llvm::sys::fs::createTemporaryFile(llvm::Twine(Info->test_case_name()) + "-" +
+ Info->name(),
+ "test", fd, name);
+
+  llvm::FileRemover remover(name);
+  ASSERT_GE(fd, 0);
+
+  File file(fd, File::eOpenOptionWrite, true);
+  ASSERT_TRUE(file.IsValid());
+
+  FILE *stream = file.GetStream();
+  ASSERT_TRUE(stream != NULL);
+
+  EXPECT_EQ(file.GetDescriptor(), fd);
+  EXPECT_EQ(file.GetWaitableHandle(), fd);
+}
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4299,9 +4299,10 @@
   IOHandlerProcessSTDIO(Process *process, int write_fd)
   : IOHandler(process->GetTarget().GetDebugger(),
   IOHandler::Type::ProcessIO),
-m_process(process), m_write_file(write_fd, false) {
+m_process(process),
+m_write_file(write_fd, File::eOpenOptionWrite, false) {
 m_pipe.CreateNew(false);
-m_read_file.SetDescriptor(GetInputFD(), false);
+m_read_file.SetDescriptor(GetInputFD(), File::eOpenOptionRead, false);
   }
 
   ~IOHandlerProcessSTDIO() override = default;
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1043,9 +1043,9 @@
   file.Close();
   // We don't own the file descriptor returned by this function, make sure the
   // File object knows about that.
-  file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), false);
   PythonString py_mode = GetAttributeValue("mode").AsType();
-  file.SetOptions(PythonFile::GetOptionsFromMode(py_mode.GetString()));
+  auto options = PythonFile::GetOptionsFromMode(py_mode.GetString());
+  file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), options, false);
   return file.IsValid();
 }
 
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
@@ -537,7 +537,7 @@
   int err = -1;
   int save_errno = 0;
   if (fd >= 0) {
-File file(fd, true);
+File file(fd, 0, true);
 Status error = file.Close();
 err = 0;
 save_errno = error.GetError();
@@ -568,7 +568,7 @@
   }
 
   std::string buffer(count, 0);
-  File file(fd, false);
+  File file(fd, File::eOpenOptionRead, false);
   Status error = file.Read(static_cast(&buffer[0]), count, offset);
   const ssize_t bytes_read = error.Success() ? count : -1;
   const int save_errno = error.GetError();
@@ -600,7 +600,7 @@
 if (packet.GetChar() == ',') {
   std::string buffer;
   if (packet.GetEscapedBinaryData(buffer)) {
-File file(fd, false);
+File file(fd, File::eOpenOptionWrite, false);
 size_t count = buffer.size();
 Status error =
 file.Write(static_cast(&buffer[0]), count, offset);
Index: lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
===
--- lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -425,11 +425,16 @@
   }
 }
 Status posix_error;
+int oflag = file_action->GetActionArgument();

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

2019-09-23 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

First off, note that this becomes complicated because this patch tied the 
decision of whether a given stop reason should warrant returning control to the 
user in lldb's "batch mode", and whether the signal should be passed back to 
the target.  That isn't a necessary binding, and indeed it might be surprising 
to people to find out that changing the "target handle -p" option changes the 
behavior of batch mode.  So I'm not 100% sure I think these two notions should 
be tied together.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67776



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

[Lldb-commits] [PATCH] D67774: [Mangle] Add flag to asm labels to disable global prefixing

2019-09-23 Thread Vedant Kumar via Phabricator via lldb-commits

vsk updated this revision to Diff 221391.
vsk marked 3 inline comments as done.
vsk added a comment.

- Address latest review feedback.


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

https://reviews.llvm.org/D67774

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/AST/Mangle.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/unittests/AST/DeclTest.cpp
  lldb/source/Symbol/ClangASTContext.cpp

Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -8300,8 +8300,8 @@
 cxx_method_decl->addAttr(clang::UsedAttr::CreateImplicit(*getASTContext()));
 
   if (mangled_name != nullptr) {
-cxx_method_decl->addAttr(
-clang::AsmLabelAttr::CreateImplicit(*getASTContext(), mangled_name));
+cxx_method_decl->addAttr(clang::AsmLabelAttr::CreateImplicit(
+*getASTContext(), mangled_name, /*literal=*/true));
   }
 
   // Populate the method decl with parameter decls
Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -10,12 +10,16 @@
 //
 //===--===//
 
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Mangle.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
 
 using namespace clang::ast_matchers;
 using namespace clang::tooling;
+using namespace clang;
 
 TEST(Decl, CleansUpAPValues) {
   MatchFinder Finder;
@@ -56,3 +60,49 @@
   "constexpr _Complex __uint128_t c = 0x;",
   Args));
 }
+
+TEST(Decl, AsmLabelAttr) {
+  // Create two method decls: `f` and `g`.
+  StringRef Code = R"(
+struct S {
+  void f() {}
+  void g() {}
+};
+  )";
+  auto AST =
+  tooling::buildASTFromCodeWithArgs(Code, {"-target", "i386-apple-darwin"});
+  ASTContext &Ctx = AST->getASTContext();
+  DiagnosticsEngine &Diags = AST->getDiagnostics();
+  SourceManager &SM = AST->getSourceManager();
+  FileID MainFileID = SM.getMainFileID();
+
+  // Find the method decls within the AST.
+  SmallVector Decls;
+  AST->findFileRegionDecls(MainFileID, Code.find('{'), 0, Decls);
+  ASSERT_TRUE(Decls.size() == 1);
+  CXXRecordDecl *DeclS = cast(Decls[0]);
+  NamedDecl *DeclF = *DeclS->method_begin();
+  NamedDecl *DeclG = *(++DeclS->method_begin());
+
+  // Attach asm labels to the decls: one literal, and one subject to global
+  // prefixing.
+  DeclF->addAttr(::new (Ctx) AsmLabelAttr(Ctx, SourceLocation(), "foo",
+  /*LiteralLabel=*/true));
+  DeclG->addAttr(::new (Ctx) AsmLabelAttr(Ctx, SourceLocation(), "goo",
+  /*LiteralLabel=*/false));
+
+  // Mangle the decl names.
+  std::string MangleF, MangleG;
+  MangleContext *MC = ItaniumMangleContext::create(Ctx, Diags);
+  {
+llvm::raw_string_ostream OS(MangleF);
+MC->mangleName(DeclF, OS);
+  }
+  {
+llvm::raw_string_ostream OS(MangleG);
+MC->mangleName(DeclG, OS);
+  }
+
+  ASSERT_TRUE(0 == MangleF.compare("foo"));
+  ASSERT_TRUE(0 == MangleG.compare("\x01goo"));
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2766,7 +2766,7 @@
 
   if (AsmLabelAttr *NewA = New->getAttr()) {
 if (AsmLabelAttr *OldA = Old->getAttr()) {
-  if (OldA->getLabel() != NewA->getLabel()) {
+  if (!OldA->equivalent(NewA)) {
 // This redeclaration changes __asm__ label.
 Diag(New->getLocation(), diag::err_different_asm_label);
 Diag(OldA->getLocation(), diag::note_previous_declaration);
@@ -6983,8 +6983,8 @@
   }
 }
 
-NewVD->addAttr(::new (Context)
-   AsmLabelAttr(Context, SE->getStrTokenLoc(0), Label));
+NewVD->addAttr(::new (Context) AsmLabelAttr(Context, SE->getStrTokenLoc(0),
+Label, /*LiteralLabel=*/false));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
@@ -8882,8 +8882,9 @@
   if (Expr *E = (Expr*) D.getAsmLabel()) {
 // The parser guarantees this is a string.
 StringLiteral *SE = cast(E);
-NewFD->addAttr(::new (Context) AsmLabelAttr(Context, SE->getStrTokenLoc(0),
-SE->getString()));
+NewFD->addAttr(::new (Context)
+   AsmLabelAttr(Context, SE->getStrTokenLoc(0),
+SE->getString(), /*LiteralLabel=*/false));
   } else if (!ExtnameUndeclaredIdentifiers.empty()) {
 llvm::DenseMap::iterator I =
   ExtnameU

1 2 >

1 - 100 of 128 matches

Mail list logo