Re: [Lldb-commits] [lldb] r367549 - [lit] Use ld.lld -z separate-code to work around a debug_line parsing bug

2019-08-01 Thread Fāng-ruì Sòng via lldb-commits
On Thu, Aug 1, 2019 at 8:38 PM Pavel Labath  wrote:

> On 01/08/2019 14:34, Fangrui Song via lldb-commits wrote:
> > Author: maskray
> > Date: Thu Aug  1 05:34:43 2019
> > New Revision: 367549
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=367549=rev
> > Log:
> > [lit] Use ld.lld -z separate-code to work around a debug_line parsing bug
> >
> > The issue was exposed by D64903/r367537.
> > http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/7321/
> >
> > In these tests, .debug_str immediately follows .text.
> > The last section of last RX PT_LOAD was originally padded with trap
> > instructions and .debug_str started at a new page (actually
> > common-page-size). Now, .debug_str immediately follows .test.
> > Add -z separate-code to use the old layout.
> >
>
>
> Thanks, I was about to commit the same patch.
>
> For future reference, here is my analysis I was going to put into the
> commit message:
>
> ---
> The lld patch changed how it lays out data in segments, which was
> resulted in the tiny test funclets being at the very end of a PT_LOAD
> segment. This exposed a bug where lldb does not correctly handle line
> table end-of-sequence entries to the very end of a section (so the byte
> they point to does not actually belong to any section).
>
> This is mostly bening in practice, as the code handling line tables will
> terminate as soon as it is not able to resolve an address, which is
> something that would happen after reading the end-of-sequence entry
> anyway. However, these tests assert that we are actually able to parse
> and display the end entry itself, and so they fail.
> ---
>

Thanks for the enlightening comment! I'll play with it to learn more about
the mechanism.

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


Re: [Lldb-commits] [lldb] r367549 - [lit] Use ld.lld -z separate-code to work around a debug_line parsing bug

2019-08-01 Thread Fāng-ruì Sòng via lldb-commits
Hi Raphael, you need lld built from top of trunk to run the lldb tests. -z
separate-code is added by rLLD367537, not included in lld 9.

On Fri, Aug 2, 2019 at 4:45 AM Raphael Isemann  wrote:

> This breaks the tests on my machine:
>
> 
> FAIL: LLDB :: SymbolFile/DWARF/dir-separator-no-comp-dir.s (114 of 1660)
>  TEST 'LLDB ::
> SymbolFile/DWARF/dir-separator-no-comp-dir.s' FAILED
> 
> Script:
> --
> : 'RUN: at line 6';   /home/teemperor/work/3llvm/rel/bin/llvm-mc
> -triple x86_64-pc-linux
>
> /home/teemperor/work/3llvm/llvm/lldb/lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s
> -filetype=obj >
>
> /home/teemperor/work/3llvm/rel/tools/lldb/lit/SymbolFile/DWARF/Output/dir-separator-no-comp-dir.s.tmp.o
> : 'RUN: at line 7';   /usr/bin/ld.lld -z separate-code
>
> /home/teemperor/work/3llvm/rel/tools/lldb/lit/SymbolFile/DWARF/Output/dir-separator-no-comp-dir.s.tmp.o
> -o /home/teemperor/work/3llvm/re
> l/tools/lldb/lit/SymbolFile/DWARF/Output/dir-separator-no-comp-dir.s.tmp
> : 'RUN: at line 8';   /home/teemperor/work/3llvm/rel/bin/lldb
> --no-lldbinit -S
> /home/teemperor/work/3llvm/rel/tools/lldb/lit/lit-lldb-init
> /home/teemperor/work/3llvm/rel/tools/lldb/lit/Symbol
> File/DWARF/Output/dir-separator-no-comp-dir.s.tmp -s
>
> /home/teemperor/work/3llvm/llvm/lldb/lit/SymbolFile/DWARF/Inputs/dir-separator-windows.lldbinit
> -o exit | /home/teemperor/work/3llvm/rel/b
> in/FileCheck
> /home/teemperor/work/3llvm/llvm/lldb/lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s
> --
> Exit Code: 1
>
> Command Output (stderr):
> --
> ld.lld: error: unknown -z value: separate-code
>
> --
>
> > ld.lld -v
> LLD 8.0.1 (compatible with GNU linkers)
>
> Do I need 9.0.0 for this to pass?
>
> Am Do., 1. Aug. 2019 um 14:38 Uhr schrieb Pavel Labath via
> lldb-commits :
> >
> > On 01/08/2019 14:34, Fangrui Song via lldb-commits wrote:
> > > Author: maskray
> > > Date: Thu Aug  1 05:34:43 2019
> > > New Revision: 367549
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=367549=rev
> > > Log:
> > > [lit] Use ld.lld -z separate-code to work around a debug_line parsing
> bug
> > >
> > > The issue was exposed by D64903/r367537.
> > > http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/7321/
> > >
> > > In these tests, .debug_str immediately follows .text.
> > > The last section of last RX PT_LOAD was originally padded with trap
> > > instructions and .debug_str started at a new page (actually
> > > common-page-size). Now, .debug_str immediately follows .test.
> > > Add -z separate-code to use the old layout.
> > >
> >
> >
> > Thanks, I was about to commit the same patch.
> >
> > For future reference, here is my analysis I was going to put into the
> > commit message:
> >
> > ---
> > The lld patch changed how it lays out data in segments, which was
> > resulted in the tiny test funclets being at the very end of a PT_LOAD
> > segment. This exposed a bug where lldb does not correctly handle line
> > table end-of-sequence entries to the very end of a section (so the byte
> > they point to does not actually belong to any section).
> >
> > This is mostly bening in practice, as the code handling line tables will
> > terminate as soon as it is not able to resolve an address, which is
> > something that would happen after reading the end-of-sequence entry
> > anyway. However, these tests assert that we are actually able to parse
> > and display the end entry itself, and so they fail.
> > ---
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>


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


[Lldb-commits] [PATCH] D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

2019-08-01 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: source/Symbol/ClangASTContext.cpp:2229
 clang::DeclContext *decl_ctx, const char *name,
-const CompilerType _type, int storage) {
+const CompilerType _type, int storage, bool add_decl) {
   ASTContext *ast = getASTContext();

Please make sure to document why this flag is necessary and especially, what 
needs to be fixed in order to make it unnecessary again.


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

https://reviews.llvm.org/D65414



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


[Lldb-commits] [PATCH] D65489: Format OptionEnumValueElement

2019-08-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367638: Format OptionEnumValueElement (NFC) (authored by 
JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65489?vs=212892=212937#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65489

Files:
  lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/trunk/source/Commands/CommandObjectCommands.cpp
  lldb/trunk/source/Commands/CommandObjectExpression.cpp
  lldb/trunk/source/Commands/CommandObjectTarget.cpp
  lldb/trunk/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/trunk/source/Core/Debugger.cpp
  lldb/trunk/source/Interpreter/OptionGroupWatchpoint.cpp
  
lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  lldb/trunk/source/Target/Target.cpp
  lldb/trunk/source/Target/TargetProperties.td

Index: lldb/trunk/source/Interpreter/OptionGroupWatchpoint.cpp
===
--- lldb/trunk/source/Interpreter/OptionGroupWatchpoint.cpp
+++ lldb/trunk/source/Interpreter/OptionGroupWatchpoint.cpp
@@ -16,16 +16,45 @@
 using namespace lldb_private;
 
 static constexpr OptionEnumValueElement g_watch_type[] = {
-{OptionGroupWatchpoint::eWatchRead, "read", "Watch for read"},
-{OptionGroupWatchpoint::eWatchWrite, "write", "Watch for write"},
-{OptionGroupWatchpoint::eWatchReadWrite, "read_write",
- "Watch for read/write"} };
+{
+OptionGroupWatchpoint::eWatchRead,
+"read",
+"Watch for read",
+},
+{
+OptionGroupWatchpoint::eWatchWrite,
+"write",
+"Watch for write",
+},
+{
+OptionGroupWatchpoint::eWatchReadWrite,
+"read_write",
+"Watch for read/write",
+},
+};
 
 static constexpr OptionEnumValueElement g_watch_size[] = {
-{1, "1", "Watch for byte size of 1"},
-{2, "2", "Watch for byte size of 2"},
-{4, "4", "Watch for byte size of 4"},
-{8, "8", "Watch for byte size of 8"} };
+{
+1,
+"1",
+"Watch for byte size of 1",
+},
+{
+2,
+"2",
+"Watch for byte size of 2",
+},
+{
+4,
+"4",
+"Watch for byte size of 4",
+},
+{
+8,
+"8",
+"Watch for byte size of 8",
+},
+};
 
 static constexpr OptionDefinition g_option_table[] = {
 {LLDB_OPT_SET_1, false, "watch", 'w', OptionParser::eRequiredArgument,
Index: lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
===
--- lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
+++ lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
@@ -57,12 +57,24 @@
   eEnableJITLoaderGDBOff,
 };
 
-static constexpr OptionEnumValueElement g_enable_jit_loader_gdb_enumerators[] = {
-{eEnableJITLoaderGDBDefault, "default", "Enable JIT compilation interface "
- "for all platforms except macOS"},
-{eEnableJITLoaderGDBOn, "on", "Enable JIT compilation interface"},
-{eEnableJITLoaderGDBOff, "off", "Disable JIT compilation interface"}
- };
+static constexpr OptionEnumValueElement g_enable_jit_loader_gdb_enumerators[] =
+{
+{
+eEnableJITLoaderGDBDefault,
+"default",
+"Enable JIT compilation interface for all platforms except macOS",
+},
+{
+eEnableJITLoaderGDBOn,
+"on",
+"Enable JIT compilation interface",
+},
+{
+eEnableJITLoaderGDBOff,
+"off",
+"Disable JIT compilation interface",
+},
+};
 
 #define LLDB_PROPERTIES_jitloadergdb
 #include "JITLoaderGDBProperties.inc"
Index: lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===
--- lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -31,8 +31,8 @@
 
 #include "DynamicLoaderDarwinKernel.h"
 
-#include 
 #include 
+#include 
 
 //#define ENABLE_DEBUG_PRINTF // COMMENT THIS LINE OUT PRIOR TO CHECKIN
 #ifdef ENABLE_DEBUG_PRINTF
@@ -61,16 +61,30 @@
 };
 
 static constexpr OptionEnumValueElement g_kaslr_kernel_scan_enum_values[] = {
-{eKASLRScanNone, "none",
- "Do not read memory looking for a Darwin kernel when attaching."},
-{eKASLRScanLowgloAddresses, "basic", "Check for the Darwin kernel's load "
- "addr in the lowglo page "
- "(boot-args=debug) only."},
-{eKASLRScanNearPC, "fast-scan", "Scan near the pc value on attach to find "

[Lldb-commits] [lldb] r367638 - Format OptionEnumValueElement (NFC)

2019-08-01 Thread Jonas Devlieghere via lldb-commits
Author: jdevlieghere
Date: Thu Aug  1 17:18:44 2019
New Revision: 367638

URL: http://llvm.org/viewvc/llvm-project?rev=367638=rev
Log:
Format OptionEnumValueElement (NFC)

Reformat OptionEnumValueElement to make it easier to distinguish between
its fields. This also removes the need to disable clang-format for these
arrays.

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

Modified:
lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp
lldb/trunk/source/Commands/CommandObjectCommands.cpp
lldb/trunk/source/Commands/CommandObjectExpression.cpp
lldb/trunk/source/Commands/CommandObjectTarget.cpp
lldb/trunk/source/Commands/CommandObjectWatchpointCommand.cpp
lldb/trunk/source/Core/Debugger.cpp
lldb/trunk/source/Interpreter/OptionGroupWatchpoint.cpp

lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
lldb/trunk/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
lldb/trunk/source/Target/Target.cpp
lldb/trunk/source/Target/TargetProperties.td

Modified: lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp?rev=367638=367637=367638=diff
==
--- lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectBreakpointCommand.cpp Thu Aug  1 
17:18:44 2019
@@ -26,19 +26,26 @@
 using namespace lldb;
 using namespace lldb_private;
 
-// CommandObjectBreakpointCommandAdd
-
 // FIXME: "script-type" needs to have its contents determined dynamically, so
-// somebody can add a new scripting
-// language to lldb and have it pickable here without having to change this
-// enumeration by hand and rebuild lldb proper.
-
+// somebody can add a new scripting language to lldb and have it pickable here
+// without having to change this enumeration by hand and rebuild lldb proper.
 static constexpr OptionEnumValueElement g_script_option_enumeration[] = {
-{eScriptLanguageNone, "command",
- "Commands are in the lldb command interpreter language"},
-{eScriptLanguagePython, "python", "Commands are in the Python language."},
-{eSortOrderByName, "default-script",
- "Commands are in the default scripting language."} };
+{
+eScriptLanguageNone,
+"command",
+"Commands are in the lldb command interpreter language",
+},
+{
+eScriptLanguagePython,
+"python",
+"Commands are in the Python language.",
+},
+{
+eSortOrderByName,
+"default-script",
+"Commands are in the default scripting language.",
+},
+};
 
 static constexpr OptionEnumValues ScriptOptionEnum() {
   return OptionEnumValues(g_script_option_enumeration);
@@ -558,7 +565,7 @@ protected:
 
 BreakpointIDList valid_bp_ids;
 CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
-command, target, result, _bp_ids, 
+command, target, result, _bp_ids,
 BreakpointName::Permissions::PermissionKinds::listPerm);
 
 if (result.Succeeded()) {
@@ -648,7 +655,7 @@ protected:
 
 BreakpointIDList valid_bp_ids;
 CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
-command, target, result, _bp_ids, 
+command, target, result, _bp_ids,
 BreakpointName::Permissions::PermissionKinds::listPerm);
 
 if (result.Succeeded()) {

Modified: lldb/trunk/source/Commands/CommandObjectCommands.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectCommands.cpp?rev=367638=367637=367638=diff
==
--- lldb/trunk/source/Commands/CommandObjectCommands.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectCommands.cpp Thu Aug  1 17:18:44 
2019
@@ -1486,12 +1486,22 @@ protected:
 
 // CommandObjectCommandsScriptAdd
 static constexpr OptionEnumValueElement g_script_synchro_type[] = {
-  {eScriptedCommandSynchronicitySynchronous, "synchronous",
-   "Run synchronous"},
-  {eScriptedCommandSynchronicityAsynchronous, "asynchronous",
-   "Run asynchronous"},
-  {eScriptedCommandSynchronicityCurrentValue, "current",
-   "Do not alter current setting"} };
+{
+eScriptedCommandSynchronicitySynchronous,
+"synchronous",
+"Run synchronous",
+},
+{
+eScriptedCommandSynchronicityAsynchronous,
+"asynchronous",
+"Run asynchronous",
+},
+{
+eScriptedCommandSynchronicityCurrentValue,
+"current",
+"Do not alter current setting",
+},
+};
 
 static constexpr OptionEnumValues ScriptSynchroType() {
   return OptionEnumValues(g_script_synchro_type);

Modified: lldb/trunk/source/Commands/CommandObjectExpression.cpp
URL: 

[Lldb-commits] [PATCH] D65489: Format OptionEnumValueElement

2019-08-01 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

Okay, if clang-format won't undo this, then I'm fine with writing them this way.


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

https://reviews.llvm.org/D65489



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


[Lldb-commits] [PATCH] D65611: [Driver] Expand the target in the driver.

2019-08-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 212924.

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

https://reviews.llvm.org/D65611

Files:
  lldb/lit/Driver/TestTarget.test
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -533,14 +533,19 @@
   if (!m_option_data.m_repl) {
 const size_t num_args = m_option_data.m_args.size();
 if (num_args > 0) {
+  char target_path[256];
+  SBFileSpec::ResolvePath(m_option_data.m_args[0].c_str(), target_path,
+  sizeof(target_path));
+
   char arch_name[64];
   if (lldb::SBDebugger::GetDefaultArchitecture(arch_name,
-   sizeof(arch_name)))
+   sizeof(arch_name))) {
 commands_stream.Printf("target create --arch=%s %s", arch_name,
-   EscapeString(m_option_data.m_args[0]).c_str());
-  else
+   EscapeString(target_path).c_str());
+  } else {
 commands_stream.Printf("target create %s",
-   EscapeString(m_option_data.m_args[0]).c_str());
+   EscapeString(target_path).c_str());
+  }
 
   if (!m_option_data.m_core_file.empty()) {
 commands_stream.Printf(" --core %s",
Index: lldb/lit/Driver/TestTarget.test
===
--- /dev/null
+++ lldb/lit/Driver/TestTarget.test
@@ -0,0 +1,7 @@
+# Make sure lldb resolves the target path.
+# RUN: mkdir -p %t/foo
+# RUN: cd %t/foo
+# RUN: %clang %p/Inputs/hello.c -g -o a.out
+# RUN: %lldb -b a.out | FileCheck %s
+
+# CHECK: target create "{{.*}}foo{{[/]+}}a.out"


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -533,14 +533,19 @@
   if (!m_option_data.m_repl) {
 const size_t num_args = m_option_data.m_args.size();
 if (num_args > 0) {
+  char target_path[256];
+  SBFileSpec::ResolvePath(m_option_data.m_args[0].c_str(), target_path,
+  sizeof(target_path));
+
   char arch_name[64];
   if (lldb::SBDebugger::GetDefaultArchitecture(arch_name,
-   sizeof(arch_name)))
+   sizeof(arch_name))) {
 commands_stream.Printf("target create --arch=%s %s", arch_name,
-   EscapeString(m_option_data.m_args[0]).c_str());
-  else
+   EscapeString(target_path).c_str());
+  } else {
 commands_stream.Printf("target create %s",
-   EscapeString(m_option_data.m_args[0]).c_str());
+   EscapeString(target_path).c_str());
+  }
 
   if (!m_option_data.m_core_file.empty()) {
 commands_stream.Printf(" --core %s",
Index: lldb/lit/Driver/TestTarget.test
===
--- /dev/null
+++ lldb/lit/Driver/TestTarget.test
@@ -0,0 +1,7 @@
+# Make sure lldb resolves the target path.
+# RUN: mkdir -p %t/foo
+# RUN: cd %t/foo
+# RUN: %clang %p/Inputs/hello.c -g -o a.out
+# RUN: %lldb -b a.out | FileCheck %s
+
+# CHECK: target create "{{.*}}foo{{[/]+}}a.out"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D65611: [Driver] Expand the target in the driver.

2019-08-01 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

IIRC both GetPath and ResolvePath return the number of characters it would take 
to actually resolve the path.  128 is actually pretty short, so you should 
check the result and malloc a buffer of the return + 1 if 128 ends up not being 
enough.


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

https://reviews.llvm.org/D65611



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


[Lldb-commits] [PATCH] D65611: [Driver] Expand the target in the driver.

2019-08-01 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

If you just want the path and don't need the FileSpec, you can call the static 
SBFileSpec::ResolvePath.


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

https://reviews.llvm.org/D65611



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


[Lldb-commits] [PATCH] D55571: [ast] CreateParameterDeclaration should use an appropriate DeclContext.

2019-08-01 Thread Stella Stamenova via Phabricator via lldb-commits
stella.stamenova added a comment.
Herald added a project: LLDB.

Test passes now. Thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55571



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


[Lldb-commits] [PATCH] D65611: [Driver] Expand the target in the driver.

2019-08-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 212916.

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

https://reviews.llvm.org/D65611

Files:
  lldb/lit/Driver/TestTarget.test
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -533,14 +533,22 @@
   if (!m_option_data.m_repl) {
 const size_t num_args = m_option_data.m_args.size();
 if (num_args > 0) {
+  char target_path[128];
   char arch_name[64];
+
+  // Resolve target.
+  SBFileSpec target(m_option_data.m_args[0].c_str());
+  target.ResolveExecutableLocation();
+  target.GetPath(target_path, sizeof(target_path));
+
   if (lldb::SBDebugger::GetDefaultArchitecture(arch_name,
-   sizeof(arch_name)))
+   sizeof(arch_name))) {
 commands_stream.Printf("target create --arch=%s %s", arch_name,
-   EscapeString(m_option_data.m_args[0]).c_str());
-  else
+   EscapeString(target_path).c_str());
+  } else {
 commands_stream.Printf("target create %s",
-   EscapeString(m_option_data.m_args[0]).c_str());
+   EscapeString(target_path).c_str());
+  }
 
   if (!m_option_data.m_core_file.empty()) {
 commands_stream.Printf(" --core %s",
Index: lldb/lit/Driver/TestTarget.test
===
--- /dev/null
+++ lldb/lit/Driver/TestTarget.test
@@ -0,0 +1,7 @@
+# Make sure lldb resolves the target path.
+# RUN: mkdir -p %t/foo
+# RUN: cd %t/foo
+# RUN: %clang %p/Inputs/hello.c -g -o a.out
+# RUN: %lldb -b a.out | FileCheck %s
+
+# CHECK: target create "{{.*}}foo{{[/]+}}a.out"


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -533,14 +533,22 @@
   if (!m_option_data.m_repl) {
 const size_t num_args = m_option_data.m_args.size();
 if (num_args > 0) {
+  char target_path[128];
   char arch_name[64];
+
+  // Resolve target.
+  SBFileSpec target(m_option_data.m_args[0].c_str());
+  target.ResolveExecutableLocation();
+  target.GetPath(target_path, sizeof(target_path));
+
   if (lldb::SBDebugger::GetDefaultArchitecture(arch_name,
-   sizeof(arch_name)))
+   sizeof(arch_name))) {
 commands_stream.Printf("target create --arch=%s %s", arch_name,
-   EscapeString(m_option_data.m_args[0]).c_str());
-  else
+   EscapeString(target_path).c_str());
+  } else {
 commands_stream.Printf("target create %s",
-   EscapeString(m_option_data.m_args[0]).c_str());
+   EscapeString(target_path).c_str());
+  }
 
   if (!m_option_data.m_core_file.empty()) {
 commands_stream.Printf(" --core %s",
Index: lldb/lit/Driver/TestTarget.test
===
--- /dev/null
+++ lldb/lit/Driver/TestTarget.test
@@ -0,0 +1,7 @@
+# Make sure lldb resolves the target path.
+# RUN: mkdir -p %t/foo
+# RUN: cd %t/foo
+# RUN: %clang %p/Inputs/hello.c -g -o a.out
+# RUN: %lldb -b a.out | FileCheck %s
+
+# CHECK: target create "{{.*}}foo{{[/]+}}a.out"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D65611: [Driver] Expand the target in the driver.

2019-08-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: davide, labath, jingham, clayborg, mgorny.
JDevlieghere added a project: LLDB.
Herald added a subscriber: teemperor.

When invoking lldb on the command line with a binary, lldb will construct a 
command stream to create the target.

  $ lldb ./foo
  (lldb) target create "./foo"
  Current executable set to './foo' (x86_64).

This poses a problem for reproducers when the path to the binary is a relative 
path. When the reproducer is replayed form a different location, the path gets 
resolved to a different absolute path, unknown to the VFS.

This patch changes the behavior of the driver to resolve the target's path 
before constructing the `target create` command.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D65611

Files:
  lldb/lit/Driver/TestTarget.test
  
lldb/packages/Python/lldbsuite/test/lang/objc/objc-dynamic-value/TestObjCDynamicValue.py
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -533,14 +533,22 @@
   if (!m_option_data.m_repl) {
 const size_t num_args = m_option_data.m_args.size();
 if (num_args > 0) {
+  char target_path[128];
   char arch_name[64];
+
+  // Resolve target.
+  SBFileSpec target(m_option_data.m_args[0].c_str());
+  target.ResolveExecutableLocation();
+  target.GetPath(target_path, sizeof(target_path));
+
   if (lldb::SBDebugger::GetDefaultArchitecture(arch_name,
-   sizeof(arch_name)))
+   sizeof(arch_name))) {
 commands_stream.Printf("target create --arch=%s %s", arch_name,
-   EscapeString(m_option_data.m_args[0]).c_str());
-  else
+   EscapeString(target_path).c_str());
+  } else {
 commands_stream.Printf("target create %s",
-   EscapeString(m_option_data.m_args[0]).c_str());
+   EscapeString(target_path).c_str());
+  }
 
   if (!m_option_data.m_core_file.empty()) {
 commands_stream.Printf(" --core %s",
Index: 
lldb/packages/Python/lldbsuite/test/lang/objc/objc-dynamic-value/TestObjCDynamicValue.py
===
--- 
lldb/packages/Python/lldbsuite/test/lang/objc/objc-dynamic-value/TestObjCDynamicValue.py
+++ 
lldb/packages/Python/lldbsuite/test/lang/objc/objc-dynamic-value/TestObjCDynamicValue.py
@@ -35,7 +35,6 @@
 
 @skipUnlessDarwin
 @add_test_categories(['pyapi'])
-@expectedFailureDarwin("llvm.org/pr20271 rdar://18684107")
 def test_get_objc_dynamic_vals(self):
 """Test fetching ObjC dynamic values."""
 if self.getArchitecture() == 'i386':
Index: lldb/lit/Driver/TestTarget.test
===
--- /dev/null
+++ lldb/lit/Driver/TestTarget.test
@@ -0,0 +1,7 @@
+# Make sure lldb resolves the target path.
+# RUN: mkdir -p %t/foo
+# RUN: cd %t/foo
+# RUN: %clang %p/Inputs/hello.c -g -o a.out
+# RUN: %lldb -b a.out | FileCheck %s
+
+# CHECK: target create "{{.*}}foo{{[/]+}}a.out"


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -533,14 +533,22 @@
   if (!m_option_data.m_repl) {
 const size_t num_args = m_option_data.m_args.size();
 if (num_args > 0) {
+  char target_path[128];
   char arch_name[64];
+
+  // Resolve target.
+  SBFileSpec target(m_option_data.m_args[0].c_str());
+  target.ResolveExecutableLocation();
+  target.GetPath(target_path, sizeof(target_path));
+
   if (lldb::SBDebugger::GetDefaultArchitecture(arch_name,
-   sizeof(arch_name)))
+   sizeof(arch_name))) {
 commands_stream.Printf("target create --arch=%s %s", arch_name,
-   EscapeString(m_option_data.m_args[0]).c_str());
-  else
+   EscapeString(target_path).c_str());
+  } else {
 commands_stream.Printf("target create %s",
-   EscapeString(m_option_data.m_args[0]).c_str());
+   EscapeString(target_path).c_str());
+  }
 
   if (!m_option_data.m_core_file.empty()) {
 commands_stream.Printf(" --core %s",
Index: lldb/packages/Python/lldbsuite/test/lang/objc/objc-dynamic-value/TestObjCDynamicValue.py
===
--- lldb/packages/Python/lldbsuite/test/lang/objc/objc-dynamic-value/TestObjCDynamicValue.py
+++ lldb/packages/Python/lldbsuite/test/lang/objc/objc-dynamic-value/TestObjCDynamicValue.py
@@ -35,7 +35,6 

[Lldb-commits] [PATCH] D65489: Format OptionEnumValueElement

2019-08-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In D65489#1611165 , @jingham wrote:

> What happens if you clang-format your reformatted version of the setter?  Do 
> we need clang-format off for them?
>
> Otherwise this looks fine to me.


This is the clang-format output!


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

https://reviews.llvm.org/D65489



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


[Lldb-commits] [PATCH] D65489: Format OptionEnumValueElement

2019-08-01 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

What happens if you clang-format your reformatted version of the setter?  Do we 
need clang-format off for them?

Otherwise this looks fine to me.


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

https://reviews.llvm.org/D65489



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


[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

2019-08-01 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.

This looks good, this is in line with what we discussed, thanks for taking it 
on!  Sorry for the delay at looking this over, it has been a little crazy this 
week.




Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:1760
+void RegisterContextLLDB::PropagateTrapHandlerFlag(
+lldb::UnwindPlanSP unwind_plan) {
+  if (unwind_plan->GetUnwindPlanForSignalTrap() != eLazyBoolYes) {

clayborg wrote:
> JosephTremoulet wrote:
> > I'm a bit ambivalent about adding this part -- the downside is that it's 
> > not concretely helping today, because if an eh_frame record has the 'S' 
> > flag in its augmentation (which is what 
> > `unwind_Plan->GetUnwindPlanForSignalTrap()` reports), we'll have already 
> > decremented pc and generated unwind plans based on the prior function when 
> > initializing the register context.  But the upside is that this connects 
> > the dots between the otherwise-unused bit on the unwind plan and the frame 
> > type, and will be in place should we subsequently add code to try the 
> > second function's unwind plan as a fallback.
> I will let Jason comment on this one.
Yeah, this was my impression of the S augmentation flag in the eh_frame too -- 
we can't really use it in lldb today without forcing a scan of eh_frame entries 
the first time we unwind a function from that Module, and that would be 
unfortunate.  But I like to see the flag being parsed and recorded; at some 
point in the future we may find a good way to use it.



Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:1744
+// frame may in fact be the first instruction of a signal return
+// trampoline, rather than the instruction after a call.
+UnwindLogMsg("Resetting current offset and re-doing symbol lookup; "

This is more a personal preference thing, but I would explicitly call out that 
we're addressing a problem on a platform where a trap handler is "added" to the 
stack by the kernel, but it hasn't actually been called and executed the way a 
normal function is, the PC is sitting on the first instruction of the function, 
so backing the pc up by 1 points us to the wrong function.

The kernel is presenting us with a stack frame that doesn't follow the normal 
ABI and execution models, and we're hacking around that.  It's perfectly OK to 
do that, but it's a specific enough thing we're addressing here, I think it'll 
be helpful to Future Us to name it explicitly.



Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.h:125
+  /// update frame type and symbol context if so.
+  void PropagateTrapHandlerFlag(lldb::UnwindPlanSP unwind_plan);
+

This is very much a personal preference, but I might call this something like 
UpdateTrapHandlerFlagFromUnwindPlan.  I'm fine with this name if you find it 
clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64993



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


[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Harlan Haskins via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367618: [clang] Change FileManager to use llvm::ErrorOr 
instead of null on failure (authored by harlanhaskins, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65534?vs=212860=212907#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65534

Files:
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp


Index: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -247,11 +247,11 @@
 
   bool is_system = true;
   bool is_framework = false;
-  auto *dir =
+  auto dir =
   HS.getFileMgr().getDirectory(module.search_path.GetStringRef());
   if (!dir)
 return error();
-  auto *file = HS.lookupModuleMapFile(dir, is_framework);
+  auto *file = HS.lookupModuleMapFile(*dir, is_framework);
   if (!file)
 return error();
   if (!HS.loadModuleMapFile(file, is_system))
Index: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -908,10 +908,13 @@
   if (file.Write(expr_text, bytes_written).Success()) {
 if (bytes_written == expr_text_len) {
   file.Close();
-  source_mgr.setMainFileID(source_mgr.createFileID(
-  m_compiler->getFileManager().getFile(result_path),
-  SourceLocation(), SrcMgr::C_User));
-  created_main_file = true;
+  if (auto fileEntry =
+  m_compiler->getFileManager().getFile(result_path)) {
+source_mgr.setMainFileID(source_mgr.createFileID(
+*fileEntry,
+SourceLocation(), SrcMgr::C_User));
+created_main_file = true;
+  }
 }
   }
 }


Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -247,11 +247,11 @@
 
   bool is_system = true;
   bool is_framework = false;
-  auto *dir =
+  auto dir =
   HS.getFileMgr().getDirectory(module.search_path.GetStringRef());
   if (!dir)
 return error();
-  auto *file = HS.lookupModuleMapFile(dir, is_framework);
+  auto *file = HS.lookupModuleMapFile(*dir, is_framework);
   if (!file)
 return error();
   if (!HS.loadModuleMapFile(file, is_system))
Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -908,10 +908,13 @@
   if (file.Write(expr_text, bytes_written).Success()) {
 if (bytes_written == expr_text_len) {
   file.Close();
-  source_mgr.setMainFileID(source_mgr.createFileID(
-  m_compiler->getFileManager().getFile(result_path),
-  SourceLocation(), SrcMgr::C_User));
-  created_main_file = true;
+  if (auto fileEntry =
+  m_compiler->getFileManager().getFile(result_path)) {
+source_mgr.setMainFileID(source_mgr.createFileID(
+*fileEntry,
+SourceLocation(), SrcMgr::C_User));
+created_main_file = true;
+  }
 }
   }
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D65594: Fix `skipIfSanitized` decorator on macOS

2019-08-01 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

fwiw this behavior is documented here,

https://developer.apple.com/documentation/bundleresources/entitlements/com_apple_security_cs_allow-dyld-environment-variables?language=objc


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65594



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


[Lldb-commits] [PATCH] D65492: Adjust a ValueObjectChild's offset when the child is a bitfield

2019-08-01 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

I'm sure this is fine, I read the code through and I've realized I haven't had 
to do anything with bitfields in lldb up to now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65492



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


Re: [Lldb-commits] [lldb] r367549 - [lit] Use ld.lld -z separate-code to work around a debug_line parsing bug

2019-08-01 Thread Raphael Isemann via lldb-commits
This breaks the tests on my machine:


FAIL: LLDB :: SymbolFile/DWARF/dir-separator-no-comp-dir.s (114 of 1660)
 TEST 'LLDB ::
SymbolFile/DWARF/dir-separator-no-comp-dir.s' FAILED

Script:
--
: 'RUN: at line 6';   /home/teemperor/work/3llvm/rel/bin/llvm-mc
-triple x86_64-pc-linux
/home/teemperor/work/3llvm/llvm/lldb/lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s
-filetype=obj >
/home/teemperor/work/3llvm/rel/tools/lldb/lit/SymbolFile/DWARF/Output/dir-separator-no-comp-dir.s.tmp.o
: 'RUN: at line 7';   /usr/bin/ld.lld -z separate-code
/home/teemperor/work/3llvm/rel/tools/lldb/lit/SymbolFile/DWARF/Output/dir-separator-no-comp-dir.s.tmp.o
-o /home/teemperor/work/3llvm/re
l/tools/lldb/lit/SymbolFile/DWARF/Output/dir-separator-no-comp-dir.s.tmp
: 'RUN: at line 8';   /home/teemperor/work/3llvm/rel/bin/lldb
--no-lldbinit -S
/home/teemperor/work/3llvm/rel/tools/lldb/lit/lit-lldb-init
/home/teemperor/work/3llvm/rel/tools/lldb/lit/Symbol
File/DWARF/Output/dir-separator-no-comp-dir.s.tmp -s
/home/teemperor/work/3llvm/llvm/lldb/lit/SymbolFile/DWARF/Inputs/dir-separator-windows.lldbinit
-o exit | /home/teemperor/work/3llvm/rel/b
in/FileCheck 
/home/teemperor/work/3llvm/llvm/lldb/lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s
--
Exit Code: 1

Command Output (stderr):
--
ld.lld: error: unknown -z value: separate-code

--

> ld.lld -v
LLD 8.0.1 (compatible with GNU linkers)

Do I need 9.0.0 for this to pass?

Am Do., 1. Aug. 2019 um 14:38 Uhr schrieb Pavel Labath via
lldb-commits :
>
> On 01/08/2019 14:34, Fangrui Song via lldb-commits wrote:
> > Author: maskray
> > Date: Thu Aug  1 05:34:43 2019
> > New Revision: 367549
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=367549=rev
> > Log:
> > [lit] Use ld.lld -z separate-code to work around a debug_line parsing bug
> >
> > The issue was exposed by D64903/r367537.
> > http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/7321/
> >
> > In these tests, .debug_str immediately follows .text.
> > The last section of last RX PT_LOAD was originally padded with trap
> > instructions and .debug_str started at a new page (actually
> > common-page-size). Now, .debug_str immediately follows .test.
> > Add -z separate-code to use the old layout.
> >
>
>
> Thanks, I was about to commit the same patch.
>
> For future reference, here is my analysis I was going to put into the
> commit message:
>
> ---
> The lld patch changed how it lays out data in segments, which was
> resulted in the tiny test funclets being at the very end of a PT_LOAD
> segment. This exposed a bug where lldb does not correctly handle line
> table end-of-sequence entries to the very end of a section (so the byte
> they point to does not actually belong to any section).
>
> This is mostly bening in practice, as the code handling line tables will
> terminate as soon as it is not able to resolve an address, which is
> something that would happen after reading the end-of-sequence entry
> anyway. However, these tests assert that we are actually able to parse
> and display the end entry itself, and so they fail.
> ---
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D65566: [lldb][CMake] Workaround debugserver code-signing issue in generated Xcode project

2019-08-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

Is there a way to use the debugserver in the framework instead? This seems like 
the inverse of the problem we had earlier with the RPATHs, where we would fix 
them up in the build tree after copying them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65566



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


[Lldb-commits] [PATCH] D65489: Format OptionEnumValueElement

2019-08-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 212892.
JDevlieghere retitled this revision from "Tablegen  option enum value elements" 
to "Format OptionEnumValueElement".
JDevlieghere edited the summary of this revision.
JDevlieghere added a reviewer: labath.
JDevlieghere added a comment.

Reformat OptionEnumValueElement instead of using tablegen.


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

https://reviews.llvm.org/D65489

Files:
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/OptionGroupWatchpoint.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -129,7 +129,7 @@
 Desc<"Allow LLDB to load scripting resources embedded in symbol files when available.">;
   def LoadCWDlldbinitFile: Property<"load-cwd-lldbinit", "Enum">,
 DefaultEnumValue<"eLoadCWDlldbinitWarn">,
-EnumValues<"OptionEnumValues(g_load_current_working_dir_lldbinit_values)">,
+EnumValues<"OptionEnumValues(g_load_cwd_lldbinit_values)">,
 Desc<"Allow LLDB to .lldbinit files from the current directory automatically.">;
   def MemoryModuleLoadLevel: Property<"memory-module-load-level", "Enum">,
 DefaultEnumValue<"eMemoryModuleLoadLevelComplete">,
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3231,33 +3231,51 @@
   s->SetIndentLevel(indent_level);
 }
 
-// class TargetProperties
-
-// clang-format off
 static constexpr OptionEnumValueElement g_dynamic_value_types[] = {
-{eNoDynamicValues, "no-dynamic-values",
- "Don't calculate the dynamic type of values"},
-{eDynamicCanRunTarget, "run-target", "Calculate the dynamic type of values "
- "even if you have to run the target."},
-{eDynamicDontRunTarget, "no-run-target",
- "Calculate the dynamic type of values, but don't run the target."} };
+{
+eNoDynamicValues,
+"no-dynamic-values",
+"Don't calculate the dynamic type of values",
+},
+{
+eDynamicCanRunTarget,
+"run-target",
+"Calculate the dynamic type of values "
+"even if you have to run the target.",
+},
+{
+eDynamicDontRunTarget,
+"no-run-target",
+"Calculate the dynamic type of values, but don't run the target.",
+},
+};
 
 OptionEnumValues lldb_private::GetDynamicValueTypes() {
   return OptionEnumValues(g_dynamic_value_types);
 }
 
 static constexpr OptionEnumValueElement g_inline_breakpoint_enums[] = {
-{eInlineBreakpointsNever, "never", "Never look for inline breakpoint "
-   "locations (fastest). This setting "
-   "should only be used if you know that "
-   "no inlining occurs in your programs."},
-{eInlineBreakpointsHeaders, "headers",
- "Only check for inline breakpoint locations when setting breakpoints in "
- "header files, but not when setting breakpoint in implementation source "
- "files (default)."},
-{eInlineBreakpointsAlways, "always",
- "Always look for inline breakpoint locations when setting file and line "
- "breakpoints (slower but most accurate)."} };
+{
+eInlineBreakpointsNever,
+"never",
+"Never look for inline breakpoint locations (fastest). This setting "
+"should only be used if you know that no inlining occurs in your"
+"programs.",
+},
+{
+eInlineBreakpointsHeaders,
+"headers",
+"Only check for inline breakpoint locations when setting breakpoints "
+"in header files, but not when setting breakpoint in implementation "
+"source files (default).",
+},
+{
+eInlineBreakpointsAlways,
+"always",
+"Always look for inline breakpoint locations when setting file and "
+"line breakpoints (slower but most accurate).",
+},
+};
 
 enum x86DisassemblyFlavor {
   eX86DisFlavorDefault,
@@ -3266,41 +3284,92 @@
 };
 
 static constexpr OptionEnumValueElement g_x86_dis_flavor_value_types[] = {
-{eX86DisFlavorDefault, "default", "Disassembler default (currently att)."},
-{eX86DisFlavorIntel, "intel", "Intel disassembler flavor."},
-{eX86DisFlavorATT, "att", "AT disassembler flavor."} };
+{
+

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Jan Korous via Phabricator via lldb-commits
jkorous accepted this revision.
jkorous marked an inline comment as done.
jkorous added a comment.
This revision is now accepted and ready to land.

LGTM

Thanks for all the work here!




Comment at: clang/lib/ARCMigrate/FileRemapper.cpp:156
+  auto newE = FileMgr->getFile(tempPath);
+  if (newE) {
+remap(origFE, *newE);

harlanhaskins wrote:
> jkorous wrote:
> > It seems like we're now avoiding the assert in `remap()` we'd hit 
> > previously. Is this fine?
> If we want to trap if this temp file fails, then I'm happy removing the 
> condition check. Do you think this should trap on failure or should it check 
> the condition?
To be clear - I didn't have any opinion, just noticed there's a functional 
change and pointed that out.
On the other hand I assume your solution is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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


[Lldb-commits] [PATCH] D65594: Fix `skipIfSanitized` decorator on macOS

2019-08-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367595: Fix `skipIfSanitized` decorator on macOS (authored 
by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65594?vs=212856=212868#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65594

Files:
  lldb/trunk/lit/Suite/lit.cfg
  lldb/trunk/packages/Python/lldbsuite/test/decorators.py
  lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py
  
lldb/trunk/packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py


Index: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py
@@ -108,7 +108,7 @@
 
 @add_test_categories(['pyapi'])
 @skipIfiOSSimulator
-@skipIfSanitized # FIXME: Hangs indefinitely.
+@skipIfAsan # FIXME: Hangs indefinitely.
 @expectedFailureNetBSD
 def test_with_attach_to_process_with_name_api(self):
 """Create target, spawn a process, and attach to it with process 
name."""
Index: lldb/trunk/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/decorators.py
+++ lldb/trunk/packages/Python/lldbsuite/test/decorators.py
@@ -821,11 +821,10 @@
 return "%s is not supported on this system." % feature
 return skipTestIfFn(is_feature_enabled)
 
-def skipIfSanitized(func):
+def skipIfAsan(func):
 """Skip this test if the environment is set up to run LLDB itself under 
ASAN."""
-def is_sanitized():
-if (('DYLD_INSERT_LIBRARIES' in os.environ) and
-'libclang_rt.asan' in os.environ['DYLD_INSERT_LIBRARIES']):
+def is_asan():
+if ('ASAN_OPTIONS' in os.environ):
 return "ASAN unsupported"
 return None
-return skipTestIfFn(is_sanitized)(func)
+return skipTestIfFn(is_asan)(func)
Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py
@@ -21,7 +21,7 @@
 @expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], 
bugnumber="rdar://problem/34559552") # this exec test has problems on ios 
systems
 @expectedFailureNetBSD
-@skipIfSanitized # rdar://problem/43756823
+@skipIfAsan # rdar://problem/43756823
 @skipIfWindows
 def test_hitting_exec (self):
 self.do_test(False)
@@ -29,7 +29,7 @@
 @expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], 
bugnumber="rdar://problem/34559552") # this exec test has problems on ios 
systems
 @expectedFailureNetBSD
-@skipIfSanitized # rdar://problem/43756823
+@skipIfAsan # rdar://problem/43756823
 @skipIfWindows
 def test_skipping_exec (self):
 self.do_test(True)
Index: lldb/trunk/lit/Suite/lit.cfg
===
--- lldb/trunk/lit/Suite/lit.cfg
+++ lldb/trunk/lit/Suite/lit.cfg
@@ -20,18 +20,16 @@
'Python', 'lldbsuite', 'test')
 config.test_exec_root = config.test_source_root
 
-# macOS flags needed for LLDB built with address sanitizer.
-if 'Address' in config.llvm_use_sanitizer and \
-   'Darwin' in config.host_os and \
-   'x86' in config.host_triple:
-  import subprocess
-  resource_dir = subprocess.check_output(
-config.cmake_cxx_compiler +' -print-resource-dir', shell=True).strip()
-  runtime = os.path.join(resource_dir, 'lib', 'darwin',
- 'libclang_rt.asan_osx_dynamic.dylib')
-  config.environment['ASAN_OPTIONS'] = \
-'detect_stack_use_after_return=1'
-  config.environment['DYLD_INSERT_LIBRARIES'] = runtime
+if 'Address' in config.llvm_use_sanitizer:
+  config.environment['ASAN_OPTIONS'] = 'detect_stack_use_after_return=1'
+  # macOS flags needed for LLDB built with address sanitizer.
+  if 'Darwin' in config.host_os and 'x86' in config.host_triple:
+import subprocess
+resource_dir = subprocess.check_output(
+[config.cmake_cxx_compiler, '-print-resource-dir']).strip()
+runtime = os.path.join(resource_dir, 'lib', 'darwin',
+   'libclang_rt.asan_osx_dynamic.dylib')
+config.environment['DYLD_INSERT_LIBRARIES'] = runtime
 
 # Shared library build of 

[Lldb-commits] [lldb] r367595 - Fix `skipIfSanitized` decorator on macOS

2019-08-01 Thread Jonas Devlieghere via lldb-commits
Author: jdevlieghere
Date: Thu Aug  1 11:35:40 2019
New Revision: 367595

URL: http://llvm.org/viewvc/llvm-project?rev=367595=rev
Log:
Fix `skipIfSanitized` decorator on macOS

For security reasons, DYLD_INSERT_LIBRARIES is not propagated to a child
process. This breaks the skipIfSanitized decorator, which checks for the
environment variable being set. Instead, always set the ASAN_OPTIONS and
make the decorator check for that.

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

Modified:
lldb/trunk/lit/Suite/lit.cfg
lldb/trunk/packages/Python/lldbsuite/test/decorators.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py

Modified: lldb/trunk/lit/Suite/lit.cfg
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lit.cfg?rev=367595=367594=367595=diff
==
--- lldb/trunk/lit/Suite/lit.cfg (original)
+++ lldb/trunk/lit/Suite/lit.cfg Thu Aug  1 11:35:40 2019
@@ -20,18 +20,16 @@ config.test_source_root = os.path.join(c
'Python', 'lldbsuite', 'test')
 config.test_exec_root = config.test_source_root
 
-# macOS flags needed for LLDB built with address sanitizer.
-if 'Address' in config.llvm_use_sanitizer and \
-   'Darwin' in config.host_os and \
-   'x86' in config.host_triple:
-  import subprocess
-  resource_dir = subprocess.check_output(
-config.cmake_cxx_compiler +' -print-resource-dir', shell=True).strip()
-  runtime = os.path.join(resource_dir, 'lib', 'darwin',
- 'libclang_rt.asan_osx_dynamic.dylib')
-  config.environment['ASAN_OPTIONS'] = \
-'detect_stack_use_after_return=1'
-  config.environment['DYLD_INSERT_LIBRARIES'] = runtime
+if 'Address' in config.llvm_use_sanitizer:
+  config.environment['ASAN_OPTIONS'] = 'detect_stack_use_after_return=1'
+  # macOS flags needed for LLDB built with address sanitizer.
+  if 'Darwin' in config.host_os and 'x86' in config.host_triple:
+import subprocess
+resource_dir = subprocess.check_output(
+[config.cmake_cxx_compiler, '-print-resource-dir']).strip()
+runtime = os.path.join(resource_dir, 'lib', 'darwin',
+   'libclang_rt.asan_osx_dynamic.dylib')
+config.environment['DYLD_INSERT_LIBRARIES'] = runtime
 
 # Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
 def find_shlibpath_var():

Modified: lldb/trunk/packages/Python/lldbsuite/test/decorators.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/decorators.py?rev=367595=367594=367595=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/decorators.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/decorators.py Thu Aug  1 11:35:40 
2019
@@ -821,11 +821,10 @@ def skipUnlessFeature(feature):
 return "%s is not supported on this system." % feature
 return skipTestIfFn(is_feature_enabled)
 
-def skipIfSanitized(func):
+def skipIfAsan(func):
 """Skip this test if the environment is set up to run LLDB itself under 
ASAN."""
-def is_sanitized():
-if (('DYLD_INSERT_LIBRARIES' in os.environ) and
-'libclang_rt.asan' in os.environ['DYLD_INSERT_LIBRARIES']):
+def is_asan():
+if ('ASAN_OPTIONS' in os.environ):
 return "ASAN unsupported"
 return None
-return skipTestIfFn(is_sanitized)(func)
+return skipTestIfFn(is_asan)(func)

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py?rev=367595=367594=367595=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py 
Thu Aug  1 11:35:40 2019
@@ -21,7 +21,7 @@ class ExecTestCase(TestBase):
 @expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], 
bugnumber="rdar://problem/34559552") # this exec test has problems on ios 
systems
 @expectedFailureNetBSD
-@skipIfSanitized # rdar://problem/43756823
+@skipIfAsan # rdar://problem/43756823
 @skipIfWindows
 def test_hitting_exec (self):
 self.do_test(False)
@@ -29,7 +29,7 @@ class ExecTestCase(TestBase):
 @expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], 
bugnumber="rdar://problem/34559552") # this exec test has problems on ios 
systems
 @expectedFailureNetBSD
-@skipIfSanitized # rdar://problem/43756823
+@skipIfAsan # 

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Harlan Haskins via Phabricator via lldb-commits
harlanhaskins updated this revision to Diff 212860.
harlanhaskins marked 8 inline comments as done.
harlanhaskins added a comment.

Updated in response to feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp
  clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
  clang-tools-extra/clang-move/Move.cpp
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/clang-reorder-fields/tool/ClangReorderFields.cpp
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/modularize/ModularizeUtilities.cpp
  clang/include/clang/Basic/FileManager.h
  clang/lib/ARCMigrate/FileRemapper.cpp
  clang/lib/ARCMigrate/ObjCMT.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/Module.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/lib/Frontend/Rewrite/FrontendActions.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/lib/Lex/HeaderMap.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/lib/Serialization/ModuleManager.cpp
  clang/lib/Tooling/Core/Replacement.cpp
  clang/lib/Tooling/Refactoring.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/tools/clang-import-test/clang-import-test.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp
  clang/tools/clang-refactor/TestSupport.cpp
  clang/tools/clang-rename/ClangRename.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/Indexing.cpp
  clang/unittests/Basic/FileManagerTest.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp
  clang/unittests/Tooling/RefactoringTest.cpp
  clang/unittests/Tooling/RewriterTestContext.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -247,11 +247,11 @@
 
   bool is_system = true;
   bool is_framework = false;
-  auto *dir =
+  auto dir =
   HS.getFileMgr().getDirectory(module.search_path.GetStringRef());
   if (!dir)
 return error();
-  auto *file = HS.lookupModuleMapFile(dir, is_framework);
+  auto *file = HS.lookupModuleMapFile(*dir, is_framework);
   if (!file)
 return error();
   if (!HS.loadModuleMapFile(file, is_system))
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -908,10 +908,13 @@
   if (file.Write(expr_text, bytes_written).Success()) {
 if (bytes_written == expr_text_len) {
   file.Close();
-  source_mgr.setMainFileID(source_mgr.createFileID(
-  m_compiler->getFileManager().getFile(result_path),
-  SourceLocation(), SrcMgr::C_User));
-  created_main_file = true;
+  if (auto fileEntry =
+  m_compiler->getFileManager().getFile(result_path)) {
+source_mgr.setMainFileID(source_mgr.createFileID(
+*fileEntry,
+SourceLocation(), SrcMgr::C_User));
+created_main_file = true;
+  }
 }
   }
 }
Index: clang/unittests/Tooling/RewriterTestContext.h
===
--- clang/unittests/Tooling/RewriterTestContext.h
+++ clang/unittests/Tooling/RewriterTestContext.h
@@ -56,9 +56,9 @@
 llvm::MemoryBuffer::getMemBuffer(Content);
 InMemoryFileSystem->addFile(Name, 0, std::move(Source));
 
-const FileEntry *Entry = Files.getFile(Name);
-assert(Entry != nullptr);
-return Sources.createFileID(Entry, SourceLocation(), SrcMgr::C_User);
+auto Entry = Files.getFile(Name);
+assert(Entry);
+return Sources.createFileID(*Entry, SourceLocation(), SrcMgr::C_User);
   }
 
   // FIXME: this code is mostly a duplicate of
@@ -73,14 +73,14 @@
  

[Lldb-commits] [PATCH] D65594: Fix `skipIfSanitized` decorator on macOS

2019-08-01 Thread Vedant Kumar via Phabricator via lldb-commits
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Thanks, lgtm.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65594



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


[Lldb-commits] [PATCH] D65594: Fix `skipIfSanitized` decorator on macOS

2019-08-01 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: davide, vsk.
Herald added a subscriber: teemperor.
Herald added a project: LLDB.

For security reasons, `DYLD_INSERT_LIBRARIES` is not propagated to a child 
process. This breaks the `skipIfSanitized` decorator, which checks for the 
environment variable being set. Instead, always set the `ASAN_OPTIONS` and make 
the decorator check for that.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D65594

Files:
  lldb/lit/Suite/lit.cfg
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py
  lldb/packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py


Index: 
lldb/packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py
+++ lldb/packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py
@@ -108,7 +108,7 @@
 
 @add_test_categories(['pyapi'])
 @skipIfiOSSimulator
-@skipIfSanitized # FIXME: Hangs indefinitely.
+@skipIfAsan # FIXME: Hangs indefinitely.
 @expectedFailureNetBSD
 def test_with_attach_to_process_with_name_api(self):
 """Create target, spawn a process, and attach to it with process 
name."""
Index: lldb/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py
@@ -21,7 +21,7 @@
 @expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], 
bugnumber="rdar://problem/34559552") # this exec test has problems on ios 
systems
 @expectedFailureNetBSD
-@skipIfSanitized # rdar://problem/43756823
+@skipIfAsan # rdar://problem/43756823
 @skipIfWindows
 def test_hitting_exec (self):
 self.do_test(False)
@@ -29,7 +29,7 @@
 @expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], 
bugnumber="rdar://problem/34559552") # this exec test has problems on ios 
systems
 @expectedFailureNetBSD
-@skipIfSanitized # rdar://problem/43756823
+@skipIfAsan # rdar://problem/43756823
 @skipIfWindows
 def test_skipping_exec (self):
 self.do_test(True)
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -821,11 +821,10 @@
 return "%s is not supported on this system." % feature
 return skipTestIfFn(is_feature_enabled)
 
-def skipIfSanitized(func):
+def skipIfAsan(func):
 """Skip this test if the environment is set up to run LLDB itself under 
ASAN."""
-def is_sanitized():
-if (('DYLD_INSERT_LIBRARIES' in os.environ) and
-'libclang_rt.asan' in os.environ['DYLD_INSERT_LIBRARIES']):
+def is_asan():
+if ('ASAN_OPTIONS' in os.environ):
 return "ASAN unsupported"
 return None
-return skipTestIfFn(is_sanitized)(func)
+return skipTestIfFn(is_asan)(func)
Index: lldb/lit/Suite/lit.cfg
===
--- lldb/lit/Suite/lit.cfg
+++ lldb/lit/Suite/lit.cfg
@@ -20,18 +20,16 @@
'Python', 'lldbsuite', 'test')
 config.test_exec_root = config.test_source_root
 
-# macOS flags needed for LLDB built with address sanitizer.
-if 'Address' in config.llvm_use_sanitizer and \
-   'Darwin' in config.host_os and \
-   'x86' in config.host_triple:
-  import subprocess
-  resource_dir = subprocess.check_output(
-config.cmake_cxx_compiler +' -print-resource-dir', shell=True).strip()
-  runtime = os.path.join(resource_dir, 'lib', 'darwin',
- 'libclang_rt.asan_osx_dynamic.dylib')
-  config.environment['ASAN_OPTIONS'] = \
-'detect_stack_use_after_return=1'
-  config.environment['DYLD_INSERT_LIBRARIES'] = runtime
+if 'Address' in config.llvm_use_sanitizer:
+  config.environment['ASAN_OPTIONS'] = 'detect_stack_use_after_return=1'
+  # macOS flags needed for LLDB built with address sanitizer.
+  if 'Darwin' in config.host_os and 'x86' in config.host_triple:
+import subprocess
+resource_dir = subprocess.check_output(
+[config.cmake_cxx_compiler, '-print-resource-dir']).strip()
+runtime = os.path.join(resource_dir, 'lib', 'darwin',
+   'libclang_rt.asan_osx_dynamic.dylib')
+config.environment['DYLD_INSERT_LIBRARIES'] = runtime
 
 # Shared library build of LLVM may require LD_LIBRARY_PATH or equivalent.
 def find_shlibpath_var():


Index: 

[Lldb-commits] [PATCH] D65492: Adjust a ValueObjectChild's offset when the child is a bitfield

2019-08-01 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65492



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


[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Nico Weber via Phabricator via lldb-commits
thakis added a comment.

Nice! This makes https://bugs.llvm.org/show_bug.cgi?id=42524#c3 easier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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


[Lldb-commits] [PATCH] D65546: Fix TestThreadSpecificBreakpoint on Windows

2019-08-01 Thread Adrian McCarthy via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367573: Fix TestThreadSpecificBreakpoint on Windows 
(authored by amccarth, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65546?vs=212681=212818#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65546

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp


Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
@@ -5,7 +5,14 @@
 thread_function ()
 {
 // Set thread-specific breakpoint here.
-std::this_thread::sleep_for(std::chrono::microseconds(100));
+std::this_thread::sleep_for(std::chrono::milliseconds(20));
+// On Windows, a sleep_for of less than about 16 ms effectively calls
+// Sleep(0).  The MS standard thread implementation uses a system thread
+// pool, which can deadlock on a Sleep(0), hanging not only the secondary
+// thread but the entire test.  I increased the delay to 20 ms to ensure
+// Sleep is called with a delay greater than 0.  The deadlock potential
+// is described here:
+// 
https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-sleep#remarks
 }
 
 int 
Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
@@ -27,7 +27,6 @@
 
 @add_test_categories(['pyapi'])
 
-@expectedFailureAll(oslist=["windows"])
 @expectedFailureAll(oslist=['ios', 'watchos', 'tvos', 'bridgeos'], 
archs=['armv7', 'armv7k'], bugnumber='rdar://problem/34563920') # armv7 ios 
problem - breakpoint with tid qualifier isn't working
 @expectedFailureNetBSD
 def test_thread_id(self):
@@ -46,13 +45,6 @@
 (target, process, main_thread, main_breakpoint) = 
lldbutil.run_to_source_breakpoint(self,
 "Set main breakpoint here", main_source_spec)
 
-main_thread_id = main_thread.GetThreadID()
-
-# This test works by setting a breakpoint in a function conditioned to 
stop only on
-# the main thread, and then calling this function on a secondary 
thread, joining,
-# and then calling again on the main thread.  If the thread specific 
breakpoint works
-# then it should not be hit on the secondary thread, only on the main
-# thread.
 thread_breakpoint = target.BreakpointCreateBySourceRegex(
 "Set thread-specific breakpoint here", main_source_spec)
 self.assertGreater(
@@ -60,11 +52,11 @@
 0,
 "thread breakpoint has no locations associated with it.")
 
-# Set the thread-specific breakpoint to only stop on the main thread.  
The run the function
-# on another thread and join on it.  If the thread-specific breakpoint 
works, the next
-# stop should be on the main thread.
-
-main_thread_id = main_thread.GetThreadID()
+# Set the thread-specific breakpoint to stop only on the main thread
+# before the secondary thread has a chance to execute it.  The main
+# thread joins the secondary thread, and then the main thread will
+# execute the code at the breakpoint.  If the thread-specific
+# breakpoint works, the next stop will be on the main thread.
 setter_method(main_thread, thread_breakpoint)
 
 process.Continue()
@@ -81,5 +73,5 @@
 "thread breakpoint stopped at unexpected number of threads")
 self.assertEqual(
 stopped_threads[0].GetThreadID(),
-main_thread_id,
+main_thread.GetThreadID(),
 "thread breakpoint stopped at the wrong thread")


Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
@@ -5,7 +5,14 @@
 thread_function ()
 {
 // Set thread-specific 

[Lldb-commits] [lldb] r367573 - Fix TestThreadSpecificBreakpoint on Windows

2019-08-01 Thread Adrian McCarthy via lldb-commits
Author: amccarth
Date: Thu Aug  1 07:49:54 2019
New Revision: 367573

URL: http://llvm.org/viewvc/llvm-project?rev=367573=rev
Log:
Fix TestThreadSpecificBreakpoint on Windows

This test was frequently hanging on Windows, causing a timeout after
10 minutes.  The short delay (100 microsecond) in the sample program
could cause a deadlock in the Windows thread pool, as I've explained
in the test program's comments.

Now that it doesn't hang, it passes reliably, so I've removed the
Windows-specific XFAIL.

I've tried to clarify the comments in TestThreadSpecificGBreakpoint.py
by eliminating some redundancy and typos, and I simplified away a
couple unnecessary assignments.

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py?rev=367573=367572=367573=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
 Thu Aug  1 07:49:54 2019
@@ -27,7 +27,6 @@ class ThreadSpecificBreakTestCase(TestBa
 
 @add_test_categories(['pyapi'])
 
-@expectedFailureAll(oslist=["windows"])
 @expectedFailureAll(oslist=['ios', 'watchos', 'tvos', 'bridgeos'], 
archs=['armv7', 'armv7k'], bugnumber='rdar://problem/34563920') # armv7 ios 
problem - breakpoint with tid qualifier isn't working
 @expectedFailureNetBSD
 def test_thread_id(self):
@@ -46,13 +45,6 @@ class ThreadSpecificBreakTestCase(TestBa
 (target, process, main_thread, main_breakpoint) = 
lldbutil.run_to_source_breakpoint(self,
 "Set main breakpoint here", main_source_spec)
 
-main_thread_id = main_thread.GetThreadID()
-
-# This test works by setting a breakpoint in a function conditioned to 
stop only on
-# the main thread, and then calling this function on a secondary 
thread, joining,
-# and then calling again on the main thread.  If the thread specific 
breakpoint works
-# then it should not be hit on the secondary thread, only on the main
-# thread.
 thread_breakpoint = target.BreakpointCreateBySourceRegex(
 "Set thread-specific breakpoint here", main_source_spec)
 self.assertGreater(
@@ -60,11 +52,11 @@ class ThreadSpecificBreakTestCase(TestBa
 0,
 "thread breakpoint has no locations associated with it.")
 
-# Set the thread-specific breakpoint to only stop on the main thread.  
The run the function
-# on another thread and join on it.  If the thread-specific breakpoint 
works, the next
-# stop should be on the main thread.
-
-main_thread_id = main_thread.GetThreadID()
+# Set the thread-specific breakpoint to stop only on the main thread
+# before the secondary thread has a chance to execute it.  The main
+# thread joins the secondary thread, and then the main thread will
+# execute the code at the breakpoint.  If the thread-specific
+# breakpoint works, the next stop will be on the main thread.
 setter_method(main_thread, thread_breakpoint)
 
 process.Continue()
@@ -81,5 +73,5 @@ class ThreadSpecificBreakTestCase(TestBa
 "thread breakpoint stopped at unexpected number of threads")
 self.assertEqual(
 stopped_threads[0].GetThreadID(),
-main_thread_id,
+main_thread.GetThreadID(),
 "thread breakpoint stopped at the wrong thread")

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp?rev=367573=367572=367573=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/main.cpp
 Thu Aug  1 07:49:54 2019
@@ -5,7 +5,14 @@ void
 thread_function ()
 {
 // Set thread-specific breakpoint here.
-std::this_thread::sleep_for(std::chrono::microseconds(100));
+std::this_thread::sleep_for(std::chrono::milliseconds(20));
+// On Windows, a sleep_for of less than 

[Lldb-commits] [lldb] r367554 - Add llvm-dwarfdump to the list of test dependencies

2019-08-01 Thread Pavel Labath via lldb-commits
Author: labath
Date: Thu Aug  1 05:50:25 2019
New Revision: 367554

URL: http://llvm.org/viewvc/llvm-project?rev=367554=rev
Log:
Add llvm-dwarfdump to the list of test dependencies

It is used by SymbolFile/DWARF/debug-types-dwo-cross-reference.cpp

Modified:
lldb/trunk/lit/CMakeLists.txt

Modified: lldb/trunk/lit/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/CMakeLists.txt?rev=367554=367553=367554=diff
==
--- lldb/trunk/lit/CMakeLists.txt (original)
+++ lldb/trunk/lit/CMakeLists.txt Thu Aug  1 05:50:25 2019
@@ -52,6 +52,7 @@ list(APPEND LLDB_TEST_DEPS
   lldb-test
   lli
   llvm-config
+  llvm-dwarfdump
   llvm-mc
   llvm-objcopy
   llvm-readobj


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


[Lldb-commits] [PATCH] D65571: [lldb][NFC] Remove unused imports from Python tests

2019-08-01 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.
Herald added a subscriber: dexonsmith.

I haven't looked at all files, but the changes are pretty straight-forward, and 
bound to break in a pretty obvious way if this is wrong, so LGTM.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65571



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


Re: [Lldb-commits] [lldb] r367549 - [lit] Use ld.lld -z separate-code to work around a debug_line parsing bug

2019-08-01 Thread Pavel Labath via lldb-commits

On 01/08/2019 14:34, Fangrui Song via lldb-commits wrote:

Author: maskray
Date: Thu Aug  1 05:34:43 2019
New Revision: 367549

URL: http://llvm.org/viewvc/llvm-project?rev=367549=rev
Log:
[lit] Use ld.lld -z separate-code to work around a debug_line parsing bug

The issue was exposed by D64903/r367537.
http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/7321/

In these tests, .debug_str immediately follows .text.
The last section of last RX PT_LOAD was originally padded with trap
instructions and .debug_str started at a new page (actually
common-page-size). Now, .debug_str immediately follows .test.
Add -z separate-code to use the old layout.




Thanks, I was about to commit the same patch.

For future reference, here is my analysis I was going to put into the 
commit message:


---
The lld patch changed how it lays out data in segments, which was
resulted in the tiny test funclets being at the very end of a PT_LOAD
segment. This exposed a bug where lldb does not correctly handle line
table end-of-sequence entries to the very end of a section (so the byte
they point to does not actually belong to any section).

This is mostly bening in practice, as the code handling line tables will
terminate as soon as it is not able to resolve an address, which is
something that would happen after reading the end-of-sequence entry
anyway. However, these tests assert that we are actually able to parse
and display the end entry itself, and so they fail.
---
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r367549 - [lit] Use ld.lld -z separate-code to work around a debug_line parsing bug

2019-08-01 Thread Fangrui Song via lldb-commits
Author: maskray
Date: Thu Aug  1 05:34:43 2019
New Revision: 367549

URL: http://llvm.org/viewvc/llvm-project?rev=367549=rev
Log:
[lit] Use ld.lld -z separate-code to work around a debug_line parsing bug

The issue was exposed by D64903/r367537.
http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/7321/

In these tests, .debug_str immediately follows .text.
The last section of last RX PT_LOAD was originally padded with trap
instructions and .debug_str started at a new page (actually
common-page-size). Now, .debug_str immediately follows .test.
Add -z separate-code to use the old layout.

Modified:
lldb/trunk/lit/SymbolFile/DWARF/debug-line-basic.s
lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir-relative-name.s
lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s
lldb/trunk/lit/SymbolFile/DWARF/dir-separator-posix.s
lldb/trunk/lit/SymbolFile/DWARF/dir-separator-windows.s

Modified: lldb/trunk/lit/SymbolFile/DWARF/debug-line-basic.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/debug-line-basic.s?rev=367549=367548=367549=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/debug-line-basic.s (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/debug-line-basic.s Thu Aug  1 05:34:43 2019
@@ -1,7 +1,7 @@
 # REQUIRES: lld, x86
 
 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o
-# RUN: ld.lld %t.o -o %t
+# RUN: ld.lld -z separate-code %t.o -o %t
 # RUN: %lldb %t -o "image dump line-table -v a.c" -o exit | FileCheck %s
 
 

Modified: 
lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir-relative-name.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir-relative-name.s?rev=367549=367548=367549=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir-relative-name.s 
(original)
+++ lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir-relative-name.s 
Thu Aug  1 05:34:43 2019
@@ -5,7 +5,7 @@
 # REQUIRES: lld, x86
 
 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o
-# RUN: ld.lld %t.o -o %t
+# RUN: ld.lld -z separate-code %t.o -o %t
 # RUN: %lldb %t -s %S/Inputs/dir-separator-no-comp-dir-relative-name.lldbinit 
-o exit | FileCheck %s
 
 # CHECK-LABEL: image dump line-table a.c

Modified: lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s?rev=367549=367548=367549=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s Thu Aug  1 
05:34:43 2019
@@ -4,7 +4,7 @@
 # REQUIRES: lld, x86
 
 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o
-# RUN: ld.lld %t.o -o %t
+# RUN: ld.lld -z separate-code %t.o -o %t
 # RUN: %lldb %t -s %S/Inputs/dir-separator-windows.lldbinit -o exit | 
FileCheck %s
 
 # CHECK-LABEL: image dump line-table a.c

Modified: lldb/trunk/lit/SymbolFile/DWARF/dir-separator-posix.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/dir-separator-posix.s?rev=367549=367548=367549=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/dir-separator-posix.s (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/dir-separator-posix.s Thu Aug  1 05:34:43 
2019
@@ -4,7 +4,7 @@
 # REQUIRES: lld, x86
 
 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o
-# RUN: ld.lld %t.o -o %t
+# RUN: ld.lld -z separate-code %t.o -o %t
 # RUN: %lldb %t -s %S/Inputs/dir-separator-posix.lldbinit -o exit | FileCheck 
%s
 
 # CHECK-LABEL: image dump line-table a.c

Modified: lldb/trunk/lit/SymbolFile/DWARF/dir-separator-windows.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/dir-separator-windows.s?rev=367549=367548=367549=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/dir-separator-windows.s (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/dir-separator-windows.s Thu Aug  1 05:34:43 
2019
@@ -4,7 +4,7 @@
 # REQUIRES: lld, x86
 
 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o
-# RUN: ld.lld %t.o -o %t
+# RUN: ld.lld -z separate-code %t.o -o %t
 # RUN: %lldb %t -s %S/Inputs/dir-separator-windows.lldbinit -o exit | 
FileCheck %s
 
 # CHECK-LABEL: image dump line-table a.c


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


[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

The error handling in LLDB seems fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65534



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


[Lldb-commits] [PATCH] D65386: [lldb][NFC] Use an enum instead of chars when handling options [WIP]

2019-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D65386#1609986 , @teemperor wrote:

> In D65386#1609956 , @labath wrote:
>
> >
>
>
> The setter function still seems more verbose than three short lines, but I 
> don't have a strong opinion about that so setters are fine. Still, if we 
> generate some function calling the appropriate setters, we will end up with 
> control flow going through that function which I would like to avoid. 
> Grepping for `SetOptionGlobal` for example would not yield any call location 
> when people try to understand who calls these methods (which is one of the 
> things that @jingham says he's worried about, correct me if I'm wrong).


I think we understand @jingham's concerns the same way. My answer to that would 
be that you'd find "SetOptionGlobal" somewhere in the tablegen file, which 
should be enough to signal that something tablegen-y is going on..

> 
> 
>> And this doesn't include the boilerplate around the switch statement -- 
>> i.e., checking the result of `toOptionEnumSettingsSet -- it's not even clear 
>> to me under which circumstances can `toOptionEnumSettingsSet` fail to return 
>> a value. Shouldn't the caller verify that it is calling us with the correct 
>> argument? If we're able to avoid that and just have the user code be:
>> 
>>   switch(toOptionEnumSettingsSet(???)) {
>>   ...
>>   }
> 
> It fails on unrecognized options (which is currently handled in every single 
> command separately). If we can lift this up (I hope every command actually 
> handles invalid options in the same name) then the switch-syntax should be 
> possible.

Hmm.. how hard would it be to achieve that? If we can make that happen, then I 
think this might be the most reasonable compromise...

In fact, how sure are you that the unrecognised options are not already handled 
somewhere higher up? Doesn't all parsing already go through Options::Parse, 
which already checks for `'?'` (and other error-ish results from getopt). After 
that point, I'd hope that we can assume that the returned value does indeed 
correspond to some entry in our option vector...

>> I agree, but on the other hand, temporary workarounds have a habit of 
>> becoming permanent, so I'd like to avoid introducing a sub-optimal solution, 
>> if there's a better way to do that available..
> 
> I think I phrased that badly. I meant that it's a workaround until the patch 
> is ready to land, not the usual temporary fix(TM) :)

Ah, ok, thanks for explaining that. I guess that would avoid generating code 
completely, but I think we should still try find a way to streamline the code 
that the user writes.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65386



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


[Lldb-commits] [PATCH] D65386: [lldb][NFC] Use an enum instead of chars when handling options [WIP]

2019-08-01 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

In D65386#1609956 , @labath wrote:

> In D65386#1609875 , @teemperor wrote:
>
> > In D65386#1604498 , @labath wrote:
> >
> > > How about codegenning the entire implementation of `SetOptionValue`? That 
> > > way the user won't have to write any switch statements at all. Ideally, 
> > > the option-setting code would be something like:
> > >
> > >   void (Status?, Error?) SetOptionForce(StringRef arg, ExecutionContext 
> > > *ctx) { m_force = true; }
> > >   void (Status?, Error?) SetOptionGlobal(StringRef arg, ExecutionContext 
> > > *ctx) { m_global = true; }
> > >  
> > >   #include "The_thing_which_generates_SetOptionValue.inc"
> > >
> > >
> > > The generated implementation of SetOptionValue could be the same as the 
> > > current one, except that it calls into these user-specified functions 
> > > instead of setting the values itself
> >
> >
> > This seems like a lot of boilerplate when we have to write 300+ 
> > one-statement methods for assigning options.
>
>
> If they would really be just one-liners, then this might still be an 
> improvement over the current solution, because now you need at least three 
> lines for each option:
>
>   case eFoo:
> foo = bar;
> break;
>


The setter function still seems more verbose than three short lines, but I 
don't have a strong opinion about that so setters are fine. Still, if we 
generate some function calling the appropriate setters, we will end up with 
control flow going through that function which I would like to avoid. Grepping 
for `SetOptionGlobal` for example would not yield any call location when people 
try to understand who calls these methods (which is one of the things that 
@jingham says he's worried about, correct me if I'm wrong).

> And this doesn't include the boilerplate around the switch statement -- i.e., 
> checking the result of `toOptionEnumSettingsSet -- it's not even clear to me 
> under which circumstances can `toOptionEnumSettingsSet` fail to return a 
> value. Shouldn't the caller verify that it is calling us with the correct 
> argument? If we're able to avoid that and just have the user code be:
> 
>   switch(toOptionEnumSettingsSet(???)) {
>   ...
>   }

It fails on unrecognized options (which is currently handled in every single 
command separately). If we can lift this up (I hope every command actually 
handles invalid options in the same name) then the switch-syntax should be 
possible.

> then I would find the enum solution fine. However, right now, it seems more 
> complicated than it ought to be..
> 
> Additionally, if setting the option is really that simple, then we could have 
> tablegen generate that too (by just giving it a variable name to set), 
> possibly with the option to fall back to a function for more complex options 
> (which is better handled in a separate function anyway).
> 
>> Also I would prefer to not use tablegen for generating executable code if 
>> possible because that is just hard to read (the function we generate here is 
>> already something I only consider as a temporary workaround).
> 
> I agree, but on the other hand, temporary workarounds have a habit of 
> becoming permanent, so I'd like to avoid introducing a sub-optimal solution, 
> if there's a better way to do that available..

I think I phrased that badly. I meant that it's a workaround until the patch is 
ready to land, not the usual temporary fix(TM) :)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65386



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


[Lldb-commits] [lldb] r367542 - [lldb][NFC] Make ClangDiagnostic::m_fixit_vec private

2019-08-01 Thread Raphael Isemann via lldb-commits
Author: teemperor
Date: Thu Aug  1 04:05:47 2019
New Revision: 367542

URL: http://llvm.org/viewvc/llvm-project?rev=367542=rev
Log:
[lldb][NFC] Make ClangDiagnostic::m_fixit_vec private

Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h

Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h?rev=367542=367541=367542=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h Thu Aug  
1 04:05:47 2019
@@ -42,6 +42,7 @@ public:
   }
 
   const FixItList () const { return m_fixit_vec; }
+private:
   FixItList m_fixit_vec;
 };
 


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


[Lldb-commits] [PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

2019-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D64993#1608946 , @JosephTremoulet 
wrote:

> In D64993#1608452 , @labath wrote:
>
> > I haven't looked at the implementation, but I did run into the problem you 
> > are fixing in the past, and I am happy that someone is finally implementing 
> > this. I just wanted to say that for testing, I think you should be able to 
> > create some hand-written assembly that creates the kind of stack frames and 
> > unwind info you need to trigger this. You can look at existing tests in 
> > `lldb/lit/Unwind` for examples.
>
>
> Thanks for the tip!  I was indeed able to copy one of those and adapt it to 
> test this.


Awesome. Thanks for adding the test. The test looks great.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64993



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


[Lldb-commits] [PATCH] D65569: Remove SymbolVendor::GetSymtab

2019-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: clayborg, JDevlieghere, jingham.

This patch removes the GetSymtab method from the SymbolVendor, which is
a no-op as it's implementation just forwards to the relevant SymbolFile.
Instead it creates a Module::GetSymtab, which calls the SymbolFile
method directly.

All callers have been updated to use the Module method directly instead
of a two phase GetSymbolVendor->GetSymtab search, which leads to reduced
intentation in a lot of deeply nested code.


https://reviews.llvm.org/D65569

Files:
  include/lldb/Core/Module.h
  include/lldb/Symbol/SymbolVendor.h
  source/API/SBModule.cpp
  source/Commands/CommandObjectTarget.cpp
  source/Core/Address.cpp
  source/Core/Module.cpp
  source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  source/Symbol/SymbolFile.cpp
  source/Symbol/SymbolVendor.cpp

Index: source/Symbol/SymbolVendor.cpp
===
--- source/Symbol/SymbolVendor.cpp
+++ source/Symbol/SymbolVendor.cpp
@@ -263,12 +263,6 @@
 s->EOL();
 if (m_sym_file_up)
   m_sym_file_up->Dump(*s);
-s->IndentMore();
-
-if (Symtab *symtab = GetSymtab())
-  symtab->Dump(s, nullptr, eSortOrderNone);
-
-s->IndentLess();
   }
 }
 
@@ -288,12 +282,6 @@
   return FileSpec();
 }
 
-Symtab *SymbolVendor::GetSymtab() {
-  if (m_sym_file_up)
-return m_sym_file_up->GetSymtab();
-  return nullptr;
-}
-
 void SymbolVendor::SectionFileAddressesChanged() {
   if (m_sym_file_up)
 m_sym_file_up->SectionFileAddressesChanged();
Index: source/Symbol/SymbolFile.cpp
===
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -244,6 +244,9 @@
 }
   }
   s.PutChar('\n');
+
+  if (Symtab *symtab = GetSymtab())
+symtab->Dump(, nullptr, eSortOrderNone);
 }
 
 SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;
Index: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
===
--- source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -370,22 +370,18 @@
 addr_t
 DynamicLoaderMacOS::GetDyldLockVariableAddressFromModule(Module *module) {
   SymbolContext sc;
-  SymbolVendor *sym_vendor = module->GetSymbolVendor();
   Target  = m_process->GetTarget();
-  if (sym_vendor) {
-Symtab *symtab = sym_vendor->GetSymtab();
-if (symtab) {
-  std::vector match_indexes;
-  ConstString g_symbol_name("_dyld_global_lock_held");
-  uint32_t num_matches = 0;
-  num_matches =
-  symtab->AppendSymbolIndexesWithName(g_symbol_name, match_indexes);
-  if (num_matches == 1) {
-Symbol *symbol = symtab->SymbolAtIndex(match_indexes[0]);
-if (symbol &&
-(symbol->ValueIsAddress() || symbol->GetAddressRef().IsValid())) {
-  return symbol->GetAddressRef().GetOpcodeLoadAddress();
-}
+  if (Symtab *symtab = module->GetSymtab()) {
+std::vector match_indexes;
+ConstString g_symbol_name("_dyld_global_lock_held");
+uint32_t num_matches = 0;
+num_matches =
+symtab->AppendSymbolIndexesWithName(g_symbol_name, match_indexes);
+if (num_matches == 1) {
+  Symbol *symbol = symtab->SymbolAtIndex(match_indexes[0]);
+  if (symbol &&
+  (symbol->ValueIsAddress() || symbol->GetAddressRef().IsValid())) {
+return symbol->GetAddressRef().GetOpcodeLoadAddress();
   }
 }
   }
Index: source/Core/Module.cpp
===
--- source/Core/Module.cpp
+++ source/Core/Module.cpp
@@ -1061,6 +1061,12 @@
   return nullptr;
 }
 
+Symtab *Module::GetSymtab() {
+  if (SymbolFile *symbols = GetSymbolFile())
+return symbols->GetSymtab();
+  return nullptr;
+}
+
 void Module::SetFileSpecAndObjectName(const FileSpec ,
   ConstString object_name) {
   // Container objects whose paths do not specify a file directly can call this
@@ -1231,9 +1237,8 @@
   if (objfile)
 objfile->Dump(s);
 
-  SymbolVendor *symbols = GetSymbolVendor();
-  if (symbols)
-symbols->Dump(s);
+  if (SymbolFile *symbols = GetSymbolFile())
+symbols->Dump(*s);
 
   s->IndentLess();
 }
@@ -1309,13 +1314,9 @@
   Timer scoped_timer(
   func_cat, "Module::FindFirstSymbolWithNameAndType (name = %s, type = %i)",
   name.AsCString(), symbol_type);
-  SymbolVendor *sym_vendor = GetSymbolVendor();
-  if (sym_vendor) {
-Symtab *symtab = sym_vendor->GetSymtab();
-if (symtab)
-  return symtab->FindFirstSymbolWithNameAndType(
-  name, symbol_type, Symtab::eDebugAny, Symtab::eVisibilityAny);
-  }
+  if (Symtab *symtab = GetSymtab())
+return symtab->FindFirstSymbolWithNameAndType(
+name, symbol_type, Symtab::eDebugAny, Symtab::eVisibilityAny);
   return nullptr;
 }
 void 

[Lldb-commits] [PATCH] D65386: [lldb][NFC] Use an enum instead of chars when handling options [WIP]

2019-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D65386#1609875 , @teemperor wrote:

> In D65386#1604498 , @labath wrote:
>
> > How about codegenning the entire implementation of `SetOptionValue`? That 
> > way the user won't have to write any switch statements at all. Ideally, the 
> > option-setting code would be something like:
> >
> >   void (Status?, Error?) SetOptionForce(StringRef arg, ExecutionContext 
> > *ctx) { m_force = true; }
> >   void (Status?, Error?) SetOptionGlobal(StringRef arg, ExecutionContext 
> > *ctx) { m_global = true; }
> >  
> >   #include "The_thing_which_generates_SetOptionValue.inc"
> >
> >
> > The generated implementation of SetOptionValue could be the same as the 
> > current one, except that it calls into these user-specified functions 
> > instead of setting the values itself
>
>
> This seems like a lot of boilerplate when we have to write 300+ one-statement 
> methods for assigning options.


If they would really be just one-liners, then this might still be an 
improvement over the current solution, because now you need at least three 
lines for each option:

  case eFoo:
foo = bar;
break;

And this doesn't include the boilerplate around the switch statement -- i.e., 
checking the result of `toOptionEnumSettingsSet -- it's not even clear to me 
under which circumstances can `toOptionEnumSettingsSet` fail to return a value. 
Shouldn't the caller verify that it is calling us with the correct argument? If 
we're able to avoid that and just have the user code be:

  switch(toOptionEnumSettingsSet(???)) {
  ...
  }

then I would find the enum solution fine. However, right now, it seems more 
complicated than it ought to be..

Additionally, if setting the option is really that simple, then we could have 
tablegen generate that too (by just giving it a variable name to set), possibly 
with the option to fall back to a function for more complex options (which is 
better handled in a separate function anyway).

> Also I would prefer to not use tablegen for generating executable code if 
> possible because that is just hard to read (the function we generate here is 
> already something I only consider as a temporary workaround).

I agree, but on the other hand, temporary workarounds have a habit of becoming 
permanent, so I'd like to avoid introducing a sub-optimal solution, if there's 
a better way to do that available..


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65386



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


[Lldb-commits] [PATCH] D65509: [lldb][CMake] Avoid 'Autogenerate scheme' dialogs in Xcode projects

2019-08-01 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367538: [lldb][CMake] Avoid Autogenerate scheme 
dialogs in Xcode projects (authored by stefan.graenitz, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65509?vs=212560=212773#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65509

Files:
  lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake


Index: lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake
===
--- lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake
+++ lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake
@@ -2,3 +2,4 @@
 
 set(LLDB_BUILD_FRAMEWORK ON CACHE BOOL "")
 set(CMAKE_OSX_DEPLOYMENT_TARGET 10.11 CACHE STRING "")
+set(CMAKE_XCODE_GENERATE_SCHEME ON CACHE BOOL "")


Index: lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake
===
--- lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake
+++ lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake
@@ -2,3 +2,4 @@
 
 set(LLDB_BUILD_FRAMEWORK ON CACHE BOOL "")
 set(CMAKE_OSX_DEPLOYMENT_TARGET 10.11 CACHE STRING "")
+set(CMAKE_XCODE_GENERATE_SCHEME ON CACHE BOOL "")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D65437: [lldb][docs] Update landing page for monorepo

2019-08-01 Thread Phabricator via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367539: [lldb][docs] Update landing page for monorepo 
(authored by stefan.graenitz, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65437?vs=212311=212774#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65437

Files:
  lldb/trunk/docs/index.rst
  lldb/trunk/docs/resources/build.rst


Index: lldb/trunk/docs/index.rst
===
--- lldb/trunk/docs/index.rst
+++ lldb/trunk/docs/index.rst
@@ -80,27 +80,31 @@
 Get Involved
 
 
-To check out the code, use:
+Check out the LLVM source-tree with git and find the sources in the `lldb`
+subdirectory:
 
-svn co http://llvm.org/svn/llvm-project/lldb/trunk lldb
+::
 
-Note that LLDB generally builds from top-of-trunk
+  > git clone https://github.com/llvm/llvm-project.git
 
-* On macOS with Xcode
-* On Linux and FreeBSD (with clang and libstdc++/libc++)
-* On NetBSD (with GCC and clang and libstdc++/libc++)
-* On Windows with VS 2012 or higher using CMake
+Note that LLDB generally builds from top-of-trunk using CMake and Ninja.
+Additionally it builds:
 
-See the LLDB Build Page for platform-specific build instructions.
+* on macOS with a :ref:`generated Xcode project `
+* on Linux and FreeBSD with clang and libstdc++/libc++
+* on NetBSD with GCC/clang and libstdc++/libc++
+* on Windows with a generated project for VS 2017 or higher
+
+See the :doc:`LLDB Build Page ` for build instructions.
 
 Discussions about LLDB should go to the `lldb-dev
 `__ mailing list. Commit
-messages for the lldb SVN module are automatically sent to the `lldb-commits
+messages are automatically sent to the `lldb-commits
 `__ mailing list , and
 this is also the preferred mailing list for patch submissions.
 
-See the Projects page if you are looking for some interesting areas to
-contribute to lldb.
+See the :doc:`Projects page ` if you are looking for some
+interesting areas to contribute to lldb.
 
 .. toctree::
:hidden:
Index: lldb/trunk/docs/resources/build.rst
===
--- lldb/trunk/docs/resources/build.rst
+++ lldb/trunk/docs/resources/build.rst
@@ -306,6 +306,8 @@
 
   > DESTDIR=/path/to/lldb-install ninja -C /path/to/lldb-build check-lldb 
install-distribution
 
+.. _CMakeGeneratedXcodeProject:
+
 Build LLDB standalone for development with Xcode:
 
 ::


Index: lldb/trunk/docs/index.rst
===
--- lldb/trunk/docs/index.rst
+++ lldb/trunk/docs/index.rst
@@ -80,27 +80,31 @@
 Get Involved
 
 
-To check out the code, use:
+Check out the LLVM source-tree with git and find the sources in the `lldb`
+subdirectory:
 
-svn co http://llvm.org/svn/llvm-project/lldb/trunk lldb
+::
 
-Note that LLDB generally builds from top-of-trunk
+  > git clone https://github.com/llvm/llvm-project.git
 
-* On macOS with Xcode
-* On Linux and FreeBSD (with clang and libstdc++/libc++)
-* On NetBSD (with GCC and clang and libstdc++/libc++)
-* On Windows with VS 2012 or higher using CMake
+Note that LLDB generally builds from top-of-trunk using CMake and Ninja.
+Additionally it builds:
 
-See the LLDB Build Page for platform-specific build instructions.
+* on macOS with a :ref:`generated Xcode project `
+* on Linux and FreeBSD with clang and libstdc++/libc++
+* on NetBSD with GCC/clang and libstdc++/libc++
+* on Windows with a generated project for VS 2017 or higher
+
+See the :doc:`LLDB Build Page ` for build instructions.
 
 Discussions about LLDB should go to the `lldb-dev
 `__ mailing list. Commit
-messages for the lldb SVN module are automatically sent to the `lldb-commits
+messages are automatically sent to the `lldb-commits
 `__ mailing list , and
 this is also the preferred mailing list for patch submissions.
 
-See the Projects page if you are looking for some interesting areas to
-contribute to lldb.
+See the :doc:`Projects page ` if you are looking for some
+interesting areas to contribute to lldb.
 
 .. toctree::
:hidden:
Index: lldb/trunk/docs/resources/build.rst
===
--- lldb/trunk/docs/resources/build.rst
+++ lldb/trunk/docs/resources/build.rst
@@ -306,6 +306,8 @@
 
   > DESTDIR=/path/to/lldb-install ninja -C /path/to/lldb-build check-lldb install-distribution
 
+.. _CMakeGeneratedXcodeProject:
+
 Build LLDB standalone for development with Xcode:
 
 ::
___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [lldb] r367539 - [lldb][docs] Update landing page for monorepo

2019-08-01 Thread Stefan Granitz via lldb-commits
Author: stefan.graenitz
Date: Thu Aug  1 03:33:54 2019
New Revision: 367539

URL: http://llvm.org/viewvc/llvm-project?rev=367539=rev
Log:
[lldb][docs] Update landing page for monorepo

Summary: Following up from D65330, here's an update for the landing page.

Reviewers: jryans, clayborg, amccarth, labath

Reviewed By: jryans, amccarth, labath

Subscribers: arphaman, lldb-commits, #lldb

Tags: #lldb

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

Modified:
lldb/trunk/docs/index.rst
lldb/trunk/docs/resources/build.rst

Modified: lldb/trunk/docs/index.rst
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/index.rst?rev=367539=367538=367539=diff
==
--- lldb/trunk/docs/index.rst (original)
+++ lldb/trunk/docs/index.rst Thu Aug  1 03:33:54 2019
@@ -80,27 +80,31 @@ expected to work, with functionality imp
 Get Involved
 
 
-To check out the code, use:
+Check out the LLVM source-tree with git and find the sources in the `lldb`
+subdirectory:
 
-svn co http://llvm.org/svn/llvm-project/lldb/trunk lldb
+::
 
-Note that LLDB generally builds from top-of-trunk
+  > git clone https://github.com/llvm/llvm-project.git
 
-* On macOS with Xcode
-* On Linux and FreeBSD (with clang and libstdc++/libc++)
-* On NetBSD (with GCC and clang and libstdc++/libc++)
-* On Windows with VS 2012 or higher using CMake
+Note that LLDB generally builds from top-of-trunk using CMake and Ninja.
+Additionally it builds:
 
-See the LLDB Build Page for platform-specific build instructions.
+* on macOS with a :ref:`generated Xcode project `
+* on Linux and FreeBSD with clang and libstdc++/libc++
+* on NetBSD with GCC/clang and libstdc++/libc++
+* on Windows with a generated project for VS 2017 or higher
+
+See the :doc:`LLDB Build Page ` for build instructions.
 
 Discussions about LLDB should go to the `lldb-dev
 `__ mailing list. Commit
-messages for the lldb SVN module are automatically sent to the `lldb-commits
+messages are automatically sent to the `lldb-commits
 `__ mailing list , and
 this is also the preferred mailing list for patch submissions.
 
-See the Projects page if you are looking for some interesting areas to
-contribute to lldb.
+See the :doc:`Projects page ` if you are looking for some
+interesting areas to contribute to lldb.
 
 .. toctree::
:hidden:

Modified: lldb/trunk/docs/resources/build.rst
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/resources/build.rst?rev=367539=367538=367539=diff
==
--- lldb/trunk/docs/resources/build.rst (original)
+++ lldb/trunk/docs/resources/build.rst Thu Aug  1 03:33:54 2019
@@ -306,6 +306,8 @@ LLVM  DESTDIR=/path/to/lldb-install ninja -C /path/to/lldb-build check-lldb 
install-distribution
 
+.. _CMakeGeneratedXcodeProject:
+
 Build LLDB standalone for development with Xcode:
 
 ::


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


[Lldb-commits] [lldb] r367538 - [lldb][CMake] Avoid 'Autogenerate scheme' dialogs in Xcode projects

2019-08-01 Thread Stefan Granitz via lldb-commits
Author: stefan.graenitz
Date: Thu Aug  1 03:33:44 2019
New Revision: 367538

URL: http://llvm.org/viewvc/llvm-project?rev=367538=rev
Log:
[lldb][CMake] Avoid 'Autogenerate scheme' dialogs in Xcode projects

Summary:
Supported in CMake 3.9 and higher: 
https://cmake.org/cmake/help/v3.9/variable/CMAKE_XCODE_GENERATE_SCHEME.html
Older versions will just report it as unused in the end of the configuration 
process.

Reviewers: jingham, JDevlieghere

Reviewed By: JDevlieghere

Subscribers: mgorny, lldb-commits, #lldb

Tags: #lldb

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

Modified:
lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake

Modified: lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake?rev=367538=367537=367538=diff
==
--- lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake (original)
+++ lldb/trunk/cmake/caches/Apple-lldb-Xcode.cmake Thu Aug  1 03:33:44 2019
@@ -2,3 +2,4 @@ include(${CMAKE_CURRENT_LIST_DIR}/Apple-
 
 set(LLDB_BUILD_FRAMEWORK ON CACHE BOOL "")
 set(CMAKE_OSX_DEPLOYMENT_TARGET 10.11 CACHE STRING "")
+set(CMAKE_XCODE_GENERATE_SCHEME ON CACHE BOOL "")


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


Re: [Lldb-commits] [lldb] r367308 - [lldb] Fix crash when tab-completing in multi-line expr

2019-08-01 Thread Hans Wennborg via lldb-commits
Merged to release_90 in r367534.

On Tue, Jul 30, 2019 at 2:53 PM Hans Wennborg  wrote:
>
> Hi Raphael,
>
> Since this just landed, let's have it bake in trunk a little bit, and
> then I'll merge it.
>
> Thanks,
> Hans
>
> On Tue, Jul 30, 2019 at 2:36 PM Raphael “Teemperor” Isemann
>  wrote:
> >
> > Hi Hans,
> >
> > Can you cherry-pick this to the 9.0 release? The issue is really easy to 
> > hit when using LLDB and the fix is obvious
> >
> > Thanks!
> > - Raphael
> >
> > > On Jul 30, 2019, at 2:31 PM, Raphael Isemann via lldb-commits 
> > >  wrote:
> > >
> > > Author: teemperor
> > > Date: Tue Jul 30 05:31:24 2019
> > > New Revision: 367308
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=367308=rev
> > > Log:
> > > [lldb] Fix crash when tab-completing in multi-line expr
> > >
> > > Summary:
> > > Tab completing inside the multiline expression command can cause LLDB to 
> > > crash. The easiest way
> > > to do this is to go inside a frame with at least one local variable and 
> > > then try to complete:
> > >
> > >(lldb) expr
> > >1. a[tab]
> > >
> > > Reason for this was some mixup when we calculate the cursor position. 
> > > Obviously we should calculate
> > > the offset inside the string by doing 'end - start', but we are doing 
> > > 'start - end' (which causes the offset to
> > > become -1 which will lead to some out-of-bounds reading).
> > >
> > > Fixes rdar://51754005
> > >
> > > I don't see any way to test this as the *multiline* expression completion 
> > > is completely untested at the moment
> > > and I don't think we have any existing code for testing infrastructure 
> > > for it.
> > >
> > > Reviewers: shafik, davide, labath
> > >
> > > Reviewed By: labath
> > >
> > > Subscribers: abidh, lldb-commits, davide, clayborg, labath
> > >
> > > Tags: #lldb
> > >
> > > Differential Revision: https://reviews.llvm.org/D64995
> > >
> > > Added:
> > >
> > > lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/
> > >
> > > lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/Makefile
> > >
> > > lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/TestMultilineCompletion.py
> > >
> > > lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/main.c
> > > Modified:
> > >lldb/trunk/source/Core/IOHandler.cpp
> > >
> > > Added: 
> > > lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/Makefile
> > > URL: 
> > > http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/Makefile?rev=367308=auto
> > > ==
> > > --- 
> > > lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/Makefile
> > >  (added)
> > > +++ 
> > > lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/Makefile
> > >  Tue Jul 30 05:31:24 2019
> > > @@ -0,0 +1,3 @@
> > > +LEVEL = ../../make
> > > +C_SOURCES := main.c
> > > +include $(LEVEL)/Makefile.rules
> > >
> > > Added: 
> > > lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/TestMultilineCompletion.py
> > > URL: 
> > > http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/TestMultilineCompletion.py?rev=367308=auto
> > > ==
> > > --- 
> > > lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/TestMultilineCompletion.py
> > >  (added)
> > > +++ 
> > > lldb/trunk/packages/Python/lldbsuite/test/expression_command/multiline-completion/TestMultilineCompletion.py
> > >  Tue Jul 30 05:31:24 2019
> > > @@ -0,0 +1,52 @@
> > > +"""
> > > +Test completion for multiline expressions.
> > > +"""
> > > +
> > > +import lldb
> > > +from lldbsuite.test.decorators import *
> > > +from lldbsuite.test.lldbtest import *
> > > +
> > > +class MultilineCompletionTest(TestBase):
> > > +
> > > +mydir = TestBase.compute_mydir(__file__)
> > > +NO_DEBUG_INFO_TESTCASE = True
> > > +
> > > +def setUp(self):
> > > +TestBase.setUp(self)
> > > +self.source = 'main.c'
> > > +
> > > +def expect_string(self, string):
> > > +import pexpect
> > > +"""This expects for "string", with timeout & EOF being test 
> > > fails."""
> > > +try:
> > > +self.child.expect_exact(string)
> > > +except pexpect.EOF:
> > > +self.fail("Got EOF waiting for '%s'" % (string))
> > > +except pexpect.TIMEOUT:
> > > +self.fail("Timed out waiting for '%s'" % (string))
> > > +
> > > +@expectedFailureAll(
> > > +oslist=["windows"],
> > > +bugnumber="llvm.org/pr22274: need a pexpect replacement for 
> > > windows")
> > > +def test_basic_completion(self):
> > > +

[Lldb-commits] [PATCH] D65566: [lldb][CMake] Workaround debugserver code-signing issue in generated Xcode project

2019-08-01 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz marked an inline comment as done.
sgraenitz added a comment.

Checked with:

  > xcodebuild -configuration Release -target debugserver
  > codesign -dv Release/bin/LLDB.framework/Versions/A/Resources/debugserver
  > codesign -dv Release/bin/debugserver




Comment at: lldb/tools/debugserver/source/CMakeLists.txt:233
+env CODESIGN_ALLOCATE=${CMAKE_CODESIGN_ALLOCATE}
+xcrun codesign -f -s ${debugserver_codesign_identity}
+${pass_entitlements} ${copy_location}

This forces code-signing and may cause double-signing warnings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65566



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


[Lldb-commits] [PATCH] D65386: [lldb][NFC] Use an enum instead of chars when handling options [WIP]

2019-08-01 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor marked an inline comment as done.
teemperor added a comment.

> It worries me a little bit that we are making it harder and harder to figure 
> out "where does the option for "-t" get stored once this CommandObject's 
> options have been parsed. Can you show the steps I would have to go through 
> to get from "-f" to OptionEnumSettingsSet::Force or whatever.

That's actually just `toOptionEnumSettingsSet('-f', error)`. I want to get rid 
of the whole generated method by just placing the enum value in the 
OptionDefinition struct (which requires some refactoring, but should be doable 
in the long-term).

> The thing I don't like about the enum approach is that it adds another layer 
> to the option-setting code, whereas I think that the main problem is that the 
> option-setting code has one too many layers already.

Agreed.

In D65386#1604498 , @labath wrote:

> How about codegenning the entire implementation of `SetOptionValue`? That way 
> the user won't have to write any switch statements at all. Ideally, the 
> option-setting code would be something like:
>
>   void (Status?, Error?) SetOptionForce(StringRef arg, ExecutionContext *ctx) 
> { m_force = true; }
>   void (Status?, Error?) SetOptionGlobal(StringRef arg, ExecutionContext 
> *ctx) { m_global = true; }
>  
>   #include "The_thing_which_generates_SetOptionValue.inc"
>
>
> The generated implementation of SetOptionValue could be the same as the 
> current one, except that it calls into these user-specified functions instead 
> of setting the values itself


This seems like a lot of boilerplate when we have to write 300+ one-statement 
methods for assigning options. Also I would prefer to not use tablegen for 
generating executable code if possible because that is just hard to read (the 
function we generate here is already something I only consider as a temporary 
workaround).




Comment at: lldb/utils/TableGen/LLDBOptionDefEmitter.cpp:211
+
+  std::string CamelCaseID = ToCamelCase(Command);
+

JDevlieghere wrote:
> Can we use the option name instead, like I did for the properties? Or would 
> that cause conflicts? 
If you mean if we can just call it `OptionEnumSet` instead of 
`OptionEnumSettingsSet`, then I assume that could cause conflicts if we 
implement multiple smaller commands in the same file (which we currently do).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65386



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


[Lldb-commits] [PATCH] D65566: [lldb][CMake] Workaround debugserver code-signing issue in generated Xcode project

2019-08-01 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz created this revision.
sgraenitz added reviewers: jingham, davide, JDevlieghere, teemperor.
Herald added a subscriber: mgorny.
Herald added a project: LLDB.
sgraenitz edited the summary of this revision.

Explicitly code-sign the LLDB.framework copy of debugserver in the build-tree. 
This is necessary, because the Xcode-specific logic in `llvm_codesign` [1] has 
the side-effect that Xcode code-signs after post-build steps (here: after 
copying debugserver over into the framework). The special case for Xcode was 
necessary to avoid double-signing errors in the past (see D55116 
 and D55816 ).

[1] 
https://github.com/llvm/llvm-project/blob/36fb93982f0e/llvm/cmake/modules/AddLLVM.cmake#L1676


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65566

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


Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -208,6 +208,34 @@
   ENTITLEMENTS ${entitlements}
 )
 
+# Workaround for Xcode-specific code-signing behavior:
+# The XCODE_ATTRIBUTE_CODE_SIGN_IDENTITY option causes debugserver to be copied
+# into the framework first and code-signed afterwards. Sign the copy manually.
+if (debugserver_codesign_identity AND LLDB_BUILD_FRAMEWORK AND
+CMAKE_GENERATOR STREQUAL "Xcode")
+  if(NOT CMAKE_CODESIGN_ALLOCATE)
+execute_process(
+  COMMAND xcrun -f codesign_allocate
+  OUTPUT_STRIP_TRAILING_WHITESPACE
+  OUTPUT_VARIABLE CMAKE_CODESIGN_ALLOCATE
+)
+  endif()
+  if(entitlements)
+set(pass_entitlements --entitlements ${entitlements})
+  endif()
+
+  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
+  set(copy_location 
${framework_build_dir}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Resources/debugserver)
+
+  add_custom_command(TARGET debugserver POST_BUILD
+COMMAND ${CMAKE_COMMAND} -E
+env CODESIGN_ALLOCATE=${CMAKE_CODESIGN_ALLOCATE}
+xcrun codesign -f -s ${debugserver_codesign_identity}
+${pass_entitlements} ${copy_location}
+COMMENT "Code-sign debugserver copy in the build-tree framework: 
${copy_location}"
+  )
+endif()
+
 set_target_properties(debugserver PROPERTIES FOLDER "lldb 
libraries/debugserver")
 
 if(IOS)


Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -208,6 +208,34 @@
   ENTITLEMENTS ${entitlements}
 )
 
+# Workaround for Xcode-specific code-signing behavior:
+# The XCODE_ATTRIBUTE_CODE_SIGN_IDENTITY option causes debugserver to be copied
+# into the framework first and code-signed afterwards. Sign the copy manually.
+if (debugserver_codesign_identity AND LLDB_BUILD_FRAMEWORK AND
+CMAKE_GENERATOR STREQUAL "Xcode")
+  if(NOT CMAKE_CODESIGN_ALLOCATE)
+execute_process(
+  COMMAND xcrun -f codesign_allocate
+  OUTPUT_STRIP_TRAILING_WHITESPACE
+  OUTPUT_VARIABLE CMAKE_CODESIGN_ALLOCATE
+)
+  endif()
+  if(entitlements)
+set(pass_entitlements --entitlements ${entitlements})
+  endif()
+
+  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
+  set(copy_location ${framework_build_dir}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Resources/debugserver)
+
+  add_custom_command(TARGET debugserver POST_BUILD
+COMMAND ${CMAKE_COMMAND} -E
+env CODESIGN_ALLOCATE=${CMAKE_CODESIGN_ALLOCATE}
+xcrun codesign -f -s ${debugserver_codesign_identity}
+${pass_entitlements} ${copy_location}
+COMMENT "Code-sign debugserver copy in the build-tree framework: ${copy_location}"
+  )
+endif()
+
 set_target_properties(debugserver PROPERTIES FOLDER "lldb libraries/debugserver")
 
 if(IOS)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D65435: SymbolVendor: Introduce Module::GetSymbolFile

2019-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 212764.
labath added a comment.

- add two null checks
- replace GetSymbolVendor with GetSymbolFile in a couple of more places, where 
it is easy to do


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

https://reviews.llvm.org/D65435

Files:
  include/lldb/Core/Module.h
  source/Commands/CommandObjectTarget.cpp
  source/Core/Module.cpp
  source/Expression/IRExecutionUnit.cpp
  source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Symbol/Block.cpp
  source/Symbol/Function.cpp
  source/Symbol/UnwindTable.cpp

Index: source/Symbol/UnwindTable.cpp
===
--- source/Symbol/UnwindTable.cpp
+++ source/Symbol/UnwindTable.cpp
@@ -182,11 +182,7 @@
   return m_arm_unwind_up.get();
 }
 
-SymbolFile *UnwindTable::GetSymbolFile() {
-  if (SymbolVendor *vendor = m_module.GetSymbolVendor())
-return vendor->GetSymbolFile();
-  return nullptr;
-}
+SymbolFile *UnwindTable::GetSymbolFile() { return m_module.GetSymbolFile(); }
 
 ArchSpec UnwindTable::GetArchitecture() { return m_module.GetArchitecture(); }
 
Index: source/Symbol/Function.cpp
===
--- source/Symbol/Function.cpp
+++ source/Symbol/Function.cpp
@@ -428,14 +428,8 @@
   ModuleSP module_sp = CalculateSymbolContextModule();
 
   if (module_sp) {
-SymbolVendor *sym_vendor = module_sp->GetSymbolVendor();
-
-if (sym_vendor) {
-  SymbolFile *sym_file = sym_vendor->GetSymbolFile();
-
-  if (sym_file)
-return sym_file->GetDeclContextForUID(GetID());
-}
+if (SymbolFile *sym_file = module_sp->GetSymbolFile())
+  return sym_file->GetDeclContextForUID(GetID());
   }
   return CompilerDeclContext();
 }
@@ -449,12 +443,7 @@
 if (!sc.module_sp)
   return nullptr;
 
-SymbolVendor *sym_vendor = sc.module_sp->GetSymbolVendor();
-
-if (sym_vendor == nullptr)
-  return nullptr;
-
-SymbolFile *sym_file = sym_vendor->GetSymbolFile();
+SymbolFile *sym_file = sc.module_sp->GetSymbolFile();
 
 if (sym_file == nullptr)
   return nullptr;
Index: source/Symbol/Block.cpp
===
--- source/Symbol/Block.cpp
+++ source/Symbol/Block.cpp
@@ -464,8 +464,7 @@
 
 SymbolFile *Block::GetSymbolFile() {
   if (ModuleSP module_sp = CalculateSymbolContextModule())
-if (SymbolVendor *sym_vendor = module_sp->GetSymbolVendor())
-  return sym_vendor->GetSymbolFile();
+return module_sp->GetSymbolFile();
   return nullptr;
 }
 
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3749,10 +3749,8 @@
   if (m_debug_map_symfile == nullptr && !m_debug_map_module_wp.expired()) {
 lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
 if (module_sp) {
-  SymbolVendor *sym_vendor = module_sp->GetSymbolVendor();
-  if (sym_vendor)
-m_debug_map_symfile =
-(SymbolFileDWARFDebugMap *)sym_vendor->GetSymbolFile();
+  m_debug_map_symfile =
+  (SymbolFileDWARFDebugMap *)module_sp->GetSymbolFile();
 }
   }
   return m_debug_map_symfile;
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -72,112 +72,106 @@
 FileSpec module_spec = module.GetFileSpec();
 
 if (module_spec) {
-  SymbolVendor *symbols = module.GetSymbolVendor();
-  if (symbols) {
-SymbolFile *symfile = symbols->GetSymbolFile();
-if (symfile) {
-  ObjectFile *objfile = symfile->GetObjectFile();
-  if (objfile) {
-FileSpec symfile_spec(objfile->GetFileSpec());
-if (symfile_spec && 
-strcasestr (symfile_spec.GetPath().c_str(), 
-".dSYM/Contents/Resources/DWARF") != nullptr &&
-FileSystem::Instance().Exists(symfile_spec)) {
-  while (module_spec.GetFilename()) {
-std::string module_basename(
-module_spec.GetFilename().GetCString());
-std::string original_module_basename(module_basename);
-
-bool was_keyword = false;
-
-// FIXME: for Python, we cannot allow certain characters in
-// module
-// filenames we import. Theoretically, different scripting
-// languages may have different sets of forbidden tokens in
-// filenames, and that should be dealt with by each
-// ScriptInterpreter. For now, we just replace dots with
-// underscores, but if we ever support 

[Lldb-commits] [PATCH] D65561: SymbolVendorELF: Perform build-id lookup even without a debug link

2019-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: jankratochvil, mgorny, clayborg.
Herald added subscribers: MaskRay, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: alexshap.

The debug link and build-id lookups are two independent ways one can
search for a separate symbol file. However, our implementation in
SymbolVendorELF was tying the two together and refusing to look up the
symbol file based on a build id if the file did not contain a debug
link.

This patch makes it possible to search for the symbol file with
just one of the two methods available. To demonstrate, I split the
build-id-case test into two, so that we test the search using both
methods.


https://reviews.llvm.org/D65561

Files:
  lit/Modules/ELF/build-id-case.yaml
  lit/Modules/ELF/gnu-debuglink.yaml
  source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
  source/Symbol/LocateSymbolFile.cpp

Index: source/Symbol/LocateSymbolFile.cpp
===
--- source/Symbol/LocateSymbolFile.cpp
+++ source/Symbol/LocateSymbolFile.cpp
@@ -261,107 +261,110 @@
   FileSystem::Instance().Exists(symbol_file_spec))
 return symbol_file_spec;
 
-  const char *symbol_filename = symbol_file_spec.GetFilename().AsCString();
-  if (symbol_filename && symbol_filename[0]) {
-FileSpecList debug_file_search_paths = default_search_paths;
+  FileSpecList debug_file_search_paths = default_search_paths;
+
+  // Add module directory.
+  FileSpec module_file_spec = module_spec.GetFileSpec();
+  // We keep the unresolved pathname if it fails.
+  FileSystem::Instance().ResolveSymbolicLink(module_file_spec,
+ module_file_spec);
+
+  ConstString file_dir = module_file_spec.GetDirectory();
+  {
+FileSpec file_spec(file_dir.AsCString("."));
+FileSystem::Instance().Resolve(file_spec);
+debug_file_search_paths.AppendIfUnique(file_spec);
+  }
 
-// Add module directory.
-FileSpec module_file_spec = module_spec.GetFileSpec();
-// We keep the unresolved pathname if it fails.
-FileSystem::Instance().ResolveSymbolicLink(module_file_spec,
-   module_file_spec);
+  if (ModuleList::GetGlobalModuleListProperties().GetEnableExternalLookup()) {
 
-ConstString file_dir = module_file_spec.GetDirectory();
+// Add current working directory.
 {
-  FileSpec file_spec(file_dir.AsCString("."));
+  FileSpec file_spec(".");
   FileSystem::Instance().Resolve(file_spec);
   debug_file_search_paths.AppendIfUnique(file_spec);
 }
 
-if (ModuleList::GetGlobalModuleListProperties().GetEnableExternalLookup()) {
-
-  // Add current working directory.
-  {
-FileSpec file_spec(".");
-FileSystem::Instance().Resolve(file_spec);
-debug_file_search_paths.AppendIfUnique(file_spec);
-  }
-
 #ifndef _WIN32
 #if defined(__NetBSD__)
-  // Add /usr/libdata/debug directory.
-  {
-FileSpec file_spec("/usr/libdata/debug");
-FileSystem::Instance().Resolve(file_spec);
-debug_file_search_paths.AppendIfUnique(file_spec);
-  }
+// Add /usr/libdata/debug directory.
+{
+  FileSpec file_spec("/usr/libdata/debug");
+  FileSystem::Instance().Resolve(file_spec);
+  debug_file_search_paths.AppendIfUnique(file_spec);
+}
 #else
-  // Add /usr/lib/debug directory.
-  {
-FileSpec file_spec("/usr/lib/debug");
-FileSystem::Instance().Resolve(file_spec);
-debug_file_search_paths.AppendIfUnique(file_spec);
-  }
+// Add /usr/lib/debug directory.
+{
+  FileSpec file_spec("/usr/lib/debug");
+  FileSystem::Instance().Resolve(file_spec);
+  debug_file_search_paths.AppendIfUnique(file_spec);
+}
 #endif
 #endif // _WIN32
-}
+  }
 
-std::string uuid_str;
-const UUID _uuid = module_spec.GetUUID();
-if (module_uuid.IsValid()) {
-  // Some debug files are stored in the .build-id directory like this:
-  //   /usr/lib/debug/.build-id/ff/e7fe727889ad82bb153de2ad065b2189693315.debug
-  uuid_str = module_uuid.GetAsString("");
-  std::transform(uuid_str.begin(), uuid_str.end(), uuid_str.begin(),
- ::tolower);
-  uuid_str.insert(2, 1, '/');
-  uuid_str = uuid_str + ".debug";
-}
+  std::string uuid_str;
+  const UUID _uuid = module_spec.GetUUID();
+  if (module_uuid.IsValid()) {
+// Some debug files are stored in the .build-id directory like this:
+//   /usr/lib/debug/.build-id/ff/e7fe727889ad82bb153de2ad065b2189693315.debug
+uuid_str = module_uuid.GetAsString("");
+std::transform(uuid_str.begin(), uuid_str.end(), uuid_str.begin(),
+   ::tolower);
+uuid_str.insert(2, 1, '/');
+uuid_str = uuid_str + ".debug";
+  }
 
-size_t num_directories = debug_file_search_paths.GetSize();
-for (size_t idx = 0; idx < num_directories; ++idx) {
-  FileSpec 

[Lldb-commits] [PATCH] D65560: ObjectFile[ELF]: Refactor gnu_debuglink interface

2019-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: clayborg, jankratochvil, mgorny.
Herald added subscribers: MaskRay, arichardson, emaste.
Herald added a reviewer: espindola.

The contents of the gnu_debuglink section were passed through the
GetDebugSymbolFilePaths interface, which was more generic than needed.
As the only class implementing this function is ObjectFileELF, we can
modify the function to return just a single FileSpec (instead of a
list). Also, since the SymbolVendorELF already assumes ELF object files,
we don't have to make this method available on the generic ObjectFile
interface -- instead we can put it on ObjectFileELF directly.

This change also makes is so that if the Module has an explicit symbol
file spec set, we disregard the value the value of the debug link
(instead of doing a secondary lookup using that). I think it makes sense
to honor the users wishes if he had explicitly set the symbol file spec,
and this seems to be consistent with what SymbolVendorMacOSX is doing
(SymbolVendorMacOSX.cpp:125).

The main reason for making these changes is to make the treatment of
build-ids and debug links simpler in the follow-up patch.


https://reviews.llvm.org/D65560

Files:
  include/lldb/Symbol/ObjectFile.h
  source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.h
  source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp

Index: source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
===
--- source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
+++ source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
@@ -71,86 +71,76 @@
   if (!uuid)
 return nullptr;
 
-  // Get the .gnu_debuglink file (if specified).
-  FileSpecList file_spec_list = obj_file->GetDebugSymbolFilePaths();
-
-  // If the module specified a filespec, use it first.
-  FileSpec debug_symbol_fspec(module_sp->GetSymbolFileFileSpec());
-  if (debug_symbol_fspec)
-file_spec_list.Insert(0, debug_symbol_fspec);
+  // If the module specified a filespec, use that.
+  FileSpec fspec = module_sp->GetSymbolFileFileSpec();
+  // Otherwise, try gnu_debuglink, if one exists.
+  if (!fspec)
+fspec = obj_file->GetDebugLink().getValueOr(FileSpec());
 
   // If we have no debug symbol files, then nothing to do.
-  if (file_spec_list.IsEmpty())
+  if (!fspec)
 return nullptr;
 
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, "SymbolVendorELF::CreateInstance (module = %s)",
  module_sp->GetFileSpec().GetPath().c_str());
 
-  for (size_t idx = 0; idx < file_spec_list.GetSize(); ++idx) {
-ModuleSpec module_spec;
-const FileSpec fspec = file_spec_list.GetFileSpecAtIndex(idx);
-
-module_spec.GetFileSpec() = obj_file->GetFileSpec();
-FileSystem::Instance().Resolve(module_spec.GetFileSpec());
-module_spec.GetSymbolFileSpec() = fspec;
-module_spec.GetUUID() = uuid;
-FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
-FileSpec dsym_fspec =
-Symbols::LocateExecutableSymbolFile(module_spec, search_paths);
-if (dsym_fspec) {
-  DataBufferSP dsym_file_data_sp;
-  lldb::offset_t dsym_file_data_offset = 0;
-  ObjectFileSP dsym_objfile_sp =
-  ObjectFile::FindPlugin(module_sp, _fspec, 0,
- FileSystem::Instance().GetByteSize(dsym_fspec),
- dsym_file_data_sp, dsym_file_data_offset);
-  if (dsym_objfile_sp) {
-// This objfile is for debugging purposes. Sadly, ObjectFileELF won't
-// be able to figure this out consistently as the symbol file may not
-// have stripped the code sections, etc.
-dsym_objfile_sp->SetType(ObjectFile::eTypeDebugInfo);
-
-SymbolVendorELF *symbol_vendor = new SymbolVendorELF(module_sp);
-if (symbol_vendor) {
-  // Get the module unified section list and add our debug sections to
-  // that.
-  SectionList *module_section_list = module_sp->GetSectionList();
-  SectionList *objfile_section_list = dsym_objfile_sp->GetSectionList();
-
-  static const SectionType g_sections[] = {
-  eSectionTypeDWARFDebugAbbrev,   eSectionTypeDWARFDebugAddr,
-  eSectionTypeDWARFDebugAranges,  eSectionTypeDWARFDebugCuIndex,
-  eSectionTypeDWARFDebugFrame,eSectionTypeDWARFDebugInfo,
-  eSectionTypeDWARFDebugLine, eSectionTypeDWARFDebugLoc,
-  eSectionTypeDWARFDebugMacInfo,  eSectionTypeDWARFDebugPubNames,
-  eSectionTypeDWARFDebugPubTypes, eSectionTypeDWARFDebugRanges,
-  eSectionTypeDWARFDebugStr,  eSectionTypeDWARFDebugStrOffsets,
-  eSectionTypeELFSymbolTable, eSectionTypeDWARFGNUDebugAltLink,
-  };
-  for (size_t idx = 0; idx < sizeof(g_sections) / sizeof(g_sections[0]);
- 

Re: [Lldb-commits] [lldb] r367414 - [ProcessWindows] Choose a register context file by preprocessor

2019-08-01 Thread Hans Wennborg via lldb-commits
Merged to release_90 in r367523.

On Wed, Jul 31, 2019 at 1:59 PM Tatyana Krasnukha via lldb-commits
 wrote:
>
> Author: tkrasnukha
> Date: Wed Jul 31 05:00:30 2019
> New Revision: 367414
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367414=rev
> Log:
> [ProcessWindows] Choose a register context file by preprocessor
>
> Replaced Cmake option based check with the preprocessor macro as 
> CMAKE_SYSTEM_PROCESSOR doesn't work as expected on Windows.
>
> Fixes llvm.org/pr42724
>
> Differential Revision: https://reviews.llvm.org/D65409
>
> Modified:
> lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
> 
> lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
> 
> lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h
> 
> lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
> 
> lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.h
>
> Modified: lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt?rev=367414=367413=367414=diff
> ==
> --- lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt (original)
> +++ lldb/trunk/source/Plugins/Process/Windows/Common/CMakeLists.txt Wed Jul 
> 31 05:00:30 2019
> @@ -7,6 +7,9 @@ add_lldb_library(lldbPluginProcessWindow
>ProcessWindowsLog.cpp
>RegisterContextWindows.cpp
>TargetThreadWindows.cpp
> +  x64/RegisterContextWindows_x64.cpp
> +  x86/RegisterContextWindows_x86.cpp
> +  # TODO add support for ARM (NT) and ARM64
>
>LINK_LIBS
>  lldbCore
> @@ -20,13 +23,3 @@ add_lldb_library(lldbPluginProcessWindow
>LINK_COMPONENTS
>  Support
>)
> -
> -# TODO add support for ARM (NT) and ARM64
> -# TODO build these unconditionally as we cannot do cross-debugging or WoW
> -if(CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64|AMD64")
> -  target_sources(lldbPluginProcessWindowsCommon PRIVATE
> -x64/RegisterContextWindows_x64.cpp)
> -elseif(CMAKE_SYSTEM_PROCESSOR MATCHES "i?86|X86")
> -  target_sources(lldbPluginProcessWindowsCommon PRIVATE
> -x86/RegisterContextWindows_x86.cpp)
> -endif()
>
> Modified: 
> lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp?rev=367414=367413=367414=diff
> ==
> --- 
> lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
>  (original)
> +++ 
> lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp
>  Wed Jul 31 05:00:30 2019
> @@ -6,6 +6,8 @@
>  //
>  
> //===--===//
>
> +#if defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
> defined(_M_AMD64)
> +
>  #include "lldb/Host/windows/HostThreadWindows.h"
>  #include "lldb/Host/windows/windows.h"
>  #include "lldb/Utility/RegisterValue.h"
> @@ -534,3 +536,5 @@ bool RegisterContextWindows_x64::WriteRe
>return ::SetThreadContext(
>wthread.GetHostThread().GetNativeThread().GetSystemHandle(), 
> _context);
>  }
> +
> +#endif // defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
> defined(_M_AMD64)
>
> Modified: 
> lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h?rev=367414=367413=367414=diff
> ==
> --- 
> lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h
>  (original)
> +++ 
> lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.h
>  Wed Jul 31 05:00:30 2019
> @@ -9,6 +9,8 @@
>  #ifndef liblldb_RegisterContextWindows_x64_H_
>  #define liblldb_RegisterContextWindows_x64_H_
>
> +#if defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
> defined(_M_AMD64)
> +
>  #include "RegisterContextWindows.h"
>  #include "lldb/lldb-forward.h"
>
> @@ -40,4 +42,6 @@ public:
>  };
>  }
>
> +#endif // defined(__x86_64__) || defined(__amd64__) || defined(_M_X64) || 
> defined(_M_AMD64)
> +
>  #endif // #ifndef liblldb_RegisterContextWindows_x64_H_
>
> Modified: 
> lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.cpp?rev=367414=367413=367414=diff
> ==
> --- 
> 

[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Implement per-thread execution control

2019-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D64647#1609712 , @mgorny wrote:

> Not sure if we're taking about the same thing. I meant the status in resume 
> actions. I think it'd be logical if 'no action' meant 'resume in same state 
> as prior to stopping the process', and explicit eStateS* meant 'keep this 
> thread stopped after resuming'.


Ah, you're right, we weren't talking about the same thing. I see what you mean 
now...

I can see how explicit modelling of suspension in resume actions would make 
sense. And it would definitely map better to how netbsd controls threads. OTOH, 
I think the current semantics (no action => keep stopped) makes sense too, and 
it maps better to how the gdb protocol represents resume actions. So either 
way, you have to do the conversion somewhere...


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

https://reviews.llvm.org/D64647



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


[Lldb-commits] [PATCH] D65489: Tablegen option enum value elements

2019-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D65489#1609001 , @JDevlieghere 
wrote:

> In D65489#1608936 , @labath wrote:
>
> > Yeah, the C arrays aren't the prettiest sight, but OTOH your tablegen files 
> > don't respect the column limit, and so if you view them with a 
> > line-wrapping editor (such as this phabricator page), they don't look 
> > particularly nice either.
>
>
> True, but at least you can grep without having to guess where the ling break 
> might have ended up :-)


Yeah, the grepping thing isn't nice, but I think working around that by putting 
the strings into tablegen is cheating  -- you're exploiting the fact that we 
don't have a code formatting tool nor a formal style guide for tablegen files. 
A lot of the official style guide cannot be directly applied to the tablegen 
files, but the column limit is one of the few things that can. :D

It looks like llvm tablegen files are not strictly adhering to that, but they 
are written with that limit in mind -- ascii art is 80 chars wide, and only 
2.5% of lines are 81 chars or over.

> 
> 
>> I'm not sure the result is better, but it is possible to control the way 
>> clang-format lays out arrays like this via trailing commas. If you include a 
>> trailing comma, it will put each entry on a separate line, which may be 
>> better for those long strings (though that's highly subjective). E.g.:
>> 
>>   static constexpr OptionEnumValueElement s_stop_show_column_values[] = {
>>   {
>>   eStopShowColumnAnsiOrCaret,
>>   "ansi-or-caret",
>>   "Highlight the stop column with ANSI terminal codes when 
>> color/ANSI "
>>   "mode is enabled; otherwise, fall back to using a text-only caret 
>> (^) "
>>   "as if \"caret-only\" mode was selected.",
>>   },
>>   {
>>   eStopShowColumnAnsi,
>>   "ansi",
>>   "Highlight the stop column with ANSI terminal codes when running 
>> LLDB "
>>   "with color/ANSI enabled.",
>>   },
>>   {
>>   eStopShowColumnCaret,
>>   "caret",
>>   "Highlight the stop column with a caret character (^) underneath 
>> the "
>>   "stop column. This method introduces a new line in source listings 
>> "
>>   "that display thread stop locations.",
>>   },
>>   {
>>   eStopShowColumnNone,
>>   "none",
>>   "Do not highlight the stop column.",
>>   },
>>   };
> 
> This looks much, much better imho. I can live with reformatting them if we 
> don't want to go the tablegen route.

Great, then I propose we go for that. :)


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

https://reviews.llvm.org/D65489



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


[Lldb-commits] [PATCH] D65555: [lldb] [Process/NetBSD] Enable reporting of new and exited threads [WIP]

2019-08-01 Thread Michał Górny via Phabricator via lldb-commits
mgorny marked an inline comment as done.
mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:738
+if (*it && ((*it)->GetID() == thread_id)) {
+  m_threads.erase(it);
+  break;

Note to self, this should be `it =... `


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

https://reviews.llvm.org/D6



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


[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Implement per-thread execution control

2019-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D64647#1609600 , @mgorny wrote:

> It seems that the patch is wrong with the current logic, and this is somehow 
> blurry because of Linux implementation limitations. FWIU, `eStateSuspended` 
> will never happen and instead we should keep threads without explicit action 
> suspsended. It's unclear yet whether this is desirable long-term, or if we 
> should change the logic to explicitly indicate state for each thread.


Yes, eStateSuspended will never happen on linux. It wouldn't be too hard to 
make use of that state -- we could set unresumed threads to "suspened" in 
NPL::Resume, and then when we go to stop all threads (NPL::StopRunningThreads), 
we would just avoid sending signals to suspended threads and just silently set 
them back to "stopped". However, I don't see a reason to do that now, as it 
would just add code for jumping between the states.

However, if it makes your life easier, I think you should be able to use 
eStateSuspended inside NetBSD code for anything you want. Nobody will inspect 
the state of threads while the process is running, so that could just be an 
implementation detail.

In fact I suspect the nobody will inspect the thread state whatsoever because 
once the process is stopped, all threads are assumed to be stopped too.


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

https://reviews.llvm.org/D64647



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


[Lldb-commits] [PATCH] D65555: [lldb] [Process/NetBSD] Enable reporting of new and exited threads [WIP]

2019-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Looks fine to me. Minor comments inline.




Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:271
+}
+SetState(StateType::eStateRunning, true);
+  } break;

I don't think you need this. Given that you never reported a stop event, as far 
as the rest of the world is concerned the process is still "running" (i.e., 
this "stop to notice a new thread" is just an implementation detail).



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:737
+  for (auto it = m_threads.begin(); it != m_threads.end(); ++it) {
+if (*it && ((*it)->GetID() == thread_id)) {
+  m_threads.erase(it);

It looks like the rest of the code assumes (as I think it should) that the 
thread list does not contain null pointers. See e.g. the loop on line 276.


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

https://reviews.llvm.org/D6



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


Re: [Lldb-commits] [lldb] r367441 - Don't crash when pass by value struct has no definition.

2019-08-01 Thread Pavel Labath via lldb-commits

On 01/08/2019 00:14, Jim Ingham via lldb-commits wrote:




On Jul 31, 2019, at 12:50 PM, via lldb-commits  
wrote:




-Original Message-
From: lldb-commits [mailto:lldb-commits-boun...@lists.llvm.org] On Behalf
Of Greg Clayton via lldb-commits
Sent: Wednesday, July 31, 2019 2:29 PM
To: Raphael Isemann
Cc: lldb-commits
Subject: Re: [Lldb-commits] [lldb] r367441 - Don't crash when pass by
value struct has no definition.




On Jul 31, 2019, at 10:57 AM, Raphael Isemann 

wrote:


It seems that patch is lacking a test (which doesn't seem too hard to

provide).

I am not the original author of this patch that was causing the crash,
just fixing a crash that was introduced by the patch.

I am all ears for anyone that can provide me with DWARF to help reproduce
this scenario where we have a DW_CC_pass_by_value struct with no
definition. Not sure how you would have a compiler that is passing a
struct to a function as a parameter and yet does not emit debug info for
that struct it is clearly using in the debug info.


Presumably one could construct a type DIE by hand which contains a 
DW_AT_declaration and a DW_AT_calling_convention. Even if this isn't the 
exact same way how the original bug came to be, it is a still a valid 
thing to test, and so it's better than no test at all.


That said, if the problem here is that the type is not fully defined, I 
am wondering if it wouldn't be better to move the calling convention 
stuff into the if(!is_forward_declaration) block right above it.

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