[Lldb-commits] [lldb] f21fccc - [LLDB] Fix 37 tests on Windows

[Adrian McCarthy via lldb-commits]

Author: Adrian McCarthy
Date: 2020-10-12T11:10:00-07:00
New Revision: f21fcccef7197f911a27b960aa2a180e0c7724aa

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

LOG: [LLDB] Fix 37 tests on Windows

A Windows-style LLDB_PYTHON_HOME path in a Cmake template didn't have the
backslashes escaped, which led to a garbled paths derived from it.  Fixed
by expanding the environment variable as a raw string literal.

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

Added: 


Modified: 
lldb/include/lldb/Host/Config.h.cmake

Removed: 




diff  --git a/lldb/include/lldb/Host/Config.h.cmake 
b/lldb/include/lldb/Host/Config.h.cmake
index 671d71d1c4e3..c667708a90a6 100644
--- a/lldb/include/lldb/Host/Config.h.cmake
+++ b/lldb/include/lldb/Host/Config.h.cmake
@@ -52,7 +52,7 @@
 
 #cmakedefine01 LLDB_EMBED_PYTHON_HOME
 
-#cmakedefine LLDB_PYTHON_HOME "${LLDB_PYTHON_HOME}"
+#cmakedefine LLDB_PYTHON_HOME R"(${LLDB_PYTHON_HOME})"
 
 #define LLDB_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}"
 



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

[Lldb-commits] [lldb] 7ddfb95 - [lldb] Fix unit test parsing to handle CR+LF as well as LF

[Adrian McCarthy via lldb-commits]

Author: Adrian McCarthy
Date: 2020-08-12T13:56:16-07:00
New Revision: 7ddfb956e1a5ee91d0d30f33ca0c84faeb471db4

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

LOG: [lldb] Fix unit test parsing to handle CR+LF as well as LF

Apparently when the strings are created, the `'\n'` is converted to the
platform's natural new line indicator, which is CR+LF on Windows.  But
upon reading back with `sscanf`, the CRs caused a matching failure.

Added: 


Modified: 
lldb/unittests/Utility/TimerTest.cpp

Removed: 




diff  --git a/lldb/unittests/Utility/TimerTest.cpp 
b/lldb/unittests/Utility/TimerTest.cpp
index 2d323c0dc2a3..c6d1facdebbb 100644
--- a/lldb/unittests/Utility/TimerTest.cpp
+++ b/lldb/unittests/Utility/TimerTest.cpp
@@ -97,7 +97,7 @@ TEST(TimerTest, CategoryTimesStats) {
   int count1, count2;
   ASSERT_EQ(
   6, sscanf(ss.GetData(),
-"%lf sec (total: %lfs; child: %lfs; count: %d) for CAT1%*[\n ]"
+"%lf sec (total: %lfs; child: %lfs; count: %d) for CAT1%*[\n\r 
]"
 "%lf sec (total: %*fs; child: %*fs; count: %d) for CAT2",
 , , , , , ))
   << "String: " << ss.GetData();



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

[Lldb-commits] [lldb] 479f5bf - [LLDB] Improve PDB discovery

[Adrian McCarthy via lldb-commits]

Author: Adrian McCarthy
Date: 2020-08-11T13:44:14-07:00
New Revision: 479f5bfdb02b191f03b3de1a7c3d5a5098b3fcaf

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

LOG: [LLDB] Improve PDB discovery

When loading a PE/COFF target, the associated PDB file often wasn't
found.  The executable module contains a path for the associated PDB
file, but people often debug from a different directory than the one
their build system uses.  (This is especially common in post-mortem
and cross platform debugging.)

Suppose the COFF executable being debugged is `~/proj/foo.exe`, but
it was built elsewhere and refers to `D:\remote\build\env\foobar.pdb`,
LLDB wouldn't find it.

With this change, if no file exists at the PDB path, LLDB will look
in the executable directory for a PDB file that matches the name of
the one it expected (e.g., `~/proj/foobar.pdb`).  If found, the PDB
is subject to the same matching criteria (GUIDs and age) as would
have been used had it been in the original location.

This same-directory-as-the-binary rule is commonly used by debuggers
on Windows.

Differential Review: https://reviews.llvm.org/D84815

Added: 
lldb/test/Shell/SymbolFile/NativePDB/Inputs/locate-pdb.lldbinit
lldb/test/Shell/SymbolFile/NativePDB/locate-pdb.cpp

Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 9f53e92afa0b..4f7244a340e7 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -134,8 +134,16 @@ loadMatchingPDBFile(std::string exe_path, 
llvm::BumpPtrAllocator ) {
 return nullptr;
   }
 
-  // if the file doesn't exist, is not a pdb, or doesn't have a matching guid,
-  // fail.
+  // If the file doesn't exist, perhaps the path specified at build time
+  // doesn't match the PDB's current location, so check the location of the
+  // executable.
+  if (!FileSystem::Instance().Exists(pdb_file)) {
+const auto exe_dir = FileSpec(exe_path).CopyByRemovingLastPathComponent();
+const auto pdb_name = FileSpec(pdb_file).GetFilename().GetCString();
+pdb_file = exe_dir.CopyByAppendingPathComponent(pdb_name).GetCString();
+  }
+
+  // If the file is not a PDB or if it doesn't have a matching GUID, fail.
   llvm::file_magic magic;
   auto ec = llvm::identify_magic(pdb_file, magic);
   if (ec || magic != llvm::file_magic::pdb)

diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/Inputs/locate-pdb.lldbinit 
b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/locate-pdb.lldbinit
new file mode 100644
index ..9d3eab7a3898
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/Inputs/locate-pdb.lldbinit
@@ -0,0 +1,2 @@
+target modules dump symfile
+quit

diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/locate-pdb.cpp 
b/lldb/test/Shell/SymbolFile/NativePDB/locate-pdb.cpp
new file mode 100644
index ..47abea129483
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/locate-pdb.cpp
@@ -0,0 +1,34 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Test that lldb can find the PDB file that corresponds to the executable.  
The linker
+// writes a path to the PDB in the executable.  If the PDB is not there, lldb 
should
+// check the directory that contains the executable.  We'll generate the PDB 
file in
+// a subdirectory and then move it into the directory with the executable.  
That will
+// ensure the PDB path stored in the executable is wrong.
+
+// Build an EXE and PDB in 
diff erent directories
+// RUN: mkdir -p %t/executable
+// RUN: rm -f %t/executable/foo.exe %t/executable/bar.pdb
+// RUN: mkdir -p %t/symbols
+// RUN: rm -f %t/symbols/bar.pdb
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c 
/Fo%t/executable/foo.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t/executable/foo.obj \
+// RUN: -out:%t/executable/foo.exe -pdb:%t/symbols/bar.pdb
+
+// Find the PDB in its build location
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t/executable/foo.exe -s \
+// RUN: %p/Inputs/locate-pdb.lldbinit | FileCheck %s
+
+// Also find the PDB when it's adjacent to the executable
+// RUN: mv -f %t/symbols/bar.pdb %t/executable/bar.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t/executable/foo.exe -s \
+// RUN: %p/Inputs/locate-pdb.lldbinit | FileCheck %s
+
+int main(int argc, char** argv) {
+  return 0;
+}
+
+// CHECK: (lldb) target modules dump symfile
+// CHECK: Dumping debug symbols for 1 modules.
+// CHECK: SymbolFile pdb



___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [lldb] 72958c9 - [lldb] Eliminated unused local variable

[Adrian McCarthy via lldb-commits]

Author: Adrian McCarthy
Date: 2020-07-16T14:44:24-07:00
New Revision: 72958c9ab1cdf18c447778b836f13694a7e4e9e1

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

LOG: [lldb] Eliminated unused local variable

I got misled by this remnant from earlier changes.

Added: 


Modified: 
lldb/source/Commands/CommandObjectTarget.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 7bb71f4d518c..e50415f930b3 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -4332,7 +4332,6 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 module_spec.GetSymbolFileSpec() = symfile_spec;
 }
 
-ArchSpec arch;
 bool symfile_exists =
 FileSystem::Instance().Exists(module_spec.GetSymbolFileSpec());
 



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

Re: [Lldb-commits] [lldb] b4a6e63 - [lldb/Target] Rework the way the inferior environment is created

[Adrian McCarthy via lldb-commits]

On Wed, Mar 25, 2020 at 9:49 AM Adrian McCarthy  wrote:

>
>
> On Wed, Mar 25, 2020 at 9:10 AM Pavel Labath  wrote:
>
>> On 25/03/2020 01:04, Adrian McCarthy wrote:
>> > I took a stab at this, but I'm not seeing any new test failures.
>>
>> That is odd.
>>
>> I was doing some stuff on windows today, so I figured I'd take a stab at
>> this. I was kind of right that the check in ProcessLauncher windows
>> prevents us from passing an empty environment.
>>
>> However, the interesting part starts when I tried to remove that check.
>> Then the test started behaving nondeterministically -- sometimes passing
>> and sometimes failing due to ERROR_INVALID_PARAMETER being returned from
>> CreateProcessW. I can see how windows might need some environment
>> variables to start up a process correctly, but I do not understand why
>> this should be nondeterministic...
>>
>
> Oh, I have a guess.  CreateProcessW takes a pointer to an environment
> block.  If that pointer is null, the process will inherit the parent
> environment.  If you want to pass it an empty environment, you have to have
> a valid pointer to an empty string (or possibly to a string with TWO
> terminating '\0's).
>

Scratch the "or possibly."  You definitely need two terminating zeros.
Since it's in UTF-16, it wants two L'\0', which is four consecutive zero
bytes.


>
>
>> This is beyond my knowledge of windows. It might be interesting to
>> reduce this to a simple test case (independent of lldb) and show it to
>> some windows expert.
>>
>> I am attaching a patch with the lldb changes I've made, in case you want
>> to play around with it.
>>
>> > I assume
>> > the problem you're seeing is in TestSettings.py, but I've never figured
>> > out how to run individual Python-based lldb tests since all the
>> > dotest.py stuff was re-written.
>>
>> These days we have multiple ways of achieving that. :)
>>
>> One way would be via the "check-lldb-api-commands-settings" target which
>> would run all tests under api/commands/settings (i.e. TestSettings.py
>> and TestQuoting.py).
>>
>> Another option would be via the lldb-dotest script.
>> "python bin\lldb-dotest -p TestSettings.py" would just run that single
>> file. That script passes all of its options to dotest, so you can use
>> any of the dotest options that you used to use.
>>
>
> I would be thrilled if either of those worked for me.  Somehow, check-lldb
> works, but the check-lldb-... doesn't.
>
> The lldb-dotest solution always fails for me because I can't figure out
> WTF it wants for PYTHONHOME and PYTHONPATH.
>
>
>>
>> pl
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] b4a6e63 - [lldb/Target] Rework the way the inferior environment is created

[Adrian McCarthy via lldb-commits]

On Wed, Mar 25, 2020 at 9:10 AM Pavel Labath  wrote:

> On 25/03/2020 01:04, Adrian McCarthy wrote:
> > I took a stab at this, but I'm not seeing any new test failures.
>
> That is odd.
>
> I was doing some stuff on windows today, so I figured I'd take a stab at
> this. I was kind of right that the check in ProcessLauncher windows
> prevents us from passing an empty environment.
>
> However, the interesting part starts when I tried to remove that check.
> Then the test started behaving nondeterministically -- sometimes passing
> and sometimes failing due to ERROR_INVALID_PARAMETER being returned from
> CreateProcessW. I can see how windows might need some environment
> variables to start up a process correctly, but I do not understand why
> this should be nondeterministic...
>

Oh, I have a guess.  CreateProcessW takes a pointer to an environment
block.  If that pointer is null, the process will inherit the parent
environment.  If you want to pass it an empty environment, you have to have
a valid pointer to an empty string (or possibly to a string with TWO
terminating '\0's).


> This is beyond my knowledge of windows. It might be interesting to
> reduce this to a simple test case (independent of lldb) and show it to
> some windows expert.
>
> I am attaching a patch with the lldb changes I've made, in case you want
> to play around with it.
>
> > I assume
> > the problem you're seeing is in TestSettings.py, but I've never figured
> > out how to run individual Python-based lldb tests since all the
> > dotest.py stuff was re-written.
>
> These days we have multiple ways of achieving that. :)
>
> One way would be via the "check-lldb-api-commands-settings" target which
> would run all tests under api/commands/settings (i.e. TestSettings.py
> and TestQuoting.py).
>
> Another option would be via the lldb-dotest script.
> "python bin\lldb-dotest -p TestSettings.py" would just run that single
> file. That script passes all of its options to dotest, so you can use
> any of the dotest options that you used to use.
>

I would be thrilled if either of those worked for me.  Somehow, check-lldb
works, but the check-lldb-... doesn't.

The lldb-dotest solution always fails for me because I can't figure out WTF
it wants for PYTHONHOME and PYTHONPATH.


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

Re: [Lldb-commits] [lldb] b4a6e63 - [lldb/Target] Rework the way the inferior environment is created

[Adrian McCarthy via lldb-commits]

Oh, and in case I wasn't clear:  I re-enabled the test TestSettings.py
locally.

On Tue, Mar 24, 2020 at 5:04 PM Adrian McCarthy  wrote:

> I took a stab at this, but I'm not seeing any new test failures.  Can you
> point me to the specific test and provide a log showing the failures?
>
> I've been using `ninja check-lldb`, which runs (almost everything) and
> none of the failures I'm seeing are related to inherit-env.  I assume the
> problem you're seeing is in TestSettings.py, but I've never figured out how
> to run individual Python-based lldb tests since all the dotest.py stuff was
> re-written.  I've started up lldb interactively and tried to emulate the
> commands that test executes via SBAPI, but everything worked as expected.
>
> Adrian.
>
> On Tue, Mar 24, 2020 at 8:19 AM Adrian McCarthy 
> wrote:
>
>> I'll take a look this morning.
>>
>> On Tue, Mar 24, 2020 at 7:00 AM Pavel Labath  wrote:
>>
>>> On 23/03/2020 17:17, Frédéric Riss via lldb-commits wrote:
>>> > The new testing for “inherit-env=false” is failing on Windows. I
>>> skipped the test for now.
>>> >
>>> > Could it be that it never worked there? (In which case XFail would be
>>> a better resolution)
>>> > Does anyone have easy access to a Windows build to try it out?
>>> >
>>> > Thanks,
>>> > Fred
>>>
>>> My guess is that this "defensive check"
>>> <
>>> https://github.com/llvm/llvm-project/blob/master/lldb/source/Host/windows/ProcessLauncherWindows.cpp#L26
>>> >
>>> prevents us from passing a completely blank environment. I am tempted to
>>> just delete it and see what happens, but maybe Adrian is able to do a
>>> quick test of this?
>>>
>>> pl
>>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] b4a6e63 - [lldb/Target] Rework the way the inferior environment is created

[Adrian McCarthy via lldb-commits]

I took a stab at this, but I'm not seeing any new test failures.  Can you
point me to the specific test and provide a log showing the failures?

I've been using `ninja check-lldb`, which runs (almost everything) and none
of the failures I'm seeing are related to inherit-env.  I assume the
problem you're seeing is in TestSettings.py, but I've never figured out how
to run individual Python-based lldb tests since all the dotest.py stuff was
re-written.  I've started up lldb interactively and tried to emulate the
commands that test executes via SBAPI, but everything worked as expected.

Adrian.

On Tue, Mar 24, 2020 at 8:19 AM Adrian McCarthy  wrote:

> I'll take a look this morning.
>
> On Tue, Mar 24, 2020 at 7:00 AM Pavel Labath  wrote:
>
>> On 23/03/2020 17:17, Frédéric Riss via lldb-commits wrote:
>> > The new testing for “inherit-env=false” is failing on Windows. I
>> skipped the test for now.
>> >
>> > Could it be that it never worked there? (In which case XFail would be a
>> better resolution)
>> > Does anyone have easy access to a Windows build to try it out?
>> >
>> > Thanks,
>> > Fred
>>
>> My guess is that this "defensive check"
>> <
>> https://github.com/llvm/llvm-project/blob/master/lldb/source/Host/windows/ProcessLauncherWindows.cpp#L26
>> >
>> prevents us from passing a completely blank environment. I am tempted to
>> just delete it and see what happens, but maybe Adrian is able to do a
>> quick test of this?
>>
>> pl
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] b4a6e63 - [lldb/Target] Rework the way the inferior environment is created

[Adrian McCarthy via lldb-commits]

I'll take a look this morning.

On Tue, Mar 24, 2020 at 7:00 AM Pavel Labath  wrote:

> On 23/03/2020 17:17, Frédéric Riss via lldb-commits wrote:
> > The new testing for “inherit-env=false” is failing on Windows. I skipped
> the test for now.
> >
> > Could it be that it never worked there? (In which case XFail would be a
> better resolution)
> > Does anyone have easy access to a Windows build to try it out?
> >
> > Thanks,
> > Fred
>
> My guess is that this "defensive check"
> <
> https://github.com/llvm/llvm-project/blob/master/lldb/source/Host/windows/ProcessLauncherWindows.cpp#L26
> >
> prevents us from passing a completely blank environment. I am tempted to
> just delete it and see what happens, but maybe Adrian is able to do a
> quick test of this?
>
> pl
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] fb0d2d4 - Fix after c25938d

[Adrian McCarthy via lldb-commits]

Author: Adrian McCarthy
Date: 2020-02-04T16:37:22-08:00
New Revision: fb0d2d455f56bca239041c8d7ad7b57da1087b35

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

LOG: Fix after c25938d

My refactor caused some changes in error reporting that TestAddDsymCommand.py
was checking, so this restores some of the changes to preserve the old
behavior and to un-xfail the affected test.

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

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/commands/add-dsym/uuid/TestAddDsymCommand.py
lldb/source/Commands/CommandObjectTarget.cpp

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/add-dsym/uuid/TestAddDsymCommand.py
 
b/lldb/packages/Python/lldbsuite/test/commands/add-dsym/uuid/TestAddDsymCommand.py
index 91e4c6a9e101..8a0fe377f268 100644
--- 
a/lldb/packages/Python/lldbsuite/test/commands/add-dsym/uuid/TestAddDsymCommand.py
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/add-dsym/uuid/TestAddDsymCommand.py
@@ -22,7 +22,6 @@ def setUp(self):
 self.teardown_hook_added = False
 
 @no_debug_info_test
-@expectedFailureDarwin('until AdrianM or I find a fix for his 2020-02-03 
CommandObjectTarget change')
 def test_add_dsym_command_with_error(self):
 """Test that the 'add-dsym' command informs the user about failures."""
 

diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 0239f7e33e02..50eb46fafca8 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -4119,23 +4119,6 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
   target->GetImages().FindModules(module_spec, matching_modules);
 }
 
-if (matching_modules.IsEmpty()) {
-  StreamString ss_symfile_uuid;
-  if (module_spec.GetUUID().IsValid()) {
-ss_symfile_uuid << " (";
-module_spec.GetUUID().Dump(_symfile_uuid);
-ss_symfile_uuid << ')';
-  }
-  result.AppendErrorWithFormat(
-  "symbol file '%s'%s does not match any existing module%s\n",
-  symfile_path, ss_symfile_uuid.GetData(),
-  !llvm::sys::fs::is_regular_file(symbol_fspec.GetPath())
-  ? "\n   please specify the full path to the symbol file"
-  : "");
-  result.SetStatus(eReturnStatusFailed);
-  return false;
-}
-
 if (matching_modules.GetSize() > 1) {
   result.AppendErrorWithFormat("multiple modules match symbol file '%s', "
"use the --uuid option to resolve the "
@@ -4144,65 +4127,72 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
   result.SetStatus(eReturnStatusFailed);
   return false;
 }
-
-assert(matching_modules.GetSize() == 1);
-ModuleSP module_sp(matching_modules.GetModuleAtIndex(0));
-
-// The module has not yet created its symbol vendor, we can just give
-// the existing target module the symfile path to use for when it
-// decides to create it!
-module_sp->SetSymbolFileFileSpec(symbol_fspec);
-
-SymbolFile *symbol_file =
-module_sp->GetSymbolFile(true, ());
-if (!symbol_file) {
-  result.AppendErrorWithFormat("symbol file '%s' could not be loaded\n",
-   symfile_path);
-  result.SetStatus(eReturnStatusFailed);
-  module_sp->SetSymbolFileFileSpec(FileSpec());
-  return false;
-}
 
-ObjectFile *object_file = symbol_file->GetObjectFile();
-if (!object_file || object_file->GetFileSpec() != symbol_fspec) {
-  result.AppendError("the object file could not be loaded\n");
-  result.SetStatus(eReturnStatusFailed);
+if (matching_modules.GetSize() == 1) {
+  ModuleSP module_sp(matching_modules.GetModuleAtIndex(0));
+
+  // The module has not yet created its symbol vendor, we can just give
+  // the existing target module the symfile path to use for when it
+  // decides to create it!
+  module_sp->SetSymbolFileFileSpec(symbol_fspec);
+
+  SymbolFile *symbol_file =
+  module_sp->GetSymbolFile(true, ());
+  if (symbol_file) {
+ObjectFile *object_file = symbol_file->GetObjectFile();
+if (object_file && object_file->GetFileSpec() == symbol_fspec) {
+  // Provide feedback that the symfile has been successfully added.
+  const FileSpec _fs = module_sp->GetFileSpec();
+  result.AppendMessageWithFormat(
+  "symbol file '%s' has been added to '%s'\n", symfile_path,
+  module_fs.GetPath().c_str());
+
+  // Let clients know something changed in the module if it is
+  // currently loaded
+  ModuleList module_list;

Re: [Lldb-commits] [lldb] c25938d - Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

[Adrian McCarthy via lldb-commits]

Suppose we give the traditional message (making the users and the test
happy) and then augment it with a little extra text to let developers
figure out where it went wrong.

symbol file '%s'%s does not match any existing module%s

symbol file '%s'%s does not match any existing module%s
  (the symbol file could not be loaded)

symbol file '%s'%s does not match any existing module%s
  (the object file could not be loaded)

Adrian.

On Tue, Feb 4, 2020 at 12:53 PM Jim Ingham  wrote:

> The question isn't so much whether it makes a difference to the user than
> whether when somebody reports getting this error in a project that we can't
> get our hands on so all we have is the screen output (a very not uncommon
> circumstance for debuggers) the different error messages would be useful to
> us...
>
> Jim
>
>
> > On Feb 4, 2020, at 12:47 PM, Adrian McCarthy 
> wrote:
> >
> > Given that the UUIDs in the test don't actually match, I think it's
> reasonable for the error message to say it's not a match.  I'm not sure
> detecting the problem in a different step of the process makes that much
> difference to the user that it warrants a different message.  (I know it
> sounds like I'm waffling.)
> >
> > I'm happy to let others (e.g., Jason) make the call.  These are
> Darwin-only tests, so I don't have a handy way to work through them.
> >
> > On Tue, Feb 4, 2020 at 12:07 PM Jim Ingham  wrote:
> > Seems like your change is more informative.  Could we just fix the tests?
> >
> > Jim
> >
> >
> > > On Feb 4, 2020, at 11:18 AM, Adrian McCarthy via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> > >
> > > I see why the behavior is different, but I'm not sure whether the new
> behavior is wrong.
> > >
> > > The old code nested a bunch of if-success statements so that any
> failure would fall through to a default error message (the one the tests is
> expecting).
> > >
> > > The new code is a linear set of if-failure-return statements so that
> failures at different steps get different messages.  (So, arguably, the new
> error message is more specific to the failure, even though it's worded
> rather vaguely.)
> > >
> > > I can send a patch to use the original message for each of the three
> error types that are now distinct.
> > >
> > > On Tue, Feb 4, 2020 at 8:49 AM Adrian McCarthy 
> wrote:
> > > Sorry.  I can check this out this morning.
> > >
> > > I didn't have any test failures locally, but I'm guessing that test
> doesn't run on Windows.
> > >
> > > On Mon, Feb 3, 2020 at 6:43 PM Jason Molenda 
> wrote:
> > > I'm going to xfail it for tonight; we end up copying the base filename
> into the ModuleSpec that we use for searching,
> > >
> > > if (!module_spec.GetUUID().IsValid()) {
> > >   if (!module_spec.GetFileSpec() &&
> !module_spec.GetPlatformFileSpec())
> > > module_spec.GetFileSpec().GetFilename() =
> symbol_fspec.GetFilename();
> > > }
> > >
> > > So when the binary is /dir1/a.out and the (wrong) dSYM is
> /dir2/a.out.dSYM/Contents/Resources/DWARF/a.out, and we did 'target symbols
> add /dir2/a.out.dSYM', "a.out" gets copied into module_spec and after
> Adrian's change, we search through the Target image list for a file called
> "a.out", find one (the wrong one), try to construct a ModuleSP with that as
> the ObjectFile and the /dir2/a.out.dSYM as the SymbolFile and it gets
> caught as inconsistent and we return the new, less helpful, error message.
> > >
> > > I looked over his patch and I'm not sure where the logic went wrong
> here; I'll step through the old version tomorrow.
> > >
> > > J
> > >
> > >
> > > > On Feb 3, 2020, at 4:18 PM, Jason Molenda 
> wrote:
> > > >
> > > > This is causing a test failure on the macos cmake bot in
> TestAddDsymCommand.py where we load a binary with uuid A, then try to
> add-dsym a dSYM uuid B and the test is looking for the error text,
> > > >
> > > > error: symbol file 'FILENAME' does not match any existing module
> > > >
> > > > but we're now getting,
> > > >
> > > > error: the object file could not be loaded
> > > >
> > > > and the test doesn't expect that.
> > > >
> > > >
> > > > I'm looking into this, to see if I can get the more specific error
> message back in here again.  I may xfail the test if I don't have a patch
> done tonight.  I'd rather not test for this new generic error message

Re: [Lldb-commits] [lldb] c25938d - Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

[Adrian McCarthy via lldb-commits]

Given that the UUIDs in the test don't actually match, I think it's
reasonable for the error message to say it's not a match.  I'm not sure
detecting the problem in a different step of the process makes that much
difference to the user that it warrants a different message.  (I know it
sounds like I'm waffling.)

I'm happy to let others (e.g., Jason) make the call.  These are Darwin-only
tests, so I don't have a handy way to work through them.

On Tue, Feb 4, 2020 at 12:07 PM Jim Ingham  wrote:

> Seems like your change is more informative.  Could we just fix the tests?
>
> Jim
>
>
> > On Feb 4, 2020, at 11:18 AM, Adrian McCarthy via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> > I see why the behavior is different, but I'm not sure whether the new
> behavior is wrong.
> >
> > The old code nested a bunch of if-success statements so that any failure
> would fall through to a default error message (the one the tests is
> expecting).
> >
> > The new code is a linear set of if-failure-return statements so that
> failures at different steps get different messages.  (So, arguably, the new
> error message is more specific to the failure, even though it's worded
> rather vaguely.)
> >
> > I can send a patch to use the original message for each of the three
> error types that are now distinct.
> >
> > On Tue, Feb 4, 2020 at 8:49 AM Adrian McCarthy 
> wrote:
> > Sorry.  I can check this out this morning.
> >
> > I didn't have any test failures locally, but I'm guessing that test
> doesn't run on Windows.
> >
> > On Mon, Feb 3, 2020 at 6:43 PM Jason Molenda  wrote:
> > I'm going to xfail it for tonight; we end up copying the base filename
> into the ModuleSpec that we use for searching,
> >
> > if (!module_spec.GetUUID().IsValid()) {
> >   if (!module_spec.GetFileSpec() &&
> !module_spec.GetPlatformFileSpec())
> > module_spec.GetFileSpec().GetFilename() =
> symbol_fspec.GetFilename();
> > }
> >
> > So when the binary is /dir1/a.out and the (wrong) dSYM is
> /dir2/a.out.dSYM/Contents/Resources/DWARF/a.out, and we did 'target symbols
> add /dir2/a.out.dSYM', "a.out" gets copied into module_spec and after
> Adrian's change, we search through the Target image list for a file called
> "a.out", find one (the wrong one), try to construct a ModuleSP with that as
> the ObjectFile and the /dir2/a.out.dSYM as the SymbolFile and it gets
> caught as inconsistent and we return the new, less helpful, error message.
> >
> > I looked over his patch and I'm not sure where the logic went wrong
> here; I'll step through the old version tomorrow.
> >
> > J
> >
> >
> > > On Feb 3, 2020, at 4:18 PM, Jason Molenda  wrote:
> > >
> > > This is causing a test failure on the macos cmake bot in
> TestAddDsymCommand.py where we load a binary with uuid A, then try to
> add-dsym a dSYM uuid B and the test is looking for the error text,
> > >
> > > error: symbol file 'FILENAME' does not match any existing module
> > >
> > > but we're now getting,
> > >
> > > error: the object file could not be loaded
> > >
> > > and the test doesn't expect that.
> > >
> > >
> > > I'm looking into this, to see if I can get the more specific error
> message back in here again.  I may xfail the test if I don't have a patch
> done tonight.  I'd rather not test for this new generic error message if
> it's avoidable.
> > >
> > >
> > >
> > >
> > > J
> > >
> > >
> > >> On Feb 3, 2020, at 2:22 PM, Adrian McCarthy via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> > >>
> > >>
> > >> Author: Adrian McCarthy
> > >> Date: 2020-02-03T14:22:05-08:00
> > >> New Revision: c25938d57b1cf9534887313405bc409e570a9b69
> > >>
> > >> URL:
> https://github.com/llvm/llvm-project/commit/c25938d57b1cf9534887313405bc409e570a9b69
> > >> DIFF:
> https://github.com/llvm/llvm-project/commit/c25938d57b1cf9534887313405bc409e570a9b69.diff
> > >>
> > >> LOG: Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols
> > >>
> > >> * [NFC] Renamed local `matching_module_list` to `matching_modules` for
> > >> conciseness.
> > >>
> > >> * [NFC] Eliminated redundant local variable `num_matches` to reduce
> the risk
> > >> that changes get it out of sync with `matching_modules.GetSize()`.
> > >>
> > >> * Used an early return from case whe

Re: [Lldb-commits] [lldb] c25938d - Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

[Adrian McCarthy via lldb-commits]

I see why the behavior is different, but I'm not sure whether the new
behavior is wrong.

The old code nested a bunch of if-success statements so that any failure
would fall through to a default error message (the one the tests is
expecting).

The new code is a linear set of if-failure-return statements so that
failures at different steps get different messages.  (So, arguably, the new
error message is more specific to the failure, even though it's worded
rather vaguely.)

I can send a patch to use the original message for each of the three error
types that are now distinct.

On Tue, Feb 4, 2020 at 8:49 AM Adrian McCarthy  wrote:

> Sorry.  I can check this out this morning.
>
> I didn't have any test failures locally, but I'm guessing that test
> doesn't run on Windows.
>
> On Mon, Feb 3, 2020 at 6:43 PM Jason Molenda  wrote:
>
>> I'm going to xfail it for tonight; we end up copying the base filename
>> into the ModuleSpec that we use for searching,
>>
>> if (!module_spec.GetUUID().IsValid()) {
>>   if (!module_spec.GetFileSpec() &&
>> !module_spec.GetPlatformFileSpec())
>> module_spec.GetFileSpec().GetFilename() =
>> symbol_fspec.GetFilename();
>> }
>>
>> So when the binary is /dir1/a.out and the (wrong) dSYM is
>> /dir2/a.out.dSYM/Contents/Resources/DWARF/a.out, and we did 'target symbols
>> add /dir2/a.out.dSYM', "a.out" gets copied into module_spec and after
>> Adrian's change, we search through the Target image list for a file called
>> "a.out", find one (the wrong one), try to construct a ModuleSP with that as
>> the ObjectFile and the /dir2/a.out.dSYM as the SymbolFile and it gets
>> caught as inconsistent and we return the new, less helpful, error message.
>>
>> I looked over his patch and I'm not sure where the logic went wrong here;
>> I'll step through the old version tomorrow.
>>
>> J
>>
>>
>> > On Feb 3, 2020, at 4:18 PM, Jason Molenda  wrote:
>> >
>> > This is causing a test failure on the macos cmake bot in
>> TestAddDsymCommand.py where we load a binary with uuid A, then try to
>> add-dsym a dSYM uuid B and the test is looking for the error text,
>> >
>> > error: symbol file 'FILENAME' does not match any existing module
>> >
>> > but we're now getting,
>> >
>> > error: the object file could not be loaded
>> >
>> > and the test doesn't expect that.
>> >
>> >
>> > I'm looking into this, to see if I can get the more specific error
>> message back in here again.  I may xfail the test if I don't have a patch
>> done tonight.  I'd rather not test for this new generic error message if
>> it's avoidable.
>> >
>> >
>> >
>> >
>> > J
>> >
>> >
>> >> On Feb 3, 2020, at 2:22 PM, Adrian McCarthy via lldb-commits <
>> lldb-commits@lists.llvm.org> wrote:
>> >>
>> >>
>> >> Author: Adrian McCarthy
>> >> Date: 2020-02-03T14:22:05-08:00
>> >> New Revision: c25938d57b1cf9534887313405bc409e570a9b69
>> >>
>> >> URL:
>> https://github.com/llvm/llvm-project/commit/c25938d57b1cf9534887313405bc409e570a9b69
>> >> DIFF:
>> https://github.com/llvm/llvm-project/commit/c25938d57b1cf9534887313405bc409e570a9b69.diff
>> >>
>> >> LOG: Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols
>> >>
>> >> * [NFC] Renamed local `matching_module_list` to `matching_modules` for
>> >> conciseness.
>> >>
>> >> * [NFC] Eliminated redundant local variable `num_matches` to reduce
>> the risk
>> >> that changes get it out of sync with `matching_modules.GetSize()`.
>> >>
>> >> * Used an early return from case where the symbol file specified
>> matches
>> >> multiple modules.  This is a slight behavior change, but it's an
>> improvement:
>> >> It didn't make sense to tell the user that the symbol file
>> simultaneously
>> >> matched multiple modules and no modules.
>> >>
>> >> * [NFC] Used an early return from the case where no matches are found,
>> to
>> >> better align with LLVM coding style.
>> >>
>> >> * [NFC] Simplified call of `AppendWarningWithFormat("%s", stuff)` to
>> >> `AppendWarning(stuff)`.  I don't think this adds any copies.  It does
>> >> construct a StringRef, but it was going to have to scan the string for
>> the
>> >> length anyway.
>> >>
>> >> *

Re: [Lldb-commits] [lldb] c25938d - Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

[Adrian McCarthy via lldb-commits]

Sorry.  I can check this out this morning.

I didn't have any test failures locally, but I'm guessing that test doesn't
run on Windows.

On Mon, Feb 3, 2020 at 6:43 PM Jason Molenda  wrote:

> I'm going to xfail it for tonight; we end up copying the base filename
> into the ModuleSpec that we use for searching,
>
> if (!module_spec.GetUUID().IsValid()) {
>   if (!module_spec.GetFileSpec() && !module_spec.GetPlatformFileSpec())
> module_spec.GetFileSpec().GetFilename() =
> symbol_fspec.GetFilename();
> }
>
> So when the binary is /dir1/a.out and the (wrong) dSYM is
> /dir2/a.out.dSYM/Contents/Resources/DWARF/a.out, and we did 'target symbols
> add /dir2/a.out.dSYM', "a.out" gets copied into module_spec and after
> Adrian's change, we search through the Target image list for a file called
> "a.out", find one (the wrong one), try to construct a ModuleSP with that as
> the ObjectFile and the /dir2/a.out.dSYM as the SymbolFile and it gets
> caught as inconsistent and we return the new, less helpful, error message.
>
> I looked over his patch and I'm not sure where the logic went wrong here;
> I'll step through the old version tomorrow.
>
> J
>
>
> > On Feb 3, 2020, at 4:18 PM, Jason Molenda  wrote:
> >
> > This is causing a test failure on the macos cmake bot in
> TestAddDsymCommand.py where we load a binary with uuid A, then try to
> add-dsym a dSYM uuid B and the test is looking for the error text,
> >
> > error: symbol file 'FILENAME' does not match any existing module
> >
> > but we're now getting,
> >
> > error: the object file could not be loaded
> >
> > and the test doesn't expect that.
> >
> >
> > I'm looking into this, to see if I can get the more specific error
> message back in here again.  I may xfail the test if I don't have a patch
> done tonight.  I'd rather not test for this new generic error message if
> it's avoidable.
> >
> >
> >
> >
> > J
> >
> >
> >> On Feb 3, 2020, at 2:22 PM, Adrian McCarthy via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >>
> >>
> >> Author: Adrian McCarthy
> >> Date: 2020-02-03T14:22:05-08:00
> >> New Revision: c25938d57b1cf9534887313405bc409e570a9b69
> >>
> >> URL:
> https://github.com/llvm/llvm-project/commit/c25938d57b1cf9534887313405bc409e570a9b69
> >> DIFF:
> https://github.com/llvm/llvm-project/commit/c25938d57b1cf9534887313405bc409e570a9b69.diff
> >>
> >> LOG: Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols
> >>
> >> * [NFC] Renamed local `matching_module_list` to `matching_modules` for
> >> conciseness.
> >>
> >> * [NFC] Eliminated redundant local variable `num_matches` to reduce the
> risk
> >> that changes get it out of sync with `matching_modules.GetSize()`.
> >>
> >> * Used an early return from case where the symbol file specified matches
> >> multiple modules.  This is a slight behavior change, but it's an
> improvement:
> >> It didn't make sense to tell the user that the symbol file
> simultaneously
> >> matched multiple modules and no modules.
> >>
> >> * [NFC] Used an early return from the case where no matches are found,
> to
> >> better align with LLVM coding style.
> >>
> >> * [NFC] Simplified call of `AppendWarningWithFormat("%s", stuff)` to
> >> `AppendWarning(stuff)`.  I don't think this adds any copies.  It does
> >> construct a StringRef, but it was going to have to scan the string for
> the
> >> length anyway.
> >>
> >> * [NFC] Removed unnecessary comments and reworded others for clarity.
> >>
> >> * Used an early return if the symbol file could not be loaded.  This is
> a
> >> behavior change because previously it could fail silently.
> >>
> >> * Used an early return if the object file could not be retrieved from
> the
> >> symbol file.  Again, this is a change because now there's an error
> message.
> >>
> >> * [NFC] Eliminated a namespace alias that wasn't particularly helpful.
> >>
> >> Differential Revision: https://reviews.llvm.org/D73594
> >>
> >> Added:
> >>
> >>
> >> Modified:
> >>   lldb/source/Commands/CommandObjectTarget.cpp
> >>
> >> Removed:
> >>
> >>
> >>
> >>
> 
> >> diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp
> b/lldb/

[Lldb-commits] [lldb] c25938d - Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

[Adrian McCarthy via lldb-commits]

Author: Adrian McCarthy
Date: 2020-02-03T14:22:05-08:00
New Revision: c25938d57b1cf9534887313405bc409e570a9b69

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

LOG: Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

* [NFC] Renamed local `matching_module_list` to `matching_modules` for
conciseness.

* [NFC] Eliminated redundant local variable `num_matches` to reduce the risk
that changes get it out of sync with `matching_modules.GetSize()`.

* Used an early return from case where the symbol file specified matches
multiple modules.  This is a slight behavior change, but it's an improvement:
It didn't make sense to tell the user that the symbol file simultaneously
matched multiple modules and no modules.

* [NFC] Used an early return from the case where no matches are found, to
better align with LLVM coding style.

* [NFC] Simplified call of `AppendWarningWithFormat("%s", stuff)` to
`AppendWarning(stuff)`.  I don't think this adds any copies.  It does
construct a StringRef, but it was going to have to scan the string for the
length anyway.

* [NFC] Removed unnecessary comments and reworded others for clarity.

* Used an early return if the symbol file could not be loaded.  This is a
behavior change because previously it could fail silently.

* Used an early return if the object file could not be retrieved from the
symbol file.  Again, this is a change because now there's an error message.

* [NFC] Eliminated a namespace alias that wasn't particularly helpful.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectTarget.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 1b51fbeb71d3..b08a29d081a0 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -4053,12 +4053,10 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 module_spec.GetFileSpec().GetFilename() = symbol_fspec.GetFilename();
 }
 
-// We now have a module that represents a symbol file that can be used
-// for a module that might exist in the current target, so we need to
-// find that module in the target
-ModuleList matching_module_list;
+// Now module_spec represents a symbol file for a module that might exist
+// in the current target.  Let's find possible matches.
+ModuleList matching_modules;
 
-size_t num_matches = 0;
 // First extract all module specs from the symbol file
 lldb_private::ModuleSpecList symfile_module_specs;
 if (ObjectFile::GetModuleSpecifications(module_spec.GetSymbolFileSpec(),
@@ -4069,34 +4067,30 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
   target_arch_module_spec.GetArchitecture() = target->GetArchitecture();
   if (symfile_module_specs.FindMatchingModuleSpec(target_arch_module_spec,
   symfile_module_spec)) {
-// See if it has a UUID?
 if (symfile_module_spec.GetUUID().IsValid()) {
   // It has a UUID, look for this UUID in the target modules
   ModuleSpec symfile_uuid_module_spec;
   symfile_uuid_module_spec.GetUUID() = symfile_module_spec.GetUUID();
   target->GetImages().FindModules(symfile_uuid_module_spec,
-  matching_module_list);
-  num_matches = matching_module_list.GetSize();
+  matching_modules);
 }
   }
 
-  if (num_matches == 0) {
-// No matches yet, iterate through the module specs to find a UUID
-// value that we can match up to an image in our target
-const size_t num_symfile_module_specs =
-symfile_module_specs.GetSize();
-for (size_t i = 0; i < num_symfile_module_specs && num_matches == 0;
-  ++i) {
+  if (matching_modules.IsEmpty()) {
+// No matches yet.  Iterate through the module specs to find a UUID
+// value that we can match up to an image in our target.
+const size_t num_symfile_module_specs = symfile_module_specs.GetSize();
+for (size_t i = 0;
+ i < num_symfile_module_specs && matching_modules.IsEmpty(); ++i) {
   if (symfile_module_specs.GetModuleSpecAtIndex(
   i, symfile_module_spec)) {
 if (symfile_module_spec.GetUUID().IsValid()) {
-  // It has a UUID, look for this UUID in the target modules
+  // It has a UUID.  Look for this UUID in the target modules.
   ModuleSpec symfile_uuid_module_spec;
   symfile_uuid_module_spec.GetUUID() =
   

[Lldb-commits] [lldb] 0e362d8 - Improve help text for (lldb) target symbols add

[Adrian McCarthy via lldb-commits]

Author: Adrian McCarthy
Date: 2020-02-03T14:22:05-08:00
New Revision: 0e362d82b97fe16b5671ceeb2f3e5b6f4ccf00dd

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

LOG: Improve help text for (lldb) target symbols add

There were some missing words and awkward syntax.  I think this is clearer.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectTarget.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index b08a29d081a0..0239f7e33e02 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3999,19 +3999,20 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
   : CommandObjectParsed(
 interpreter, "target symbols add",
 "Add a debug symbol file to one of the target's current modules by 
"
-"specifying a path to a debug symbols file, or using the options "
-"to specify a module to download symbols for.",
+"specifying a path to a debug symbols file or by using the options 
"
+"to specify a module.",
 "target symbols add  []",
 eCommandRequiresTarget),
 m_option_group(),
 m_file_option(
 LLDB_OPT_SET_1, false, "shlib", 's',
 CommandCompletions::eModuleCompletion, eArgTypeShlibName,
-"Fullpath or basename for module to find debug symbols for."),
+"Locate the debug symbols for the shared library specified by "
+"name."),
 m_current_frame_option(
 LLDB_OPT_SET_2, false, "frame", 'F',
-"Locate the debug symbols the currently selected frame.", false,
-true)
+"Locate the debug symbols for the currently selected frame.",
+false, true)
 
   {
 m_option_group.Append(_uuid_option_group, LLDB_OPT_SET_ALL,



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

[Lldb-commits] [lldb] 3b69f0c - [NFC] Refactor and improve comments in CommandObjectTarget

[Adrian McCarthy via lldb-commits]

Author: Adrian McCarthy
Date: 2019-11-21T08:37:35-08:00
New Revision: 3b69f0c5550a229dd6d39e361182cdd7cecc36a4

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

LOG: [NFC] Refactor and improve comments in CommandObjectTarget

Made small improvements while debugging through
CommandObjectTarget::AddModuleSymbols.

1.  Refactored error case for an early out, reducing the indentation of
the rest of this long function.
2.  Clarified some comments by correcting spelling and punctuation.
3.  Reduced duplicate code at the end of the function.

Tested with `ninja check-lldb`

Differential Review: https://reviews.llvm.org/D70458

Added: 


Modified: 
lldb/source/Commands/CommandObjectTarget.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index 64e7e691cf82..90578d2b0092 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -4044,169 +4044,165 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
   bool AddModuleSymbols(Target *target, ModuleSpec _spec, bool ,
 CommandReturnObject ) {
 const FileSpec _fspec = module_spec.GetSymbolFileSpec();
-if (symbol_fspec) {
-  char symfile_path[PATH_MAX];
-  symbol_fspec.GetPath(symfile_path, sizeof(symfile_path));
+if (!symbol_fspec) {
+  result.AppendError(
+  "one or more executable image paths must be specified");
+  result.SetStatus(eReturnStatusFailed);
+  return false;
+}
 
-  if (!module_spec.GetUUID().IsValid()) {
-if (!module_spec.GetFileSpec() && !module_spec.GetPlatformFileSpec())
-  module_spec.GetFileSpec().GetFilename() = symbol_fspec.GetFilename();
-  }
-  // We now have a module that represents a symbol file that can be used
-  // for a module that might exist in the current target, so we need to
-  // find that module in the target
-  ModuleList matching_module_list;
-
-  size_t num_matches = 0;
-  // First extract all module specs from the symbol file
-  lldb_private::ModuleSpecList symfile_module_specs;
-  if (ObjectFile::GetModuleSpecifications(module_spec.GetSymbolFileSpec(),
-  0, 0, symfile_module_specs)) {
-// Now extract the module spec that matches the target architecture
-ModuleSpec target_arch_module_spec;
-ModuleSpec symfile_module_spec;
-target_arch_module_spec.GetArchitecture() = target->GetArchitecture();
-if 
(symfile_module_specs.FindMatchingModuleSpec(target_arch_module_spec,
-symfile_module_spec)) {
-  // See if it has a UUID?
-  if (symfile_module_spec.GetUUID().IsValid()) {
-// It has a UUID, look for this UUID in the target modules
-ModuleSpec symfile_uuid_module_spec;
-symfile_uuid_module_spec.GetUUID() = symfile_module_spec.GetUUID();
-target->GetImages().FindModules(symfile_uuid_module_spec,
-matching_module_list);
-num_matches = matching_module_list.GetSize();
-  }
+char symfile_path[PATH_MAX];
+symbol_fspec.GetPath(symfile_path, sizeof(symfile_path));
+
+if (!module_spec.GetUUID().IsValid()) {
+  if (!module_spec.GetFileSpec() && !module_spec.GetPlatformFileSpec())
+module_spec.GetFileSpec().GetFilename() = symbol_fspec.GetFilename();
+}
+
+// We now have a module that represents a symbol file that can be used
+// for a module that might exist in the current target, so we need to
+// find that module in the target
+ModuleList matching_module_list;
+
+size_t num_matches = 0;
+// First extract all module specs from the symbol file
+lldb_private::ModuleSpecList symfile_module_specs;
+if (ObjectFile::GetModuleSpecifications(module_spec.GetSymbolFileSpec(),
+0, 0, symfile_module_specs)) {
+  // Now extract the module spec that matches the target architecture
+  ModuleSpec target_arch_module_spec;
+  ModuleSpec symfile_module_spec;
+  target_arch_module_spec.GetArchitecture() = target->GetArchitecture();
+  if (symfile_module_specs.FindMatchingModuleSpec(target_arch_module_spec,
+  symfile_module_spec)) {
+// See if it has a UUID?
+if (symfile_module_spec.GetUUID().IsValid()) {
+  // It has a UUID, look for this UUID in the target modules
+  ModuleSpec symfile_uuid_module_spec;
+  symfile_uuid_module_spec.GetUUID() = symfile_module_spec.GetUUID();
+  

Re: [Lldb-commits] [lldb] f8a92af - [LLDB] Remove incorrect dotest.py invocation

[Adrian McCarthy via lldb-commits]

It looks like this was the only call to getRerunArgs, so why not delete it?

On Mon, Oct 28, 2019 at 1:24 PM Jonas Devlieghere via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

>
> Author: Jonas Devlieghere
> Date: 2019-10-28T13:24:07-07:00
> New Revision: f8a92af893eee7ac7ffda93c24b9e69df506148f
>
> URL:
> https://github.com/llvm/llvm-project/commit/f8a92af893eee7ac7ffda93c24b9e69df506148f
> DIFF:
> https://github.com/llvm/llvm-project/commit/f8a92af893eee7ac7ffda93c24b9e69df506148f.diff
>
> LOG: [LLDB] Remove incorrect dotest.py invocation
>
> The invocation shown by dotest.py to re-run a single test is misleading:
> it ranges from missing arguments (best case scenario) to being totally
> wrong (worst case scenario).
>
> In the past I've tried to get it right, but given the dotest
> architecture this is harder than it looks. Furthermore, we have pretty
> good documentation on the website [1] for most use cases.
>
> This patch removes the rerun invocation.
>
> [1] https://lldb.llvm.org/resources/test.html
>
> Added:
>
>
> Modified:
> lldb/packages/Python/lldbsuite/test/lldbtest.py
>
> Removed:
>
>
>
>
> 
> diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py
> b/lldb/packages/Python/lldbsuite/test/lldbtest.py
> index 34e6aa8f460d..f3165ab32585 100644
> --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
> +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
> @@ -1169,27 +1169,11 @@ def dumpSessionInfo(self):
>  if test is self:
>  print(traceback, file=self.session)
>
> -# put footer (timestamp/rerun instructions) into session
> -testMethod = getattr(self, self._testMethodName)
> -if getattr(testMethod, "__benchmarks_test__", False):
> -benchmarks = True
> -else:
> -benchmarks = False
> -
>  import datetime
>  print(
>  "Session info generated @",
>  datetime.datetime.now().ctime(),
>  file=self.session)
> -print(
> -"To rerun this test, issue the following command from the
> 'test' directory:\n",
> -file=self.session)
> -print(
> -"./dotest.py %s -v %s %s" %
> -(self.getRunOptions(),
> - ('+b' if benchmarks else '-t'),
> -self.getRerunArgs()),
> -file=self.session)
>  self.session.close()
>  del self.session
>
>
>
>
> ___
> 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] [lldb] 5a3c657 - Fix after 738af7a6241c98164625b9cd1ba9f8af4e36f197

[Adrian McCarthy via lldb-commits]

Author: Adrian McCarthy
Date: 2019-10-25T15:57:52-07:00
New Revision: 5a3c657f3e8fe83ed5074edee94fb2303cd5fa2e

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

LOG: Fix after 738af7a6241c98164625b9cd1ba9f8af4e36f197

Default implementation of a new virtual method wasn't returning a value.

Added: 


Modified: 
lldb/include/lldb/Interpreter/ScriptInterpreter.h

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h 
b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index 2213274f1dbf..69af88091a40 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -323,7 +323,11 @@ class ScriptInterpreter : public PluginInterface {
   SetBreakpointCommandCallbackFunction(
   BreakpointOptions *bp_options,
   const char *function_name,
-  StructuredData::ObjectSP extra_args_sp) {}
+  StructuredData::ObjectSP extra_args_sp) {
+Status error;
+error.SetErrorString("unimplemented");
+return error;
+  }
 
   /// Set a one-liner as the callback for the watchpoint.
   virtual void SetWatchpointCommandCallback(WatchpointOptions *wp_options,



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

[Lldb-commits] [lldb] r371882 - Fix error in ProcessLauncherWindows.cpp

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Fri Sep 13 11:50:39 2019
New Revision: 371882

URL: http://llvm.org/viewvc/llvm-project?rev=371882=rev
Log:
Fix error in ProcessLauncherWindows.cpp

Restored missing parens on a function call.

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

Modified: lldb/trunk/source/Host/windows/ProcessLauncherWindows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/ProcessLauncherWindows.cpp?rev=371882=371881=371882=diff
==
--- lldb/trunk/source/Host/windows/ProcessLauncherWindows.cpp (original)
+++ lldb/trunk/source/Host/windows/ProcessLauncherWindows.cpp Fri Sep 13 
11:50:39 2019
@@ -46,7 +46,7 @@ bool GetFlattenedWindowsCommandString(Ar
 
   std::vector args_ref;
   for (auto  : args.entries())
-args_ref.push_back(entry.ref);
+args_ref.push_back(entry.ref());
 
   command = llvm::sys::flattenWindowsCommandLine(args_ref);
   return true;


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

[Lldb-commits] [lldb] r371090 - Fix windows-x86-debug compilation with python enabled using multi-target generator

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Thu Sep  5 10:22:30 2019
New Revision: 371090

URL: http://llvm.org/viewvc/llvm-project?rev=371090=rev
Log:
Fix windows-x86-debug compilation with python enabled using multi-target 
generator

[Patch by Leonid Mashinskiy]

Visual Studio CMake generator is multi-target and does not define
CMAKE_BUILD_TYPE, so Debug build on VS was failing due selection of release
python library. This patch reverts back some of latest changes and fixes
building by raw VS using CMake expression generators.

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

Modified:
lldb/trunk/cmake/modules/LLDBConfig.cmake

Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=371090=371089=371090=diff
==
--- lldb/trunk/cmake/modules/LLDBConfig.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake Thu Sep  5 10:22:30 2019
@@ -134,6 +134,48 @@ endif()
 #locate 64-bit Python libraries.
 # This function is designed to address those limitations.  Currently it only 
partially
 # addresses them, but it can be improved and extended on an as-needed basis.
+function(find_python_libs_windows_helper LOOKUP_DEBUG OUT_EXE_PATH_VARNAME 
OUT_LIB_PATH_VARNAME OUT_DLL_PATH_VARNAME OUT_VERSION_VARNAME)
+  if(LOOKUP_DEBUG)
+  set(POSTFIX "_d")
+  else()
+  set(POSTFIX "")
+  endif()
+
+  file(TO_CMAKE_PATH "${PYTHON_HOME}/python${POSTFIX}.exe" 
  PYTHON_EXE)
+  file(TO_CMAKE_PATH 
"${PYTHON_HOME}/libs/${PYTHONLIBS_BASE_NAME}${POSTFIX}.lib" PYTHON_LIB)
+  file(TO_CMAKE_PATH "${PYTHON_HOME}/${PYTHONLIBS_BASE_NAME}${POSTFIX}.dll"
  PYTHON_DLL)
+
+  foreach(component PYTHON_EXE;PYTHON_LIB;PYTHON_DLL)
+if(NOT EXISTS ${${component}})
+  message(WARNING "Unable to find ${component}")
+  unset(${component})
+endif()
+  endforeach()
+
+  if (NOT PYTHON_EXE OR NOT PYTHON_LIB OR NOT PYTHON_DLL)
+message(WARNING "Unable to find all Python components.  Python support 
will be disabled for this build.")
+set(LLDB_DISABLE_PYTHON 1 PARENT_SCOPE)
+return()
+  endif()
+
+  # Find the version of the Python interpreter.
+  execute_process(COMMAND "${PYTHON_EXE}" -c
+  "import sys; sys.stdout.write('.'.join([str(x) for x in 
sys.version_info[:3]]))"
+  OUTPUT_VARIABLE PYTHON_VERSION_OUTPUT
+  RESULT_VARIABLE PYTHON_VERSION_RESULT
+  ERROR_QUIET)
+
+  if(PYTHON_VERSION_RESULT)
+message(WARNING "Unable to retrieve Python executable version")
+set(PYTHON_VERSION_OUTPUT "")
+  endif()
+
+  set(${OUT_EXE_PATH_VARNAME} ${PYTHON_EXE} PARENT_SCOPE)
+  set(${OUT_LIB_PATH_VARNAME} ${PYTHON_LIB} PARENT_SCOPE)
+  set(${OUT_DLL_PATH_VARNAME} ${PYTHON_DLL} PARENT_SCOPE)
+  set(${OUT_VERSION_VARNAME}  ${PYTHON_VERSION_OUTPUT} PARENT_SCOPE)
+endfunction()
+
 function(find_python_libs_windows)
   if ("${PYTHON_HOME}" STREQUAL "")
 message(WARNING "LLDB embedded Python on Windows requires specifying a 
value for PYTHON_HOME.  Python support disabled.")
@@ -161,55 +203,65 @@ function(find_python_libs_windows)
   file(TO_CMAKE_PATH "${PYTHON_HOME}" PYTHON_HOME)
   # TODO(compnerd) when CMake Policy `CMP0091` is set to NEW, we should use
   # if(CMAKE_MSVC_RUNTIME_LIBRARY MATCHES MultiThreadedDebug)
-  if(CMAKE_BUILD_TYPE STREQUAL Debug)
-file(TO_CMAKE_PATH "${PYTHON_HOME}/python_d.exe" PYTHON_EXE)
-file(TO_CMAKE_PATH "${PYTHON_HOME}/libs/${PYTHONLIBS_BASE_NAME}_d.lib" 
PYTHON_LIB)
-file(TO_CMAKE_PATH "${PYTHON_HOME}/${PYTHONLIBS_BASE_NAME}_d.dll" 
PYTHON_DLL)
-  else()
-file(TO_CMAKE_PATH "${PYTHON_HOME}/python.exe" PYTHON_EXE)
-file(TO_CMAKE_PATH "${PYTHON_HOME}/libs/${PYTHONLIBS_BASE_NAME}.lib" 
PYTHON_LIB)
-file(TO_CMAKE_PATH "${PYTHON_HOME}/${PYTHONLIBS_BASE_NAME}.dll" PYTHON_DLL)
-  endif()
-
-  foreach(component PYTHON_EXE;PYTHON_LIB;PYTHON_DLL)
-if(NOT EXISTS ${${component}})
-  message(WARNING "unable to find ${component}")
-  unset(${component})
+  if(NOT DEFINED CMAKE_BUILD_TYPE)
+# Multi-target generator was selected (like Visual Studio or Xcode) where 
no concrete build type was passed
+# Lookup for both debug and release python installations
+find_python_libs_windows_helper(TRUE  PYTHON_DEBUG_EXE   PYTHON_DEBUG_LIB  
 PYTHON_DEBUG_DLL   PYTHON_DEBUG_VERSION_STRING)
+find_python_libs_windows_helper(FALSE PYTHON_RELEASE_EXE 
PYTHON_RELEASE_LIB PYTHON_RELEASE_DLL PYTHON_RELEASE_VERSION_STRING)
+if(LLDB_DISABLE_PYTHON)
+  set(LLDB_DISABLE_PYTHON 1 PARENT_SCOPE)
+  return()
 endif()
-  endforeach()
 
-  if (NOT PYTHON_EXE OR NOT PYTHON_LIB OR NOT PYTHON_DLL)
-message(WARNING "Unable to find all Python components.  Python support 
will be disabled for this build.")
-set(LLDB_DISABLE_PYTHON 1 PARENT_SCOPE)
-return()
+# We should have been found both debug and release python 

Re: [Lldb-commits] [lldb] r370776 - [lldb][NFC] Disable added frame select and all log option test on windows

[Adrian McCarthy via lldb-commits]

Does disabling tests really quality a patch for the [NFC] label?

Are these buggy tests or is there something that makes the actually not
relevant on Windows?

On Tue, Sep 3, 2019 at 9:20 AM Raphael Isemann via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: teemperor
> Date: Tue Sep  3 09:21:57 2019
> New Revision: 370776
>
> URL: http://llvm.org/viewvc/llvm-project?rev=370776=rev
> Log:
> [lldb][NFC] Disable added frame select and all log option test on windows
>
> Modified:
>
> lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
>
> lldb/trunk/packages/Python/lldbsuite/test/commands/log/basic/TestLogging.py
>
> Modified:
> lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py?rev=370776=370775=370776=diff
>
> ==
> ---
> lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
> (original)
> +++
> lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
> Tue Sep  3 09:21:57 2019
> @@ -11,6 +11,7 @@ class TestFrameSelect(TestBase):
>  mydir = TestBase.compute_mydir(__file__)
>
>  @no_debug_info_test
> +@skipIfWindows
>  def test_relative(self):
>  self.build()
>
>
> Modified:
> lldb/trunk/packages/Python/lldbsuite/test/commands/log/basic/TestLogging.py
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/log/basic/TestLogging.py?rev=370776=370775=370776=diff
>
> ==
> ---
> lldb/trunk/packages/Python/lldbsuite/test/commands/log/basic/TestLogging.py
> (original)
> +++
> lldb/trunk/packages/Python/lldbsuite/test/commands/log/basic/TestLogging.py
> Tue Sep  3 09:21:57 2019
> @@ -91,6 +91,7 @@ class LogTestCase(TestBase):
>  self.assertEquals(contents.find("bacon"), 0)
>
>  # Enable all log options and check that nothing crashes.
> +@skipIfWindows
>  def test_all_log_options(self):
>  if (os.path.exists(self.log_file)):
>  os.remove(self.log_file)
>
>
> ___
> 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] [lldb] r367573 - Fix TestThreadSpecificBreakpoint on Windows

[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] r366703 - [Windows] Fix race condition between state changes

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Mon Jul 22 10:03:20 2019
New Revision: 366703

URL: http://llvm.org/viewvc/llvm-project?rev=366703=rev
Log:
[Windows] Fix race condition between state changes

Patch by Martin Andersson (martin.anders...@evoma.se)

If the process is resumed before the state is changed to "running"
there is a possibility (when single stepping) that the debugger stops
and changes the state to "stopped" before it is first changed to
"running". This causes the process to ignore the stop event (since
the state did not change) which in turn leads the DebuggerThread to
wait indefinitely for the exception predicate in HandleExceptionEvent.

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

Modified:
lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp

Modified: lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp?rev=366703=366702=366703=diff
==
--- lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp Mon Jul 
22 10:03:20 2019
@@ -205,16 +205,6 @@ Status ProcessWindows::DoResume() {
  m_session_data->m_debugger->GetProcess().GetProcessId(),
  GetPrivateState());
 
-ExceptionRecordSP active_exception =
-m_session_data->m_debugger->GetActiveException().lock();
-if (active_exception) {
-  // Resume the process and continue processing debug events.  Mask the
-  // exception so that from the process's view, there is no indication that
-  // anything happened.
-  m_session_data->m_debugger->ContinueAsyncException(
-  ExceptionResult::MaskException);
-}
-
 LLDB_LOG(log, "resuming {0} threads.", m_thread_list.GetSize());
 
 bool failed = false;
@@ -233,10 +223,19 @@ Status ProcessWindows::DoResume() {
 
 if (failed) {
   error.SetErrorString("ProcessWindows::DoResume failed");
-  return error;
 } else {
   SetPrivateState(eStateRunning);
 }
+
+ExceptionRecordSP active_exception =
+m_session_data->m_debugger->GetActiveException().lock();
+if (active_exception) {
+  // Resume the process and continue processing debug events.  Mask the
+  // exception so that from the process's view, there is no indication that
+  // anything happened.
+  m_session_data->m_debugger->ContinueAsyncException(
+  ExceptionResult::MaskException);
+}
   } else {
 LLDB_LOG(log, "error: process {0} is in state {1}.  Returning...",
  m_session_data->m_debugger->GetProcess().GetProcessId(),


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

[Lldb-commits] [lldb] r366383 - [NFC] Clarify a Cmake status message regarding Python on LLDBConfig

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Wed Jul 17 15:36:26 2019
New Revision: 366383

URL: http://llvm.org/viewvc/llvm-project?rev=366383=rev
Log:
[NFC] Clarify a Cmake status message regarding Python on LLDBConfig

Modified:
lldb/trunk/cmake/modules/LLDBConfig.cmake

Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=366383=366382=366383=diff
==
--- lldb/trunk/cmake/modules/LLDBConfig.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake Wed Jul 17 15:36:26 2019
@@ -137,7 +137,7 @@ function(find_python_libs_windows)
  REGEX "^#define[ \t]+PY_VERSION[ \t]+\"[^\"]+\"")
 string(REGEX REPLACE "^#define[ \t]+PY_VERSION[ \t]+\"([^\"+]+)[+]?\".*" 
"\\1"
  PYTHONLIBS_VERSION_STRING "${python_version_str}")
-message(STATUS "Found Python version ${PYTHONLIBS_VERSION_STRING}")
+message(STATUS "Found Python library version ${PYTHONLIBS_VERSION_STRING}")
 string(REGEX REPLACE "([0-9]+)[.]([0-9]+)[.][0-9]+" "python\\1\\2" 
PYTHONLIBS_BASE_NAME "${PYTHONLIBS_VERSION_STRING}")
 set(PYTHONLIBS_VERSION_STRING "${PYTHONLIBS_VERSION_STRING}" PARENT_SCOPE)
 unset(python_version_str)


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

Re: [Lldb-commits] [lldb] r366356 - [dotest] Disable color while testing.

[Adrian McCarthy via lldb-commits]

I thought the color code was smart enough to figure out whether it was
actually going to an actual terminal or being redirected.  Is this hiding a
bug in that logic?

On Wed, Jul 17, 2019 at 10:56 AM Jonas Devlieghere via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: jdevlieghere
> Date: Wed Jul 17 10:56:57 2019
> New Revision: 366356
>
> URL: http://llvm.org/viewvc/llvm-project?rev=366356=rev
> Log:
> [dotest] Disable color while testing.
>
> Disable colors so we don't risk having unexpected ANSI codes in the test
> output. Currently, the behavior of a test can change depending on
> whether it's run under a color-supporting terminal, or under a dummy
> terminal, for example when using lit or multiprocessing.
>
> Modified:
> lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
>
> Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py?rev=366356=366355=366356=diff
>
> ==
> --- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py (original)
> +++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py Wed Jul 17
> 10:56:57 2019
> @@ -1869,6 +1869,9 @@ class TestBase(Base):
>  # differ in the debug info, which is not being hashed.
>  self.runCmd('settings set symbols.enable-external-lookup false')
>
> +# Disable color.
> +self.runCmd("settings set use-color false")
> +
>  # Make sure that a sanitizer LLDB's environment doesn't get
> passed on.
>  if 'DYLD_LIBRARY_PATH' in os.environ:
>  self.runCmd('settings set target.env-vars DYLD_LIBRARY_PATH=')
>
>
> ___
> 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] [lldb] r364361 - Fix a typo in help text.

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Tue Jun 25 16:13:16 2019
New Revision: 364361

URL: http://llvm.org/viewvc/llvm-project?rev=364361=rev
Log:
Fix a typo in help text.

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

Modified: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandInterpreter.cpp?rev=364361=364360=364361=diff
==
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp (original)
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp Tue Jun 25 16:13:16 
2019
@@ -821,7 +821,7 @@ void CommandInterpreter::LoadCommandDict
   *this, "_regexp-env",
   "Shorthand for viewing and setting environment variables.",
   "\n"
-  "_regexp-env  // Show enrivonment\n"
+  "_regexp-env  // Show environment\n"
   "_regexp-env =   // Set an environment variable",
   2, 0, false));
   if (env_regex_cmd_up) {


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

[Lldb-commits] [lldb] r362845 - NFC: Fix typo in a cmake message

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Fri Jun  7 14:14:01 2019
New Revision: 362845

URL: http://llvm.org/viewvc/llvm-project?rev=362845=rev
Log:
NFC:  Fix typo in a cmake message

Modified:
lldb/trunk/cmake/modules/LLDBConfig.cmake

Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=362845=362844=362845=diff
==
--- lldb/trunk/cmake/modules/LLDBConfig.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake Fri Jun  7 14:14:01 2019
@@ -181,7 +181,7 @@ function(find_python_libs_windows)
   set (PYTHON_DLL ${PYTHON_DLL} PARENT_SCOPE)
   set (PYTHON_INCLUDE_DIR ${PYTHON_INCLUDE_DIR} PARENT_SCOPE)
 
-  message("-- LLDB Found PythonExecutable: ${PYTHON_EXE}}")
+  message("-- LLDB Found PythonExecutable: ${PYTHON_EXE}")
   message("-- LLDB Found PythonLibs: ${PYTHON_LIB}")
   message("-- LLDB Found PythonDLL: ${PYTHON_DLL}")
   message("-- LLDB Found PythonIncludeDirs: ${PYTHON_INCLUDE_DIR}")


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

[Lldb-commits] [lldb] r362844 - Fix lit tests on Windows related to CR+LF

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Fri Jun  7 14:13:30 2019
New Revision: 362844

URL: http://llvm.org/viewvc/llvm-project?rev=362844=rev
Log:
Fix lit tests on Windows related to CR+LF

Problem discovered in the breakpoint lit test, but probably exists in others.
lldb-test splits lines on LF.  Input files that are CR+LF separated (as is
common on Windows) then resulted in commands being sent to LLDB that ended
in CR, which confused the command interpreter.

This could be fixed at different levels:

1.  Treat '\r' like a tab or space in the argument splitter.
2.  Fix the line splitters (plural) in lldb-test.
3.  Normalize the test files to LF only.

If we did only 3, I'd expect similar problems to recur, so this patch does
1 and 2.  I may also do 3 in a separate patch later, but that's tricky
because I believe we have some input files that MUST use CR+LF.

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

Modified:
lldb/trunk/source/Utility/Args.cpp
lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/source/Utility/Args.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Args.cpp?rev=362844=362843=362844=diff
==
--- lldb/trunk/source/Utility/Args.cpp (original)
+++ lldb/trunk/source/Utility/Args.cpp Fri Jun  7 14:13:30 2019
@@ -95,7 +95,7 @@ ParseSingleArgument(llvm::StringRef comm
   bool arg_complete = false;
   do {
 // Skip over over regular characters and append them.
-size_t regular = command.find_first_of(" \t\"'`\\");
+size_t regular = command.find_first_of(" \t\r\"'`\\");
 arg += command.substr(0, regular);
 command = command.substr(regular);
 
@@ -123,6 +123,7 @@ ParseSingleArgument(llvm::StringRef comm
 
 case ' ':
 case '\t':
+case '\r':
   // We are not inside any quotes, we just found a space after an argument.
   // We are done.
   arg_complete = true;

Modified: lldb/trunk/tools/lldb-test/lldb-test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-test/lldb-test.cpp?rev=362844=362843=362844=diff
==
--- lldb/trunk/tools/lldb-test/lldb-test.cpp (original)
+++ lldb/trunk/tools/lldb-test/lldb-test.cpp Fri Jun  7 14:13:30 2019
@@ -312,7 +312,7 @@ int opts::breakpoint::evaluateBreakpoint
   while (!Rest.empty()) {
 StringRef Line;
 std::tie(Line, Rest) = Rest.split('\n');
-Line = Line.ltrim();
+Line = Line.ltrim().rtrim();
 if (Line.empty() || Line[0] == '#')
   continue;
 
@@ -939,7 +939,7 @@ int opts::irmemorymap::evaluateMemoryMap
   while (!Rest.empty()) {
 StringRef Line;
 std::tie(Line, Rest) = Rest.split('\n');
-Line = Line.ltrim();
+Line = Line.ltrim().rtrim();
 
 if (Line.empty() || Line[0] == '#')
   continue;


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

[Lldb-commits] [lldb] r357626 - Fix and simplify PrepareCommandsForSourcing

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Wed Apr  3 12:49:14 2019
New Revision: 357626

URL: http://llvm.org/viewvc/llvm-project?rev=357626=rev
Log:
Fix and simplify PrepareCommandsForSourcing

Spotted some problems in the Driver's PrepareCommandsForSourcing while
helping a colleague track another problem.

1. One error case was not handled because there was no else clause.
Fixed by switching to llvm's early-out style instead of nested
`if (succes) { } else { }` cases.  This keeps error handling close
to the actual error.

2. One call-site failed to call the clean-up function.  I solved this
by simplifying the API.  PrepareCommandsForSourcing no longer requires
the caller to provide a buffer for the pipe's file descriptors and to
call a separate clean-up function later.  PrepareCommandsForSourcing
now ensures the file descriptors are handled before returning.
(The read end of the pipe is held open by the returned FILE * as
before.)

I also eliminated an unnecessary local, shorted the lifetime of another,
and tried to improve the comments.

I wrapped the call to open the pipe to get the `#ifdef`s out of the
mainline.  I replaced the `close`/`_close` calls with a platform-neutral
helper from `llvm::sys` for the same reason.  Per discussion on the
review, I'm leaving the `fdopen` call to use the spelling that Windows
has officially deprecated because it still works it avoids more `#ifdef`s.

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

Modified:
lldb/trunk/tools/driver/Driver.cpp

Modified: lldb/trunk/tools/driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/driver/Driver.cpp?rev=357626=357625=357626=diff
==
--- lldb/trunk/tools/driver/Driver.cpp (original)
+++ lldb/trunk/tools/driver/Driver.cpp Wed Apr  3 12:49:14 2019
@@ -22,6 +22,7 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
@@ -469,85 +470,58 @@ SBError Driver::ProcessArgs(const opt::I
   return error;
 }
 
-static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
-  size_t commands_size, int fds[2]) {
-  enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE
-
-  ::FILE *commands_file = nullptr;
-  fds[0] = -1;
-  fds[1] = -1;
-  int err = 0;
+static inline int OpenPipe(int fds[2], std::size_t size) {
 #ifdef _WIN32
-  err = _pipe(fds, commands_size, O_BINARY);
+  return _pipe(fds, size, O_BINARY);
 #else
-  err = pipe(fds);
+  (void) size;
+  return pipe(fds);
 #endif
-  if (err == 0) {
-ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
-if (nrwr < 0) {
-  WithColor::error()
-  << format(
- "write(%i, %p, %" PRIu64
- ") failed (errno = %i) when trying to open LLDB commands 
pipe",
- fds[WRITE], static_cast(commands_data),
- static_cast(commands_size), errno)
-  << '\n';
-} else if (static_cast(nrwr) == commands_size) {
-// Close the write end of the pipe so when we give the read end to
-// the debugger/command interpreter it will exit when it consumes all
-// of the data
-#ifdef _WIN32
-  _close(fds[WRITE]);
-  fds[WRITE] = -1;
-#else
-  close(fds[WRITE]);
-  fds[WRITE] = -1;
-#endif
-  // Now open the read file descriptor in a FILE * that we can give to
-  // the debugger as an input handle
-  commands_file = fdopen(fds[READ], "r");
-  if (commands_file != nullptr) {
-fds[READ] = -1; // The FILE * 'commands_file' now owns the read
-// descriptor Hand ownership if the FILE * over to the
-// debugger for "commands_file".
-  } else {
-WithColor::error() << format("fdopen(%i, \"r\") failed (errno = %i) "
- "when trying to open LLDB commands pipe",
- fds[READ], errno)
-   << '\n';
-  }
-}
-  } else {
+}
+
+static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
+  size_t commands_size) {
+  enum PIPES { READ, WRITE }; // Indexes for the read and write fds
+  int fds[2] = {-1, -1};
+
+  if (OpenPipe(fds, commands_size) != 0) {
 WithColor::error()
 << "can't create pipe file descriptors for LLDB commands\n";
+return nullptr;
   }
 
-  return commands_file;
-}
-
-void CleanupAfterCommandSourcing(int fds[2]) {
-  enum PIPES { READ, WRITE }; // Constants 0 and 1 for READ and WRITE
-
-  // Close any pipes that we still have ownership of
-  if (fds[WRITE] != -1) {
-#ifdef _WIN32
-_close(fds[WRITE]);
-fds[WRITE] = -1;
-#else
-close(fds[WRITE]);
-fds[WRITE] = -1;
-#endif
+  ssize_t nrwr = 

[Lldb-commits] [lldb] r355943 - Correcting some comments in PdbIndex.cpp [NFC]

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Tue Mar 12 10:40:51 2019
New Revision: 355943

URL: http://llvm.org/viewvc/llvm-project?rev=355943=rev
Log:
Correcting some comments in PdbIndex.cpp [NFC]

ICF can cause multiple symbols to start at the same virtual address.
I plan to handle this shortly, but I wanted to correct the comment for
now.

Deleted an obsolete comment about adjusting the offset for the magic
number at the beginning of the debug info stream.  This adjustment is
handled at a lower level now.

Modified:
lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbIndex.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbIndex.cpp?rev=355943=355942=355943=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbIndex.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbIndex.cpp Tue Mar 12 
10:40:51 2019
@@ -132,9 +132,8 @@ void PdbIndex::BuildAddrToSymbolMap(Comp
 
 PdbCompilandSymId cu_sym_id(modi, iter.offset());
 
-// If the debug info is incorrect, we could have multiple symbols with the
-// same address.  So use try_emplace instead of insert, and the first one
-// will win.
+// It's rare, but we could have multiple symbols with the same address
+// because of identical comdat folding.  Right now, the first one will win.
 cci.m_symbols_by_va.insert(std::make_pair(va, PdbSymUid(cu_sym_id)));
   }
 }
@@ -186,8 +185,6 @@ std::vector PdbIndex::Find
 }
 
 CVSymbol PdbIndex::ReadSymbolRecord(PdbCompilandSymId cu_sym) const {
-  // We need to subtract 4 here to adjust for the codeview debug magic
-  // at the beginning of the debug info stream.
   const CompilandIndexItem *cci = compilands().GetCompiland(cu_sym.modi);
   auto iter = cci->m_debug_stream.getSymbolArray().at(cu_sym.offset);
   lldbassert(iter != cci->m_debug_stream.getSymbolArray().end());


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

[Lldb-commits] [lldb] r355121 - Improve process launch comments for Windows

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Thu Feb 28 11:14:02 2019
New Revision: 355121

URL: http://llvm.org/viewvc/llvm-project?rev=355121=rev
Log:
Improve process launch comments for Windows

The existing comment about over-allocating the command line was incorrect.  The
contents of the command line may be changed, but it's not necessary to over
allocate.  The changes will be limited to the existing contents of the string
(e.g., by replacing spaces with L'\0' to tokenize the command line).

Also added a comment explaining a possible cause of failure to save the next
programmer some time when they try to debug a 64-bit process from a 32-bit
LLDB.

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

Modified: lldb/trunk/source/Host/windows/ProcessLauncherWindows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/ProcessLauncherWindows.cpp?rev=355121=355120=355121=diff
==
--- lldb/trunk/source/Host/windows/ProcessLauncherWindows.cpp (original)
+++ lldb/trunk/source/Host/windows/ProcessLauncherWindows.cpp Thu Feb 28 
11:14:02 2019
@@ -104,17 +104,21 @@ ProcessLauncherWindows::LaunchProcess(co
   llvm::ConvertUTF8toWide(commandLine, wcommandLine);
   llvm::ConvertUTF8toWide(launch_info.GetWorkingDirectory().GetCString(),
   wworkingDirectory);
+  // If the command line is empty, it's best to pass a null pointer to tell
+  // CreateProcessW to use the executable name as the command line.  If the
+  // command line is not empty, its contents may be modified by CreateProcessW.
+  WCHAR *pwcommandLine = wcommandLine.empty() ? nullptr : [0];
 
-  wcommandLine.resize(PATH_MAX); // Needs to be over-allocated because
- // CreateProcessW can modify it
   BOOL result = ::CreateProcessW(
-  wexecutable.c_str(), [0], NULL, NULL, TRUE, flags, 
env_block,
+  wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block,
   wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(),
   , );
 
   if (!result) {
 // Call GetLastError before we make any other system calls.
 error.SetError(::GetLastError(), eErrorTypeWin32);
+// Note that error 50 ("The request is not supported") will occur if you
+// try debug a 64-bit inferior from a 32-bit LLDB.
   }
 
   if (result) {


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

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

[Adrian McCarthy via lldb-commits]

>  But here, we're talking about a situation where there is no EXE, only a
minidump.  If there is a minidump and no EXE then neither WinDbg nor VS
will search the minidump folder for the PDB.

For the record, the experiments do not bear this out.  VS will indeed
search in the minidump folder for the PDB.  Unfortunately, a lot of this
conversation was taken offline.

On Tue, Dec 11, 2018 at 3:30 PM Zachary Turner via Phabricator <
revi...@reviews.llvm.org> wrote:

> zturner added a comment.
>
> In D55142#1326247 , @lemo wrote:
>
> > > How large is the PDB file here?
> >
> > ~100kb
>
>
> We have a couple of tests in LLVM where PDB files are checked in, but they
> are very few.  We cannot explode the repo with large numbers of binary
> files.  So this is probably fine, but if this becomes a pattern, we will
> need to come up with a different solution.
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55142/new/
>
> https://reviews.llvm.org/D55142
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

[Adrian McCarthy via lldb-commits]

I believe the PDB is searched for in the EXE directory before the symbol
search path is used.  At least, that's what it used to do, back when I used
VS debugger for post-mortem debugging.  It was the only sane way to ensure
it would find the right version of the PDB if you didn't have a local
symbol server.

Yes, I understand that the security note was about DLL loading.  My point
was that, in general, Windows looks in well known places first, before
checking the current working directory.

On Tue, Dec 11, 2018 at 2:12 PM Leonard Mosescu  wrote:

> The Windowsy thing to do is what Zach said:  Check the directory that
>> contains the .dmp for the .pdb.  It's the first place the VS debugger looks
>> when opening a minidump.  It's less sensitive to the user's environment.
>> (Making the user change the current working directory for this could be at
>> odds with other bits of the software that look relative the cwd.)
>>
>> https://docs.microsoft.com/en-us/visualstudio/debugger/using-dump-files?view=vs-2017#BKMK_Find_binaries__symbol___pdb__files__and_source_files
>>
>
> Except that it doesn't :) Neither VisualStudio nor the Windows Debuggers
> (windbg & co) look for PDBs in the same directory as the dump file. The
> search is controlled by an explicit "symbol search path". The link you
> mentioned is a bit confusing indeed (although it only claims that the
> .exe's are searched in the same directory as the .dmp)
>
> While security is not a big issue here, note that Windows generally
>> searches for DLLs in the known/expected places _before_ checking to see if
>> it's in the current working directory.  This prevents a sneaky download
>> from effectively replacing a DLL.  Replacing a PDB is probably less of a
>> concern.
>>
>> https://docs.microsoft.com/en-us/windows/desktop/dlls/dynamic-link-library-search-order#standard-search-order-for-desktop-applications
>>
>
> This is about the runtime dynamic module loader, not the debugger.
>
>
>
>
> On Tue, Dec 11, 2018 at 1:28 PM Adrian McCarthy 
> wrote:
>
>> It's really frustrating how the email discussion doesn't always make it
>> to Phabricator.
>>
>> The Windowsy thing to do is what Zach said:  Check the directory that
>> contains the .dmp for the .pdb.  It's the first place the VS debugger looks
>> when opening a minidump.  It's less sensitive to the user's environment.
>> (Making the user change the current working directory for this could be at
>> odds with other bits of the software that look relative the cwd.)
>>
>>
>> https://docs.microsoft.com/en-us/visualstudio/debugger/using-dump-files?view=vs-2017#BKMK_Find_binaries__symbol___pdb__files__and_source_files
>>
>> While security is not a big issue here, note that Windows generally
>> searches for DLLs in the known/expected places _before_ checking to see if
>> it's in the current working directory.  This prevents a sneaky download
>> from effectively replacing a DLL.  Replacing a PDB is probably less of a
>> concern.
>>
>>
>> https://docs.microsoft.com/en-us/windows/desktop/dlls/dynamic-link-library-search-order#standard-search-order-for-desktop-applications
>>
>> So I +1 Zach's proposal.
>>
>> On Tue, Dec 11, 2018 at 12:07 PM Leonard Mosescu 
>> wrote:
>>
>>> I think as combination of explicit symbol search path + something
>>> similar to Microsoft's symsrv would be a good "real" solution (and yes,
>>> that would be packaged as a SymbolVendor, outside SymbolFilePDB)
>>>
>>> For short term, I don't see a clearly superior alternative to searching
>>> the current directory.
>>>
>>> On Tue, Dec 11, 2018 at 12:02 PM Pavel Labath  wrote:
>>>
 On 11/12/2018 20:34, Zachary Turner wrote:
 > I meant the location of the minidump.  So if you have
 C:\A\B\C\foo.dmp
 > which is the dump file for bar.exe which crashed on another machine,
 > then it would look for C:\A\B\C\bar.pdb.  That actually seems like
 > fairly intuitive behavior to me, but maybe I'm in the minority :)
 >
 > We can see what Pavel, Adrian, and others think though or if they
 have
 > any other suggestions.
 >

 It sounds like there is a precedent for searching in CWD. I don't know
 how useful it is (I traced it back to r185366, but it is not mentioned
 there specifically), but it is there, and I guess it's not completely
 nonsensical from the POV of a command line user.

 I guess we can just keep that there and not call it a hack (though, the
 fact that the searching happens inside SymbolFilePDB *is* a hack).

 Searching in the minidump directory would also make sense somewhat, but
 I expect you would need more plumbing for that to happen (and I don't
 know of a precedent for that).

 pl

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

Re: [Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

[Adrian McCarthy via lldb-commits]

It's really frustrating how the email discussion doesn't always make it to
Phabricator.

The Windowsy thing to do is what Zach said:  Check the directory that
contains the .dmp for the .pdb.  It's the first place the VS debugger looks
when opening a minidump.  It's less sensitive to the user's environment.
(Making the user change the current working directory for this could be at
odds with other bits of the software that look relative the cwd.)

https://docs.microsoft.com/en-us/visualstudio/debugger/using-dump-files?view=vs-2017#BKMK_Find_binaries__symbol___pdb__files__and_source_files

While security is not a big issue here, note that Windows generally
searches for DLLs in the known/expected places _before_ checking to see if
it's in the current working directory.  This prevents a sneaky download
from effectively replacing a DLL.  Replacing a PDB is probably less of a
concern.

https://docs.microsoft.com/en-us/windows/desktop/dlls/dynamic-link-library-search-order#standard-search-order-for-desktop-applications

So I +1 Zach's proposal.

On Tue, Dec 11, 2018 at 12:07 PM Leonard Mosescu  wrote:

> I think as combination of explicit symbol search path + something similar
> to Microsoft's symsrv would be a good "real" solution (and yes, that would
> be packaged as a SymbolVendor, outside SymbolFilePDB)
>
> For short term, I don't see a clearly superior alternative to searching
> the current directory.
>
> On Tue, Dec 11, 2018 at 12:02 PM Pavel Labath  wrote:
>
>> On 11/12/2018 20:34, Zachary Turner wrote:
>> > I meant the location of the minidump.  So if you have C:\A\B\C\foo.dmp
>> > which is the dump file for bar.exe which crashed on another machine,
>> > then it would look for C:\A\B\C\bar.pdb.  That actually seems like
>> > fairly intuitive behavior to me, but maybe I'm in the minority :)
>> >
>> > We can see what Pavel, Adrian, and others think though or if they have
>> > any other suggestions.
>> >
>>
>> It sounds like there is a precedent for searching in CWD. I don't know
>> how useful it is (I traced it back to r185366, but it is not mentioned
>> there specifically), but it is there, and I guess it's not completely
>> nonsensical from the POV of a command line user.
>>
>> I guess we can just keep that there and not call it a hack (though, the
>> fact that the searching happens inside SymbolFilePDB *is* a hack).
>>
>> Searching in the minidump directory would also make sense somewhat, but
>> I expect you would need more plumbing for that to happen (and I don't
>> know of a precedent for that).
>>
>> pl
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] Support of MSVC function-level linking

[Adrian McCarthy via lldb-commits]

Can you post your patch to https://reviews.llvm.org/ ?

On Thu, May 31, 2018 at 10:36 AM, Leonard Mosescu via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> If anyone's working on this I'd suggest adding a test case for the "split
> code" case as well (where even a single function is split into multiple
> ranges). MSVC with PGO should help produce hot/cold cold split repros.
>
> On Thu, May 31, 2018 at 10:24 AM, Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
>
>>
>>
>> On May 31, 2018, at 2:31 AM, Aleksandr Urakov via lldb-commits <
>> lldb-commits@lists.llvm.org> wrote:
>>
>> Hello!
>>
>> I'm Aleksandr from JetBrains. We are working on improving support of
>> MSVC-compiled binaries in lldb. We have made several fixes and would like
>> to upstream them.
>>
>> The first patch adds support of function-level linking feature. The
>> SymbolFilePDB::ParseCompileUnitLineTable function relies on the fact
>> that ranges of compiled source files in the binary are continuous and don't
>> intersect with each other. ParseCompileUnitLineTable creates LineSequence
>> for each file and inserts it into LineTable, and the implementation of
>> LineTable relies on continuity of the sequence. But it's not always true
>> when function-level linking is enabled, e.g. in the attached input test
>> file test-pdb-function-level-linking.exe there is xstring's
>> std__basic_string_char_std__char_traits_char__std__allocator_char_max_size
>> (.00454820) between test-pdb-function-level-linking.cpp's foo
>> (.00454770) and main (.004548F0). The source is compiled with Microsoft
>> C/C++ compiler version 19.14.26429.4 for x86.
>>
>> To fix the problem we propose to renew the sequence on each address gap.
>>
>>
>> That is what DWARF does as well. A line table can have many sequences
>> where each sequence is a vector of rows whose addresses must always
>> increase or stay the same. The line tables we have in LLDB mimic the DWARF
>> line tables in many ways.
>>
>>
>>
>> The link to the patch and related files is: https://drive.google.com/o
>> pen?id=1ozp06jyqugjLGT-6wuJKS1UhRuXFsixf
>>
>> Thanks!
>>
>> --
>> Aleksandr Urakov
>> Software Developer
>> JetBrains
>> http://www.jetbrains.com
>> The Drive to Develop
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>>
>>
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r326130 - Partial fix for TestConflictingSymbol.py on Windows

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Mon Feb 26 13:22:39 2018
New Revision: 326130

URL: http://llvm.org/viewvc/llvm-project?rev=326130=rev
Log:
Partial fix for TestConflictingSymbol.py on Windows

Without this fix, the test ERRORs because the link of the inferior fails. This
patch adds the LLDB_TEST_API macro where needed and uses the new -2 magic
value for num_expected_locations to account for lazy-loading of module symbols
on Windows.

With this fix, the test itself still fails:  conflicting_symbol isn't in the
debug info nor the export table, and Windows binaries don't have an equivalent
of the ELF .symtab.  We need to understand why the test works to keep the
symbol out of the debug info.  In the mean time, having the test fail at this
point is a better indication of the remaining problem than a build error.

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h

lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py

lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h?rev=326130=326129=326130=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One/One.h 
Mon Feb 26 13:22:39 2018
@@ -1,4 +1,4 @@
 #ifndef ONE_H
 #define ONE_H
-void one();
+LLDB_TEST_API void one();
 #endif

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py?rev=326130=326129=326130=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/TestConflictingSymbol.py
 Mon Feb 26 13:22:39 2018
@@ -33,9 +33,9 @@ class TestConflictingSymbols(TestBase):
 target, ['One', 'Two'])
 
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
-extra_options='-f One.c')
+extra_options='-f One.c', num_expected_locations=-2)
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
-extra_options='-f Two.c')
+extra_options='-f Two.c', num_expected_locations=-2)
 lldbutil.run_break_set_by_source_regexp(self, '// break here',
 extra_options='-f main.c', num_expected_locations=1)
 

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h?rev=326130=326129=326130=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two/Two.h 
Mon Feb 26 13:22:39 2018
@@ -1,4 +1,4 @@
 #ifndef TWO_H
 #define TWO_H
-void two();
+LLDB_TEST_API void two();
 #endif


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

[Lldb-commits] [lldb] r326095 - Fix tabs/spaces indentation problem in TestUnicodeSymbols.py

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Mon Feb 26 07:53:31 2018
New Revision: 326095

URL: http://llvm.org/viewvc/llvm-project?rev=326095=rev
Log:
Fix tabs/spaces indentation problem in TestUnicodeSymbols.py

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py?rev=326095=326094=326095=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/unicode/TestUnicodeSymbols.py 
Mon Feb 26 07:53:31 2018
@@ -10,10 +10,10 @@ class TestUnicodeSymbols(TestBase):
 
 def test_union_members(self):
 self.build()
-   spec = lldb.SBModuleSpec()
-   spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact("a.out")))
-   module = lldb.SBModule(spec)
-   self.assertTrue(module.IsValid())
-   mytype = module.FindFirstType("foobár")
-   self.assertTrue(mytype.IsValid())
-   self.assertTrue(mytype.IsPointerType())
+spec = lldb.SBModuleSpec()
+spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact("a.out")))
+module = lldb.SBModule(spec)
+self.assertTrue(module.IsValid())
+mytype = module.FindFirstType("foobár")
+self.assertTrue(mytype.IsValid())
+self.assertTrue(mytype.IsPointerType())


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

[Lldb-commits] [lldb] r325836 - Fix TestMoveNearest on Windows

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Thu Feb 22 14:47:47 2018
New Revision: 325836

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

The header file for the DLL tried to declare inline functions and a local
function as dllexport which broke the compile and link.  Removing the bad
declarations solves the problem, and the test passes on Windows now.

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.h

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.h?rev=325836=325835=325836=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.h
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/foo.h
 Thu Feb 22 14:47:47 2018
@@ -1,6 +1,5 @@
-LLDB_TEST_API inline int foo1() { return 1; } // !BR1
+inline int foo1() { return 1; } // !BR1
 
-LLDB_TEST_API inline int foo2() { return 2; } // !BR2
+inline int foo2() { return 2; } // !BR2
 
 LLDB_TEST_API extern int call_foo1();
-LLDB_TEST_API extern int call_foo2();


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

[Lldb-commits] [lldb] r325835 - Fix TestSBData.py on Windows

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Thu Feb 22 14:47:14 2018
New Revision: 325835

URL: http://llvm.org/viewvc/llvm-project?rev=325835=rev
Log:
Fix TestSBData.py on Windows

Ensure that the test data is an array of bytes rather than a string that gets
encoded differently between Python 2 and Python 3.

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

Modified:
lldb/trunk/packages/Python/lldbsuite/test/python_api/sbdata/TestSBData.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/sbdata/TestSBData.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/sbdata/TestSBData.py?rev=325835=325834=325835=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/python_api/sbdata/TestSBData.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/python_api/sbdata/TestSBData.py 
Thu Feb 22 14:47:14 2018
@@ -25,7 +25,7 @@ class SBDataAPICase(TestBase):
 def test_byte_order_and_address_byte_size(self):
 """Test the SBData::SetData() to ensure the byte order and address 
 byte size are obeyed"""
-addr_data = '\x11\x22\x33\x44\x55\x66\x77\x88'
+addr_data = b'\x11\x22\x33\x44\x55\x66\x77\x88'
 error = lldb.SBError()
 data = lldb.SBData()
 data.SetData(error, addr_data, lldb.eByteOrderBig, 4)


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

[Lldb-commits] [lldb] r325704 - Fix TestBreakpointInGlobalConstructor for Windows

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Wed Feb 21 10:08:23 2018
New Revision: 325704

URL: http://llvm.org/viewvc/llvm-project?rev=325704=rev
Log:
Fix TestBreakpointInGlobalConstructor for Windows

Summary:
This test was failing on Windows because it expected the breakpoint in the
dynamic library to be resolved before the process is launched.  Since the DLL
isn't loaded until the process is launched this didn't work.

The fix creates a special value (-2) for num_expected_locations that ignores
the actual number of breakpoint locations found.

Reviewers: jasonmolenda

Subscribers: sanjoy, lldb-commits

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py?rev=325704=325703=325704=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
 Wed Feb 21 10:08:23 2018
@@ -29,8 +29,9 @@ class TestBreakpointInGlobalConstructors
 
 bp_main = lldbutil.run_break_set_by_file_and_line(
 self, 'main.cpp', self.line_main)
+
 bp_foo = lldbutil.run_break_set_by_file_and_line(
-self, 'foo.cpp', self.line_foo)
+self, 'foo.cpp', self.line_foo, num_expected_locations=-2)
 
 process = target.LaunchSimple(
 None, env, self.get_process_working_directory())

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py?rev=325704=325703=325704=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py Wed Feb 21 10:08:23 
2018
@@ -343,7 +343,8 @@ def run_break_set_by_file_and_line(
 
 If extra_options is not None, then we append it to the breakpoint set 
command.
 
-If num_expected_locations is -1 we check that we got AT LEAST one 
location, otherwise we check that num_expected_locations equals the number of 
locations.
+If num_expected_locations is -1, we check that we got AT LEAST one 
location. If num_expected_locations is -2, we don't
+check the actual number at all. Otherwise, we check that 
num_expected_locations equals the number of locations.
 
 If loc_exact is true, we check that there is one location, and that 
location must be at the input file and line number."""
 
@@ -563,7 +564,7 @@ def check_breakpoint_result(
 if num_locations == -1:
 test.assertTrue(out_num_locations > 0,
 "Expecting one or more locations, got none.")
-else:
+elif num_locations != -2:
 test.assertTrue(
 num_locations == out_num_locations,
 "Expecting %d locations, got %d." %


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

[Lldb-commits] [lldb] r325188 - Supply missing break in case statement.

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Wed Feb 14 15:16:36 2018
New Revision: 325188

URL: http://llvm.org/viewvc/llvm-project?rev=325188=rev
Log:
Supply missing break in case statement.

Summary:
All the tests pass without hitting the situation mentioned in the FIXME, so,
per Aaron Smith's suggestion, this case will now return unconditionally.

Subscribers: sanjoy, mgorny, JDevlieghere

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp?rev=325188=325187=325188=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp Wed Feb 14 
15:16:36 2018
@@ -107,9 +107,7 @@ CompilerType GetBuiltinTypeForPDBEncodin
   case PDB_BuiltinType::None:
 return CompilerType();
   case PDB_BuiltinType::Void:
-// FIXME: where is non-zero size of `void` from?
-if (width == 0)
-  return clang_ast->GetBasicType(eBasicTypeVoid);
+return clang_ast->GetBasicType(eBasicTypeVoid);
   case PDB_BuiltinType::Bool:
 return clang_ast->GetBasicType(eBasicTypeBool);
   case PDB_BuiltinType::Long:


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

[Lldb-commits] [lldb] r324925 - Remove dead code for handling DWARF pubnames

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Mon Feb 12 11:19:04 2018
New Revision: 324925

URL: http://llvm.org/viewvc/llvm-project?rev=324925=rev
Log:
Remove dead code for handling DWARF pubnames

Summary:
LLDB doesn't use this code, the code has no tests, and the code does suspicious
things like hashing pointers to strings instead of the strings themselves.

Subscribers: sanjoy, mgorny, JDevlieghere

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

Removed:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugPubnames.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugPubnames.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugPubnamesSet.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugPubnamesSet.h
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt?rev=324925=324924=324925=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt Mon Feb 12 
11:19:04 2018
@@ -17,8 +17,6 @@ add_lldb_library(lldbPluginSymbolFileDWA
   DWARFDebugMacro.cpp
   DWARFDebugMacinfo.cpp
   DWARFDebugMacinfoEntry.cpp
-  DWARFDebugPubnames.cpp
-  DWARFDebugPubnamesSet.cpp
   DWARFDebugRanges.cpp
   DWARFDeclContext.cpp
   DWARFDefines.cpp

Removed: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugPubnames.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugPubnames.cpp?rev=324924=auto
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugPubnames.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugPubnames.cpp (removed)
@@ -1,255 +0,0 @@
-//===-- DWARFDebugPubnames.cpp --*- C++ 
-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "DWARFDebugPubnames.h"
-
-#include "lldb/Utility/Stream.h"
-#include "lldb/Utility/Timer.h"
-
-#include "DWARFCompileUnit.h"
-#include "DWARFDIECollection.h"
-#include "DWARFDebugInfo.h"
-#include "DWARFFormValue.h"
-#include "LogChannelDWARF.h"
-#include "SymbolFileDWARF.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-DWARFDebugPubnames::DWARFDebugPubnames() : m_sets() {}
-
-bool DWARFDebugPubnames::Extract(const DWARFDataExtractor ) {
-  static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
-  Timer scoped_timer(func_cat,
- "DWARFDebugPubnames::Extract (byte_size = %" PRIu64 ")",
- (uint64_t)data.GetByteSize());
-  Log *log(LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_PUBNAMES));
-  if (log)
-log->Printf("DWARFDebugPubnames::Extract (byte_size = %" PRIu64 ")",
-(uint64_t)data.GetByteSize());
-
-  if (data.ValidOffset(0)) {
-lldb::offset_t offset = 0;
-
-DWARFDebugPubnamesSet set;
-while (data.ValidOffset(offset)) {
-  if (set.Extract(data, )) {
-m_sets.push_back(set);
-offset = set.GetOffsetOfNextEntry();
-  } else
-break;
-}
-if (log)
-  Dump(log);
-return true;
-  }
-  return false;
-}
-
-bool DWARFDebugPubnames::GeneratePubnames(SymbolFileDWARF *dwarf2Data) {
-  static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
-  Timer scoped_timer(func_cat,
- "DWARFDebugPubnames::GeneratePubnames (data = %p)",
- static_cast(dwarf2Data));
-
-  Log *log(LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_PUBNAMES));
-  if (log)
-log->Printf("DWARFDebugPubnames::GeneratePubnames (data = %p)",
-static_cast(dwarf2Data));
-
-  m_sets.clear();
-  DWARFDebugInfo *debug_info = dwarf2Data->DebugInfo();
-  if (debug_info) {
-uint32_t cu_idx = 0;
-const uint32_t num_compile_units = dwarf2Data->GetNumCompileUnits();
-for (cu_idx = 0; cu_idx < num_compile_units; ++cu_idx) {
-
-  DWARFCompileUnit *cu = debug_info->GetCompileUnitAtIndex(cu_idx);
-
-  DWARFFormValue::FixedFormSizes fixed_form_sizes =
-  DWARFFormValue::GetFixedFormSizesForAddressSize(
-  cu->GetAddressByteSize(), cu->IsDWARF64());
-
-  bool clear_dies = cu->ExtractDIEsIfNeeded(false) > 1;
-
-  DWARFDIECollection dies;
-  const size_t die_count = cu->AppendDIEsWithTag(DW_TAG_subprogram, dies) +
-   cu->AppendDIEsWithTag(DW_TAG_variable, dies);
-
-  dw_offset_t cu_offset = 

Re: [Lldb-commits] [lldb] r320242 - Change uses of strncpy in debugserver to strlcpy

[Adrian McCarthy via lldb-commits]

I have some concerns about this change.

1.  strlcpy is not a C++ standard function, so it's not available for
non-POSIX targets.  As far as I can tell, this is the first use of strlcpy
in LLVM.

2.  In some of the changed calls, the buffer size argument still has a -1,
which is redundant with what strlcpy is going to do, so this could cause
strings to be truncated a char too soon.

3.  At some of the call sites, there remains now-redundant code to
guarantee termination.. Since that's the point of changing these calls, we
should probably eliminate that.

4.  I'm not familiar with this part of the code base.  Is it necessary for
the APIs to work with pointer+length rather than a std::string (or other
class) that would give us safety and portability?

On Sun, Dec 10, 2017 at 3:52 PM, Davide Italiano via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> On Fri, Dec 8, 2017 at 7:37 PM, Jason Molenda via lldb-commits
>  wrote:
> > Author: jmolenda
> > Date: Fri Dec  8 19:37:09 2017
> > New Revision: 320242
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=320242=rev
> > Log:
> > Change uses of strncpy in debugserver to strlcpy
> > for better safety.
> >
> > 
> >
>
> Thanks!
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r313540 - Revert "Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)"

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Mon Sep 18 08:59:44 2017
New Revision: 313540

URL: http://llvm.org/viewvc/llvm-project?rev=313540=rev
Log:
Revert "Fix for bug 34532 - A few rough corners related to post-mortem 
debugging (core/minidump)"

Broke Windows and FreeBSD (at least).

This reverts commit 628ca7052b4a5dbace0f6205409113e12c8a78fa.

Modified:
lldb/trunk/include/lldb/Target/Process.h

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
lldb/trunk/source/Commands/CommandObjectThread.cpp
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h
lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/include/lldb/Target/Process.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=313540=313539=313540=diff
==
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Mon Sep 18 08:59:44 2017
@@ -1248,13 +1248,7 @@ public:
   /// @return
   /// Returns an error object.
   //--
-  virtual Status WillResume() {
-Status error;
-error.SetErrorStringWithFormat(
-"error: %s does not support resuming processes",
-GetPluginName().GetCString());
-return error;
-  }
+  virtual Status WillResume() { return Status(); }
 
   //--
   /// Resumes all of a process's threads as configured using the

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py?rev=313540=313539=313540=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
 Mon Sep 18 08:59:44 2017
@@ -244,34 +244,6 @@ class LinuxCoreTestCase(TestBase):
 end_region.GetRegionBase())
 self.assertEqual(end_region.GetRegionEnd(), lldb.LLDB_INVALID_ADDRESS)
 
-def check_state(self, process):
-with open(os.devnull) as devnul:
-# sanitize test output
-self.dbg.SetOutputFileHandle(devnul, False)
-self.dbg.SetErrorFileHandle(devnul, False)
-
-self.assertTrue(process.is_stopped)
-
-# Process.Continue
-error = process.Continue()
-self.assertFalse(error.Success())
-self.assertTrue(process.is_stopped)
-
-# Thread.StepOut
-thread = process.GetSelectedThread()
-thread.StepOut()
-self.assertTrue(process.is_stopped)
-
-# command line
-self.dbg.HandleCommand('s')
-self.assertTrue(process.is_stopped)
-self.dbg.HandleCommand('c')
-self.assertTrue(process.is_stopped)
-
-# restore file handles
-self.dbg.SetOutputFileHandle(None, False)
-self.dbg.SetErrorFileHandle(None, False)
-
 def do_test(self, filename, pid, region_count):
 target = self.dbg.CreateTarget(filename + ".out")
 process = target.LoadCore(filename + ".core")
@@ -279,8 +251,6 @@ class LinuxCoreTestCase(TestBase):
 self.assertEqual(process.GetNumThreads(), 1)
 self.assertEqual(process.GetProcessID(), pid)
 
-self.check_state(process)
-
 thread = process.GetSelectedThread()
 self.assertTrue(thread)
 self.assertEqual(thread.GetThreadID(), pid)

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py?rev=313540=313539=313540=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 Mon Sep 18 08:59:44 2017
@@ -31,34 +31,6 @@ class MiniDumpNewTestCase(TestBase):
 lldb.DBG.SetSelectedPlatform(self._initial_platform)
 super(MiniDumpNewTestCase, self).tearDown()
 
-def check_state(self):
-with open(os.devnull) as devnul:
-# sanitize test output
-self.dbg.SetOutputFileHandle(devnul, False)
-self.dbg.SetErrorFileHandle(devnul, False)
-
-self.assertTrue(self.process.is_stopped)
-
-   

[Lldb-commits] [lldb] r313210 - Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Wed Sep 13 15:57:11 2017
New Revision: 313210

URL: http://llvm.org/viewvc/llvm-project?rev=313210=rev
Log:
Fix for bug 34532 - A few rough corners related to post-mortem debugging 
(core/minidump)

The main change is to avoid setting the process state as running when
debugging core/minidumps (details in the bug).

Also included a few small, related fixes around how the errors propagate in
this case.

patch by lemo

Bug: https://bugs.llvm.org/show_bug.cgi?id=34532

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

Modified:
lldb/trunk/include/lldb/Target/Process.h

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
lldb/trunk/source/Commands/CommandObjectThread.cpp
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h
lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/include/lldb/Target/Process.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=313210=313209=313210=diff
==
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Wed Sep 13 15:57:11 2017
@@ -1248,7 +1248,13 @@ public:
   /// @return
   /// Returns an error object.
   //--
-  virtual Status WillResume() { return Status(); }
+  virtual Status WillResume() {
+Status error;
+error.SetErrorStringWithFormat(
+"error: %s does not support resuming processes",
+GetPluginName().GetCString());
+return error;
+  }
 
   //--
   /// Resumes all of a process's threads as configured using the

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py?rev=313210=313209=313210=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
 Wed Sep 13 15:57:11 2017
@@ -244,6 +244,34 @@ class LinuxCoreTestCase(TestBase):
 end_region.GetRegionBase())
 self.assertEqual(end_region.GetRegionEnd(), lldb.LLDB_INVALID_ADDRESS)
 
+def check_state(self, process):
+with open(os.devnull) as devnul:
+# sanitize test output
+self.dbg.SetOutputFileHandle(devnul, False)
+self.dbg.SetErrorFileHandle(devnul, False)
+
+self.assertTrue(process.is_stopped)
+
+# Process.Continue
+error = process.Continue()
+self.assertFalse(error.Success())
+self.assertTrue(process.is_stopped)
+
+# Thread.StepOut
+thread = process.GetSelectedThread()
+thread.StepOut()
+self.assertTrue(process.is_stopped)
+
+# command line
+self.dbg.HandleCommand('s')
+self.assertTrue(process.is_stopped)
+self.dbg.HandleCommand('c')
+self.assertTrue(process.is_stopped)
+
+# restore file handles
+self.dbg.SetOutputFileHandle(None, False)
+self.dbg.SetErrorFileHandle(None, False)
+
 def do_test(self, filename, pid, region_count):
 target = self.dbg.CreateTarget(filename + ".out")
 process = target.LoadCore(filename + ".core")
@@ -251,6 +279,8 @@ class LinuxCoreTestCase(TestBase):
 self.assertEqual(process.GetNumThreads(), 1)
 self.assertEqual(process.GetProcessID(), pid)
 
+self.check_state(process)
+
 thread = process.GetSelectedThread()
 self.assertTrue(thread)
 self.assertEqual(thread.GetThreadID(), pid)

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py?rev=313210=313209=313210=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 Wed Sep 13 15:57:11 2017
@@ -31,6 +31,34 @@ class MiniDumpNewTestCase(TestBase):
 lldb.DBG.SetSelectedPlatform(self._initial_platform)
 super(MiniDumpNewTestCase, self).tearDown()
 
+def check_state(self):
+with open(os.devnull) as devnul:
+  

[Lldb-commits] [lldb] r312735 - Fix for bug 34510 - Minidump target does not resolve new symbols correctly

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Thu Sep  7 11:29:48 2017
New Revision: 312735

URL: http://llvm.org/viewvc/llvm-project?rev=312735=rev
Log:
Fix for bug 34510 - Minidump target does not resolve new symbols correctly

Even though the content of the minidump does not change in a debugging session,
frames can't be indiscriminately be cached since modules and symbols can be
explicitly added after the minidump is loaded.

The fix is simple, just let the base Thread::ClearStackFrames() do its job.

submitted by amccarth on behalf of lemo

Bug: https://bugs.llvm.org/show_bug.cgi?id=34510

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

Modified:
lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.cpp
lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.h

Modified: lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.cpp?rev=312735=312734=312735=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.cpp Thu Sep  7 
11:29:48 2017
@@ -43,8 +43,6 @@ ThreadMinidump::~ThreadMinidump() {}
 
 void ThreadMinidump::RefreshStateAfterStop() {}
 
-void ThreadMinidump::ClearStackFrames() {}
-
 RegisterContextSP ThreadMinidump::GetRegisterContext() {
   if (!m_reg_context_sp) {
 m_reg_context_sp = CreateRegisterContextForFrame(nullptr);

Modified: lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.h?rev=312735=312734=312735=diff
==
--- lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.h (original)
+++ lldb/trunk/source/Plugins/Process/minidump/ThreadMinidump.h Thu Sep  7 
11:29:48 2017
@@ -37,8 +37,6 @@ public:
   lldb::RegisterContextSP
   CreateRegisterContextForFrame(StackFrame *frame) override;
 
-  void ClearStackFrames() override;
-
 protected:
   lldb::RegisterContextSP m_thread_reg_ctx_sp;
   llvm::ArrayRef m_gpregset_data;


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

Re: [Lldb-commits] [lldb] r300610 - Fix Windows bot failure

[Adrian McCarthy via lldb-commits]

Actually, Windows does have `struct timespec`.  It's defined in ``
and ``.

https://msdn.microsoft.com/en-us/library/mt633792.aspx

I suspect somebody had suppressed it for some reason.

On Tue, Apr 18, 2017 at 2:47 PM, Chris Bieneman via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: cbieneman
> Date: Tue Apr 18 16:47:50 2017
> New Revision: 300610
>
> URL: http://llvm.org/viewvc/llvm-project?rev=300610=rev
> Log:
> Fix Windows bot failure
>
> timespec is not available on Windows, and we should use size_t instead of
> nfds_t.
>
> Modified:
> lldb/trunk/source/Host/common/MainLoop.cpp
>
> Modified: lldb/trunk/source/Host/common/MainLoop.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/
> Host/common/MainLoop.cpp?rev=300610=300609=300610=diff
> 
> ==
> --- lldb/trunk/source/Host/common/MainLoop.cpp (original)
> +++ lldb/trunk/source/Host/common/MainLoop.cpp Tue Apr 18 16:47:50 2017
> @@ -33,7 +33,14 @@
>
>  #if !HAVE_PPOLL && !HAVE_SYS_EVENT_H
>
> -int ppoll(struct pollfd *fds, nfds_t nfds, const struct timespec
> *timeout_ts,
> +#ifdef LLVM_ON_WIN32
> +struct timespec {
> +  time_t   tv_sec;
> +  suseconds_t  tv_nsec;
> +};
> +#endif
> +
> +int ppoll(struct pollfd *fds, size_t nfds, const struct timespec
> *timeout_ts,
>const sigset_t *) {
>int timeout =
>(timeout_ts == nullptr)
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r293336 - NFC: Improve comments in SymbolFilePDB.cpp

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Fri Jan 27 15:42:28 2017
New Revision: 293336

URL: http://llvm.org/viewvc/llvm-project?rev=293336=rev
Log:
NFC:  Improve comments in SymbolFilePDB.cpp

Mostly this just fixes bad wrapping caused by the reformat, with tiny
changes sprinkled here and there.

Modified:
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp?rev=293336=293335=293336=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Fri Jan 27 
15:42:28 2017
@@ -125,9 +125,8 @@ uint32_t SymbolFilePDB::GetNumCompileUni
 m_cached_compile_unit_count = compilands->getChildCount();
 
 // The linker can inject an additional "dummy" compilation unit into the
-// PDB.
-// Ignore this special compile unit for our purposes, if it is there.  It 
is
-// always the last one.
+// PDB. Ignore this special compile unit for our purposes, if it is there.
+// It is always the last one.
 auto last_cu = compilands->getChildAtIndex(m_cached_compile_unit_count - 
1);
 std::string name = last_cu->getName();
 if (name == "* Linker *")
@@ -187,12 +186,10 @@ bool SymbolFilePDB::ParseCompileUnitSupp
 return false;
 
   // In theory this is unnecessary work for us, because all of this information
-  // is easily
-  // (and quickly) accessible from DebugInfoPDB, so caching it a second time
-  // seems like a waste.
-  // Unfortunately, there's no good way around this short of a moderate
-  // refactor, since SymbolVendor
-  // depends on being able to cache this list.
+  // is easily (and quickly) accessible from DebugInfoPDB, so caching it a
+  // second time seems like a waste.  Unfortunately, there's no good way around
+  // this short of a moderate refactor since SymbolVendor depends on being able
+  // to cache this list.
   auto cu = m_session_up->getConcreteSymbolById(
   sc.comp_unit->GetID());
   if (!cu)
@@ -269,9 +266,8 @@ lldb_private::CompilerDecl SymbolFilePDB
 lldb_private::CompilerDeclContext
 SymbolFilePDB::GetDeclContextForUID(lldb::user_id_t uid) {
   // PDB always uses the translation unit decl context for everything.  We can
-  // improve this later
-  // but it's not easy because PDB doesn't provide a high enough level of type
-  // fidelity in this area.
+  // improve this later but it's not easy because PDB doesn't provide a high
+  // enough level of type fidelity in this area.
   return *m_tu_decl_ctx_up;
 }
 
@@ -295,30 +291,25 @@ uint32_t SymbolFilePDB::ResolveSymbolCon
 uint32_t resolve_scope, lldb_private::SymbolContextList _list) {
   if (resolve_scope & lldb::eSymbolContextCompUnit) {
 // Locate all compilation units with line numbers referencing the specified
-// file.  For example, if
-// `file_spec` is , then this should return all source files and
-// header files that reference
-// , either directly or indirectly.
+// file.  For example, if `file_spec` is , then this should return
+// all source files and header files that reference , either
+// directly or indirectly.
 auto compilands = m_session_up->findCompilandsForSourceFile(
 file_spec.GetPath(), PDB_NameSearchFlags::NS_CaseInsensitive);
 
-// For each one, either find get its previously parsed data, or parse it
-// afresh and add it to
-// the symbol context list.
+// For each one, either find its previously parsed data or parse it afresh
+// and add it to the symbol context list.
 while (auto compiland = compilands->getNext()) {
   // If we're not checking inlines, then don't add line information for 
this
-  // file unless the FileSpec
-  // matches.
+  // file unless the FileSpec matches.
   if (!check_inlines) {
 // `getSourceFileName` returns the basename of the original source file
-// used to generate this compiland.
-// It does not return the full path.  Currently the only way to get 
that
-// is to do a basename lookup to
-// get the IPDBSourceFile, but this is ambiguous in the case of two
-// source files with the same name
-// contributing to the same compiland.  This is a moderately extreme
-// edge case, so we consider this ok
-// for now, although we need to find a long term solution.
+// used to generate this compiland.  It does not return the full path.
+// Currently the only way to get that is to do a basename lookup to get
+// the IPDBSourceFile, but this is ambiguous in the case of two source
+// files with the same name contributing to the same compiland.  This 
is
+// a moderately extreme edge case, so we consider this OK for now,
+// 

[Lldb-commits] [lldb] r287113 - Remove Windows-specific minidump plugin

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Wed Nov 16 10:04:14 2016
New Revision: 287113

URL: http://llvm.org/viewvc/llvm-project?rev=287113=rev
Log:
Remove Windows-specific minidump plugin

With the cross-platform minidump plugin working, the Windows-specific one is no 
longer needed. This eliminates the unnecessary code.

This does not eliminate the Windows-specific tests, as they hit a few cases the 
general tests don't. (The Windows-specific tests are currently passing.) I'll 
look into a separate patch to make sure we're not doing too much duplicate 
testing.

After that I might do a little re-org in the Windows plugin, as there was some 
factoring there (Common & Live) that probably isn't necessary anymore.

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

Removed:
lldb/trunk/source/Plugins/Process/Windows/MiniDump/CMakeLists.txt
lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp
lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.h
lldb/trunk/source/Plugins/Process/Windows/MiniDump/ThreadWinMiniDump.cpp
lldb/trunk/source/Plugins/Process/Windows/MiniDump/ThreadWinMiniDump.h

lldb/trunk/source/Plugins/Process/Windows/MiniDump/x64/RegisterContextWindowsMiniDump_x64.cpp

lldb/trunk/source/Plugins/Process/Windows/MiniDump/x64/RegisterContextWindowsMiniDump_x64.h

lldb/trunk/source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.cpp

lldb/trunk/source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.h
Modified:
lldb/trunk/cmake/LLDBDependencies.cmake
lldb/trunk/source/Plugins/Process/CMakeLists.txt

Modified: lldb/trunk/cmake/LLDBDependencies.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/LLDBDependencies.cmake?rev=287113=287112=287113=diff
==
--- lldb/trunk/cmake/LLDBDependencies.cmake (original)
+++ lldb/trunk/cmake/LLDBDependencies.cmake Wed Nov 16 10:04:14 2016
@@ -21,7 +21,7 @@ set( LLDB_USED_LIBS
   lldbPluginDynamicLoaderPosixDYLD
   lldbPluginDynamicLoaderHexagonDYLD
   lldbPluginDynamicLoaderWindowsDYLD
-  
+
   lldbPluginCPlusPlusLanguage
   lldbPluginGoLanguage
   lldbPluginJavaLanguage
@@ -91,7 +91,6 @@ set( LLDB_USED_LIBS
 if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
   list(APPEND LLDB_USED_LIBS
 lldbPluginProcessWindows
-lldbPluginProcessWinMiniDump
 lldbPluginProcessWindowsCommon
 Ws2_32
 Rpcrt4

Modified: lldb/trunk/source/Plugins/Process/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/CMakeLists.txt?rev=287113=287112=287113=diff
==
--- lldb/trunk/source/Plugins/Process/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/Process/CMakeLists.txt Wed Nov 16 10:04:14 2016
@@ -8,7 +8,6 @@ elseif (CMAKE_SYSTEM_NAME MATCHES "NetBS
   add_subdirectory(POSIX)
 elseif (CMAKE_SYSTEM_NAME MATCHES "Windows")
   add_subdirectory(Windows/Live)
-  add_subdirectory(Windows/MiniDump)
   add_subdirectory(Windows/Common)
 elseif (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(MacOSX-Kernel)

Removed: lldb/trunk/source/Plugins/Process/Windows/MiniDump/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/MiniDump/CMakeLists.txt?rev=287112=auto
==
--- lldb/trunk/source/Plugins/Process/Windows/MiniDump/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/Process/Windows/MiniDump/CMakeLists.txt (removed)
@@ -1,21 +0,0 @@
-include_directories(../../Utility)
-include_directories(../Common)
-
-set(PROC_WINDOWS_MINIDUMP_SOURCES
-  ProcessWinMiniDump.cpp
-  ThreadWinMiniDump.cpp
-  )
-
-if (CMAKE_SIZEOF_VOID_P EQUAL 4)
-  set(PROC_WINDOWS_MINIDUMP_SOURCES ${PROC_WINDOWS_MINIDUMP_SOURCES}
-x86/RegisterContextWindowsMiniDump_x86.cpp
-)
-elseif (CMAKE_SIZEOF_VOID_P EQUAL 8)
-  set(PROC_WINDOWS_MINIDUMP_SOURCES ${PROC_WINDOWS_MINIDUMP_SOURCES}
-x64/RegisterContextWindowsMiniDump_x64.cpp
-)
-endif()
-
-add_lldb_library(lldbPluginProcessWinMiniDump
-  ${PROC_WINDOWS_MINIDUMP_SOURCES}
-  )

Removed: 
lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp?rev=287112=auto
==
--- lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp 
(removed)
@@ -1,631 +0,0 @@
-//===-- ProcessWinMiniDump.cpp --*- C++ 
-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for 

[Lldb-commits] [PATCH] D26697: Remove Windows-specific minidump plugin

[Adrian McCarthy via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL287113: Remove Windows-specific minidump plugin (authored by 
amccarth).

Changed prior to commit:
  https://reviews.llvm.org/D26697?vs=78065=78194#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26697

Files:
  lldb/trunk/cmake/LLDBDependencies.cmake
  lldb/trunk/source/Plugins/Process/CMakeLists.txt
  lldb/trunk/source/Plugins/Process/Windows/MiniDump/CMakeLists.txt
  lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp
  lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.h
  lldb/trunk/source/Plugins/Process/Windows/MiniDump/ThreadWinMiniDump.cpp
  lldb/trunk/source/Plugins/Process/Windows/MiniDump/ThreadWinMiniDump.h
  
lldb/trunk/source/Plugins/Process/Windows/MiniDump/x64/RegisterContextWindowsMiniDump_x64.cpp
  
lldb/trunk/source/Plugins/Process/Windows/MiniDump/x64/RegisterContextWindowsMiniDump_x64.h
  
lldb/trunk/source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.cpp
  
lldb/trunk/source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.h

Index: lldb/trunk/source/Plugins/Process/CMakeLists.txt
===
--- lldb/trunk/source/Plugins/Process/CMakeLists.txt
+++ lldb/trunk/source/Plugins/Process/CMakeLists.txt
@@ -8,7 +8,6 @@
   add_subdirectory(POSIX)
 elseif (CMAKE_SYSTEM_NAME MATCHES "Windows")
   add_subdirectory(Windows/Live)
-  add_subdirectory(Windows/MiniDump)
   add_subdirectory(Windows/Common)
 elseif (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(MacOSX-Kernel)
Index: lldb/trunk/source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.h
===
--- lldb/trunk/source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.h
+++ lldb/trunk/source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.h
@@ -1,37 +0,0 @@
-//===-- RegisterContextWindowsMiniDump_x86.h *- C++
-//-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef liblldb_RegisterContextWindowsMiniDump_x86_H_
-#define liblldb_RegisterContextWindowsMiniDump_x86_H_
-
-#include "Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.h"
-#include "lldb/lldb-forward.h"
-
-namespace lldb_private {
-
-class Thread;
-
-class RegisterContextWindowsMiniDump_x86 : public RegisterContextWindows_x86 {
-public:
-  RegisterContextWindowsMiniDump_x86(Thread ,
- uint32_t concrete_frame_idx,
- const CONTEXT *context);
-
-  virtual ~RegisterContextWindowsMiniDump_x86();
-
-  bool WriteRegister(const RegisterInfo *reg_info,
- const RegisterValue _value) override;
-
-protected:
-  bool CacheAllRegisterValues() override;
-};
-}
-
-#endif // #ifndef liblldb_RegisterContextWindowsMiniDump_x86_H_
Index: lldb/trunk/source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.cpp
===
--- lldb/trunk/source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.cpp
+++ lldb/trunk/source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.cpp
@@ -1,42 +0,0 @@
-//===-- RegisterContextWindowsMiniDump_x86.cpp --*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "lldb/Host/windows/windows.h"
-#include "lldb/lldb-private-types.h"
-
-#include "RegisterContextWindowsMiniDump_x86.h"
-
-using namespace lldb;
-
-namespace lldb_private {
-
-RegisterContextWindowsMiniDump_x86::RegisterContextWindowsMiniDump_x86(
-Thread , uint32_t concrete_frame_idx, const CONTEXT *context)
-: RegisterContextWindows_x86(thread, concrete_frame_idx) {
-  if (context) {
-m_context = *context;
-m_context_stale = false;
-  }
-}
-
-RegisterContextWindowsMiniDump_x86::~RegisterContextWindowsMiniDump_x86() {}
-
-bool RegisterContextWindowsMiniDump_x86::WriteRegister(
-const RegisterInfo * /* reg_info */,
-const RegisterValue & /* reg_value */) {
-  return false;
-}
-
-bool RegisterContextWindowsMiniDump_x86::CacheAllRegisterValues() {
-  // Since this is post-mortem debugging, we either have the context or we
-  // don't.
-  return !m_context_stale;
-}
-
-} // namespace lldb_private
Index: lldb/trunk/source/Plugins/Process/Windows/MiniDump/x64/RegisterContextWindowsMiniDump_x64.h

[Lldb-commits] [PATCH] D26697: Remove Windows-specific minidump plugin

[Adrian McCarthy via lldb-commits]

amccarth created this revision.
amccarth added reviewers: labath, zturner.
amccarth added a subscriber: lldb-commits.
Herald added subscribers: modocache, mgorny.

With the cross-platform minidump plugin working, the Windows-specific one is no 
longer needed.  This eliminates the unnecessary code.

This does not eliminate the Windows-specific tests, as they hit a few cases the 
general tests don't.  (The Windows-specific tests are currently passing.)  I'll 
look into a separate patch to make sure we're not doing too much duplicate 
testing.

After that I might do a little re-org in the Windows plugin, as there was some 
factoring there (Common & Live) that probably isn't necessary anymore.


https://reviews.llvm.org/D26697

Files:
  cmake/LLDBDependencies.cmake
  source/Plugins/Process/CMakeLists.txt
  source/Plugins/Process/Windows/MiniDump/CMakeLists.txt
  source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp
  source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.h
  source/Plugins/Process/Windows/MiniDump/ThreadWinMiniDump.cpp
  source/Plugins/Process/Windows/MiniDump/ThreadWinMiniDump.h
  
source/Plugins/Process/Windows/MiniDump/x64/RegisterContextWindowsMiniDump_x64.cpp
  
source/Plugins/Process/Windows/MiniDump/x64/RegisterContextWindowsMiniDump_x64.h
  
source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.cpp
  
source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.h

Index: source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.h
===
--- source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.h
+++ /dev/null
@@ -1,37 +0,0 @@
-//===-- RegisterContextWindowsMiniDump_x86.h *- C++
-//-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef liblldb_RegisterContextWindowsMiniDump_x86_H_
-#define liblldb_RegisterContextWindowsMiniDump_x86_H_
-
-#include "Plugins/Process/Windows/Common/x86/RegisterContextWindows_x86.h"
-#include "lldb/lldb-forward.h"
-
-namespace lldb_private {
-
-class Thread;
-
-class RegisterContextWindowsMiniDump_x86 : public RegisterContextWindows_x86 {
-public:
-  RegisterContextWindowsMiniDump_x86(Thread ,
- uint32_t concrete_frame_idx,
- const CONTEXT *context);
-
-  virtual ~RegisterContextWindowsMiniDump_x86();
-
-  bool WriteRegister(const RegisterInfo *reg_info,
- const RegisterValue _value) override;
-
-protected:
-  bool CacheAllRegisterValues() override;
-};
-}
-
-#endif // #ifndef liblldb_RegisterContextWindowsMiniDump_x86_H_
Index: source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.cpp
===
--- source/Plugins/Process/Windows/MiniDump/x86/RegisterContextWindowsMiniDump_x86.cpp
+++ /dev/null
@@ -1,42 +0,0 @@
-//===-- RegisterContextWindowsMiniDump_x86.cpp --*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "lldb/Host/windows/windows.h"
-#include "lldb/lldb-private-types.h"
-
-#include "RegisterContextWindowsMiniDump_x86.h"
-
-using namespace lldb;
-
-namespace lldb_private {
-
-RegisterContextWindowsMiniDump_x86::RegisterContextWindowsMiniDump_x86(
-Thread , uint32_t concrete_frame_idx, const CONTEXT *context)
-: RegisterContextWindows_x86(thread, concrete_frame_idx) {
-  if (context) {
-m_context = *context;
-m_context_stale = false;
-  }
-}
-
-RegisterContextWindowsMiniDump_x86::~RegisterContextWindowsMiniDump_x86() {}
-
-bool RegisterContextWindowsMiniDump_x86::WriteRegister(
-const RegisterInfo * /* reg_info */,
-const RegisterValue & /* reg_value */) {
-  return false;
-}
-
-bool RegisterContextWindowsMiniDump_x86::CacheAllRegisterValues() {
-  // Since this is post-mortem debugging, we either have the context or we
-  // don't.
-  return !m_context_stale;
-}
-
-} // namespace lldb_private
Index: source/Plugins/Process/Windows/MiniDump/x64/RegisterContextWindowsMiniDump_x64.h
===
--- source/Plugins/Process/Windows/MiniDump/x64/RegisterContextWindowsMiniDump_x64.h
+++ /dev/null
@@ -1,36 +0,0 @@
-//===-- RegisterContextWindowsMiniDump_x64.h *- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. 

[Lldb-commits] [PATCH] D26643: Fix TestMiniDumpNew.py test for Python 2/3 issue

[Adrian McCarthy via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL286909: Fix TestMiniDumpNew.py test for Python 2/3 issue 
(authored by amccarth).

Changed prior to commit:
  https://reviews.llvm.org/D26643?vs=77900=77907#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26643

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py


Index: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -108,15 +108,16 @@
 /proc/PID/status which is written in the file
 """
 shutil.copyfile(core, newcore)
-with open(newcore, "r+") as f:
+with open(newcore, "rb+") as f:
 f.seek(offset)
-self.assertEqual(f.read(5), oldpid)
+currentpid = f.read(5).decode('utf-8')
+self.assertEqual(currentpid, oldpid)
 
 f.seek(offset)
 if len(newpid) < len(oldpid):
 newpid += " " * (len(oldpid) - len(newpid))
 newpid += "\n"
-f.write(newpid)
+f.write(newpid.encode('utf-8'))
 
 def test_deeper_stack_in_minidump_with_same_pid_running(self):
 """Test that we read the information from the core correctly even if we


Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -108,15 +108,16 @@
 /proc/PID/status which is written in the file
 """
 shutil.copyfile(core, newcore)
-with open(newcore, "r+") as f:
+with open(newcore, "rb+") as f:
 f.seek(offset)
-self.assertEqual(f.read(5), oldpid)
+currentpid = f.read(5).decode('utf-8')
+self.assertEqual(currentpid, oldpid)
 
 f.seek(offset)
 if len(newpid) < len(oldpid):
 newpid += " " * (len(oldpid) - len(newpid))
 newpid += "\n"
-f.write(newpid)
+f.write(newpid.encode('utf-8'))
 
 def test_deeper_stack_in_minidump_with_same_pid_running(self):
 """Test that we read the information from the core correctly even if we
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r286909 - Fix TestMiniDumpNew.py test for Python 2/3 issue

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Mon Nov 14 17:53:45 2016
New Revision: 286909

URL: http://llvm.org/viewvc/llvm-project?rev=286909=rev
Log:
Fix TestMiniDumpNew.py test for Python 2/3 issue

On Windows, where we use Python 3 for testing, we have to be more explicit 
about converting between binary and string representations.  I believe this 
should still work for Python 2, but I don't have a convenient way to try it out.

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py?rev=286909=286908=286909=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
 Mon Nov 14 17:53:45 2016
@@ -108,15 +108,16 @@ class MiniDumpNewTestCase(TestBase):
 /proc/PID/status which is written in the file
 """
 shutil.copyfile(core, newcore)
-with open(newcore, "r+") as f:
+with open(newcore, "rb+") as f:
 f.seek(offset)
-self.assertEqual(f.read(5), oldpid)
+currentpid = f.read(5).decode('utf-8')
+self.assertEqual(currentpid, oldpid)
 
 f.seek(offset)
 if len(newpid) < len(oldpid):
 newpid += " " * (len(oldpid) - len(newpid))
 newpid += "\n"
-f.write(newpid)
+f.write(newpid.encode('utf-8'))
 
 def test_deeper_stack_in_minidump_with_same_pid_running(self):
 """Test that we read the information from the core correctly even if we


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

[Lldb-commits] [PATCH] D26643: Fix TestMiniDumpNew.py test for Python 2/3 issue

[Adrian McCarthy via lldb-commits]

amccarth created this revision.
amccarth added a reviewer: zturner.
amccarth added a subscriber: lldb-commits.

On Windows, where we use Python 3 for testing, we have to be more explicit 
about converting between binary and string representations.  I believe this 
should still work for Python 2, but I don't have a convenient way to try it out.


https://reviews.llvm.org/D26643

Files:
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py


Index: 
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
===
--- 
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ 
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -108,15 +108,16 @@
 /proc/PID/status which is written in the file
 """
 shutil.copyfile(core, newcore)
-with open(newcore, "r+") as f:
+with open(newcore, "rb+") as f:
 f.seek(offset)
-self.assertEqual(f.read(5), oldpid)
+currentpid = f.read(5).decode('utf-8')
+self.assertEqual(currentpid, oldpid)
 
 f.seek(offset)
 if len(newpid) < len(oldpid):
 newpid += " " * (len(oldpid) - len(newpid))
 newpid += "\n"
-f.write(newpid)
+f.write(newpid.encode('utf-8'))
 
 def test_deeper_stack_in_minidump_with_same_pid_running(self):
 """Test that we read the information from the core correctly even if we


Index: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
===
--- packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -108,15 +108,16 @@
 /proc/PID/status which is written in the file
 """
 shutil.copyfile(core, newcore)
-with open(newcore, "r+") as f:
+with open(newcore, "rb+") as f:
 f.seek(offset)
-self.assertEqual(f.read(5), oldpid)
+currentpid = f.read(5).decode('utf-8')
+self.assertEqual(currentpid, oldpid)
 
 f.seek(offset)
 if len(newpid) < len(oldpid):
 newpid += " " * (len(oldpid) - len(newpid))
 newpid += "\n"
-f.write(newpid)
+f.write(newpid.encode('utf-8'))
 
 def test_deeper_stack_in_minidump_with_same_pid_running(self):
 """Test that we read the information from the core correctly even if we
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D26393: Disable windows-only minidump plugin

[Adrian McCarthy via lldb-commits]

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

Problem was at my end.  This patch works fine for me now.

I'm planning to do some more minidump work, so I'd be happy to take over with 
eliminating the old Windows-specific implementation.


https://reviews.llvm.org/D26393



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

[Lldb-commits] [PATCH] D26393: Disable windows-only minidump plugin

[Adrian McCarthy via lldb-commits]

amccarth added a comment.

In https://reviews.llvm.org/D26393#589421, @labath wrote:

> In https://reviews.llvm.org/D26393#589363, @amccarth wrote:
>
> > I started testing the new plugin on Windows yesterday, and it doesn't work 
> > (all the tests fail).  I'm planning to debug today, and, once I get it 
> > working, I'd be happy to switch it over.
>
>
> Interesting. It seemed to work fine for me. How did you test that?


I made changes very similar to this patch and ran check-lldb.  The tests for 
the new minidump plugin passed, but the original Windows ones all failed, 
apparently because it cannot find any threads in the dumps.  I'll have more 
time to look at it this afternoon.


https://reviews.llvm.org/D26393



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

[Lldb-commits] [PATCH] D26393: Disable windows-only minidump plugin

[Adrian McCarthy via lldb-commits]

amccarth added a comment.

I started testing the new plugin on Windows yesterday, and it doesn't work (all 
the tests fail).  I'm planning to debug today, and, once I get it working, I'd 
be happy to switch it over.


https://reviews.llvm.org/D26393



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

[Lldb-commits] [PATCH] D25905: Minidump plugin: Adding ProcessMinidump, ThreadMinidump and register the plugin in SystemInitializerFull

[Adrian McCarthy via lldb-commits]

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

I like that this keeps the WoW64 detection and support.  That's a very 
Windows-specific thing, and I was concerned that doing generic minidump parsing 
could lose this.

This looks really good.  On Monday or Tuesday, I'll try patching it into a 
Windows build to see if it's complete enough to remove the Windows-specific 
implementation.




Comment at: 
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/makefile.txt:16
+# Then the binaries dynamically link to that lib.
+# The other optimisation is not using the standart library (hense the _start
+# instead of main). We only link dynamically to some standart libraries.

s/hense/hence/



Comment at: 
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/makefile.txt:17
+# The other optimisation is not using the standart library (hense the _start
+# instead of main). We only link dynamically to some standart libraries.
+# This way we have a tiny binary (~8K) that has debug symbols and uses breakpad

x/standart/standard/



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:273
+// 64 bit windows
+if (llvm::StringRef(name.getValue()).endswith_lower("wow64.dll")) {
+  m_is_wow64 = true;

This is probably fine, but it's a slightly less precise check than what we're 
doing in ProcessWinMiniDump.cpp.  The endswith check would  accept 
"bowwow64.dll".  It's probably not important, since detecting WoW64 is by 
looking at module names is a hack anyway.


https://reviews.llvm.org/D25905



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

[Lldb-commits] [PATCH] D25832: Minidump plugin: Adding x86_32 register context converter

[Adrian McCarthy via lldb-commits]

amccarth accepted this revision.
amccarth added a reviewer: amccarth.
amccarth added inline comments.



Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:340
   std::map reg_values;
 
+  reg_values[lldb_rax_x86_64] = 0x;

It seems a shame to remove the `// clang-format off` comment for this section.  
The aligned version is easier to read.


https://reviews.llvm.org/D25832



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

[Lldb-commits] [PATCH] D26081: Improve ".." handling in FileSpec normalization

[Adrian McCarthy via lldb-commits]

amccarth added inline comments.



Comment at: source/Host/common/FileSpec.cpp:550
+  (m_filename.GetStringRef() != ".." && m_filename.GetStringRef() != "."))
+return *this;
 

Do we have to worry about an unnecessary single dot in the directory, like 
`/foo/./bar/`?  Are those handled when the FileSpec is constructed?



Comment at: source/Host/common/FileSpec.cpp:576
+if (component == ".")
+  continue; // Skip these.
+if (component != "..") {

Ah, here we're skipping the unnecessary single dots, but the short-circuit at 
the top would prevent us from getting here unless the directory also contained 
a `/..`.



Comment at: unittests/Host/FileSpecTest.cpp:144
   {R"(C:\bar)", R"(C:\foo\..\bar)"},
   };
 

How about a test to make sure `C:/foo/./bar` is the same as `C:/foo/bar`?



Comment at: unittests/Host/FileSpecTest.cpp:212
+  {"foo/../bar", "bar"},
+  {"../foo/..", ".."},
+  };

Here, too, I'd like to see test cases for paths with single dots.


https://reviews.llvm.org/D26081



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

[Lldb-commits] [PATCH] D25247: Make LLDB -Werror clean under clang

[Adrian McCarthy via lldb-commits]

amccarth accepted this revision.
amccarth added a comment.

lgtm



> UDPSocket.cpp:106
> +#if defined(_MSC_VER) && defined(UNICODE)
> +"getaddrinfo(%s, %s, , ) returned error %i (%S)",
> +#else

Yuck.  Given that this is going to get reduced from UTF-16 to MBCS, it might be 
cleaner to leave the format string alone and call gai_strerrorA explicitly on 
Windows.  I guess it would be just as ugly that way.

> SelectHelper.cpp:122
> +  updateMaxFd(max_error_fd, fd);
> +updateMaxFd(max_fd, fd);
>}

I find this logic less clear than the original version, since it's not obvious 
that `updateMaxFd` uses an in-out parameter until you go look up to see what it 
does.  It also seems weird to have an output parameter come before an 
input-only parameter.

Perhaps if `updateMaxFd` returned the value instead of updating the parameter, 
it would be more obvious:

  max_fd = updateMaxFd(max_fd, fd);

https://reviews.llvm.org/D25247



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

[Lldb-commits] [PATCH] D25196: Adding a new Minidump post-mortem debugging plugin

[Adrian McCarthy via lldb-commits]

amccarth added a comment.

I was hoping that, with your new mini dump parser, you'd be able to eliminate 
the need for the Windows-specific minidump process plugin.

When I wrote the Windows mini dump plugin, I tried to isolate the Windows 
API-specific bits using the pimpl idiom.  Now that you've written a mini dump 
parser, we shouldn't need the Windows API calls, and nearly all the rest of the 
code should be shareable between Windows and Linux.  Is there a plan to 
eliminate this redundancy and merge this new mini dump process plugin with the 
Windows-specific one?


https://reviews.llvm.org/D25196



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

[Lldb-commits] [PATCH] D25099: Refactor Args a different way

[Adrian McCarthy via lldb-commits]

amccarth added a comment.

Just a drive by.



> Args.h:449
>//--
>// Classes that inherit from Args can see and modify these
>//--

This comment is no longer true given the change from protected to private just 
above.

> Args.cpp:96
> +ParseSingleArgument(llvm::StringRef command) {
> +  // Argument can be split into multiple discontiguous pieces, // for 
> example:
>//  "Hello ""World"

Minor formatting glitch with the `\\` in the comment.

> Args.cpp:192
> +//--
> +// We have to be very careful on the copy constructor of this class
> +// to make sure we copy all of the string values, but we can't copy the

This says "copy constructor" but it seems to be documenting the copy assignment 
operator.

> Args.cpp:195
> +// rhs.m_argv into m_argv since it will point to the "const char *" c
> +// strings in rhs.m_args. We need to copy the string list and update our
> +// own m_argv appropriately.

You got right of m_args, so maybe this whole comment needs a rewrite.

> Args.cpp:282
> +
> +  // Now m_argv might be out of date with m_args, so we need to fix that.
> +  // This happens because getopt_long_only may permute the order of the

`m_args` is gone.

https://reviews.llvm.org/D25099



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

[Lldb-commits] [lldb] r282871 - Add namespace qualifiers for UTF functions that just moved.

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Fri Sep 30 11:11:42 2016
New Revision: 282871

URL: http://llvm.org/viewvc/llvm-project?rev=282871=rev
Log:
Add namespace qualifiers for UTF functions that just moved.

Modified:
lldb/trunk/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp

Modified: lldb/trunk/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp?rev=282871=282870=282871=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp Fri Sep 30 
11:11:42 2016
@@ -33,7 +33,7 @@ bool SaveMiniDump(const lldb::ProcessSP
   std::wstring wide_name;
   wide_name.resize(file_name.size() + 1);
   char *result_ptr = reinterpret_cast(_name[0]);
-  const UTF8 *error_ptr = nullptr;
+  const llvm::UTF8 *error_ptr = nullptr;
   if (!llvm::ConvertUTF8toWide(sizeof(wchar_t), file_name, result_ptr,
error_ptr)) {
 error.SetErrorString("cannot convert file name");

Modified: 
lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp?rev=282871=282870=282871=diff
==
--- lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp 
Fri Sep 30 11:11:42 2016
@@ -505,17 +505,17 @@ std::string ProcessWinMiniDump::Impl::Ge
   }
   auto md_string = reinterpret_cast(
   static_cast(m_base_addr) + rva);
-  auto source_start = reinterpret_cast(md_string->Buffer);
+  auto source_start = reinterpret_cast(md_string->Buffer);
   const auto source_length = ::wcslen(md_string->Buffer);
   const auto source_end = source_start + source_length;
   result.resize(UNI_MAX_UTF8_BYTES_PER_CODE_POINT *
 source_length); // worst case length
-  auto result_start = reinterpret_cast([0]);
+  auto result_start = reinterpret_cast([0]);
   const auto result_end = result_start + result.size();
-  ConvertUTF16toUTF8(_start, source_end, _start, result_end,
- strictConversion);
+  llvm::ConvertUTF16toUTF8(_start, source_end, _start, 
result_end,
+   llvm::ConversionFlags::strictConversion);
   const auto result_size =
-  std::distance(reinterpret_cast([0]), result_start);
+  std::distance(reinterpret_cast([0]), result_start);
   result.resize(result_size); // shrink to actual length
   return result;
 }


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

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

[Adrian McCarthy via lldb-commits]

amccarth added inline comments.


Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:177
@@ +176,3 @@
+#define REG_VAL(x) *(reinterpret_cast(x))
+
+TEST_F(MinidumpParserTest, ConvertRegisterContext) {

`EXPECT_xxx` will check the condition and report if it's wrong.  But then the 
test proceeds.

`ASSERT_xxx` will check the condition, and, if it's wrong, it will report _and_ 
terminate the current test.

So my rule of thumb is:

1.  Prefer `EXPECT_xxx` when checking that the code does what it's supposed to 
do.
2.  Use `ASSERT_xxx` when the rest of the test depends on this condition.


https://reviews.llvm.org/D24919



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

Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv

[Adrian McCarthy via lldb-commits]

amccarth added a subscriber: amccarth.


Comment at: include/lldb/Interpreter/Args.h:154
@@ -167,3 +153,3 @@
   //--
-  const char **GetConstArgumentVector() const;
+  void GetArgumentVector(std::vector ) const;
 

Perhaps this should actually return the vector instead of void, and rely on the 
return value optimization or, at worst, vector move.  It would be easier to use 
and it would let you initialize variables as you declare them, (which means you 
could use it in constructor intializers and you could declare the vector as 
const).  It also would eliminate the ambiguity of what happens if the `args` 
already contains some values (e.g., the caller might expect it to append rather 
than replace).


Comment at: source/API/SBCommandInterpreter.cpp:119
@@ +118,3 @@
+std::vector argv;
+command.GetArgumentVector(argv);
+bool ret = m_backend->DoExecute(debugger_sb, [0], sb_return);

Here's an example where, if GetArgumentVector actually returned the result, you 
could simplify the delcaration as:

const auto argv = command.GetArgumentVector();


Comment at: source/Host/common/FileSpec.cpp:251
@@ +250,3 @@
+
+  for (auto  : name_list) {
+matches.AppendString(name);

Should `name` be `const`?  As in:

for (const auto  : name_list) { ... }


Comment at: source/Interpreter/Args.cpp:46
@@ -44,3 +45,3 @@
 // strings in rhs.m_args. We need to copy the string list and update our
 // own m_argv appropriately.
 //--

I think your change eliminated the need for this comment.  If not, it at least 
needs an update.


Comment at: source/Interpreter/Args.cpp:275
@@ +274,3 @@
+  // new_argv.
+  // So make a copy and then swap it in.
+  std::vector new_args(new_argv.begin(), new_argv.end());

clang-format didn't do a great job wrapping the long lines in this comment.  
Can you fix it up?


Comment at: source/Interpreter/Args.cpp:427
@@ -512,1 +426,3 @@
+  GetArgumentVector(args);
+  args.insert(args.begin(), "dummy");
   while (1) {

This is an example where the ambiguity of whether GetArgumentVector appends or 
not could mislead the caller into doing the wrong this.  This looks plausible 
(and more efficient):

std::vector args;
args.push_back("dummy");
GetArgumentVector(args);

If GetArgumentVector returned the vector, then this ambiguity wouldn't exist.


https://reviews.llvm.org/D24952



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

Re: [Lldb-commits] [PATCH] D24919: Adding a RegisterContextMinidump_x86_64 converter

[Adrian McCarthy via lldb-commits]

amccarth added inline comments.


Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:1
@@ +1,2 @@
+//===-- Registers_x86_64.cpp *- C++ 
-*-===//
+//

Should match file name.


Comment at: 
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:31
@@ +30,3 @@
+// specified in the RegisterInfoInterface argument
+// This way we can reuse the already existing register contexts
+lldb::DataBufferSP lldb_private::minidump::ConvertMinidumpContextToRegIface(

It might be better to put this comment with the declaration in the header file, 
since it explains what to pass in and what it does.  Comments in .cpp files 
should contain implementation details that the callers don't necessarily need 
to know.


Comment at: 
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:47-48
@@ +46,4 @@
+
+  if (*context_flags & uint32_t(MinidumpContext_x86_64_Flags::Control)) {
+writeRegister(source_data, result_base, _info[lldb_cs_x86_64], 2);
+  }

dvlahovski wrote:
> If it is then when I do a `&` the result is an enum class of type 
> `MinidumpContext_x86_64_Flags`. And the compiler complains that this is not 
> convertible to bool
I think what Zach means is that you could locally define a uint32_t const, 
initialized with the value from the enum.  Then each if statement could use 
that constant without a cast.

Also, is this right?  `MinidumpContext_x86_x64_Flags::Control` has two bits 
set, so the condition will be true if either of them is set.  Is that the 
intended behavior?  Or should you be ensuring that they're both set like this:

const utin32_t ControlFlags = MinidumpContext_x86_64::Control;
if ((*context_flags & ControlFlags) == ControlFlags) {
  ...
}

?


Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h:1
@@ +1,2 @@
+//===-- Registers_x86_64.h --*- C++ 
-*-===//
+//

Please make this line match the file name.


Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:177
@@ +176,3 @@
+static void registerEqualToVal(const uint64_t val, uint8_t *reg_val) {
+  ASSERT_EQ(val, *(reinterpret_cast(reg_val)));
+}

+1 to Zach's idea.

Also, consider whether `EXPECT_EQ` is more appropriate than `ASSERT_EQ` for 
these.


https://reviews.llvm.org/D24919



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

Re: [Lldb-commits] [lldb] r282112 - Fix integer sign warning from r282105

[Adrian McCarthy via lldb-commits]

Thanks for fixing it.

On Wed, Sep 21, 2016 at 4:20 PM, Ed Maste  wrote:

> On 21 September 2016 at 21:38, Adrian McCarthy 
> wrote:
> > That fix doesn't look complete:
>
> Thanks, I've applied your fix in r282119, and sorry for being hasty
> with the original change.
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r282112 - Fix integer sign warning from r282105

[Adrian McCarthy via lldb-commits]

That fix doesn't look complete:

 for (size_t i = 0; i < column - 1 && i < src_line.length(); ++i)

`column` is an unsigned integral type, so doing subtraction from it can
lead to surprising results.  It's probably best to write it as:

  for (size_t i = 0; i  + 1 < column && i < src_line.length(); ++i)

That will more gracefully handle the case where column is 0.

Adrian.

On Wed, Sep 21, 2016 at 2:14 PM, Ed Maste via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: emaste
> Date: Wed Sep 21 16:14:31 2016
> New Revision: 282112
>
> URL: http://llvm.org/viewvc/llvm-project?rev=282112=rev
> Log:
> Fix integer sign warning from r282105
>
> Modified:
> lldb/trunk/source/Core/SourceManager.cpp
>
> Modified: lldb/trunk/source/Core/SourceManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/
> Core/SourceManager.cpp?rev=282112=282111=282112=diff
> 
> ==
> --- lldb/trunk/source/Core/SourceManager.cpp (original)
> +++ lldb/trunk/source/Core/SourceManager.cpp Wed Sep 21 16:14:31 2016
> @@ -172,7 +172,7 @@ size_t SourceManager::DisplaySourceLines
>  m_last_file_sp->GetLine(line, src_line);
>  return_value += s->Printf("\t");
>  // Insert a space for every non-tab character in the source line.
> -for (int i = 0; i < column - 1 && i < src_line.length(); ++i)
> +for (size_t i = 0; i < column - 1 && i < src_line.length(); ++i)
>return_value += s->PutChar(src_line[i] == '\t' ? '\t' : ' ');
>  // Now add the caret.
>  return_value += s->Printf("^\n");
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D24385: MinidumpParsing: pid, modules, exceptions

[Adrian McCarthy via lldb-commits]

> Even if it's length prefixed, the logic here basically just consumes the
entire buffer, which doesn't seem right

Yes, agreed.

On Fri, Sep 9, 2016 at 5:45 PM, Zachary Turner  wrote:

> Even if it's length prefixed, the logic here basically just consumes the
> entire buffer, which doesn't seem right
>
> On Fri, Sep 9, 2016 at 5:43 PM Adrian McCarthy 
> wrote:
>
>> amccarth added inline comments.
>>
>> 
>> Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:21
>> @@ +20,3 @@
>> +llvm::StringRef
>> +lldb_private::minidump::consumeString(llvm::ArrayRef ) {
>> +  return llvm::StringRef(reinterpret_cast(Buffer.data()),
>> 
>> zturner wrote:
>> > labath wrote:
>> > > This is not consistent with the consumeObject function, which also
>> drops the consumed bytes from the buffer.
>> > Is this logic correct?  A buffer may be arbitrarily large and have more
>> data in it besides the string.  Perhaps you need to search forward for a
>> null terminator, then only return that portion of the string, then drop
>> that many bytes (plus the null terminator) from the input buffer?
>> Minidump strings aren't zero-terminated.  They're counted (in UTF16 code
>> units).  So this would have to read the count and "consume" the appropriate
>> number of bytes.
>>
>> Oh, but this isn't used for minidump strings.  It looks like it's for
>> these Linux proc status fields.
>>
>>
>> https://reviews.llvm.org/D24385
>>
>>
>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D24385: MinidumpParsing: pid, modules, exceptions

[Adrian McCarthy via lldb-commits]

amccarth added inline comments.


Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:21
@@ +20,3 @@
+llvm::StringRef
+lldb_private::minidump::consumeString(llvm::ArrayRef ) {
+  return llvm::StringRef(reinterpret_cast(Buffer.data()),

zturner wrote:
> labath wrote:
> > This is not consistent with the consumeObject function, which also drops 
> > the consumed bytes from the buffer.
> Is this logic correct?  A buffer may be arbitrarily large and have more data 
> in it besides the string.  Perhaps you need to search forward for a null 
> terminator, then only return that portion of the string, then drop that many 
> bytes (plus the null terminator) from the input buffer?
Minidump strings aren't zero-terminated.  They're counted (in UTF16 code 
units).  So this would have to read the count and "consume" the appropriate 
number of bytes.

Oh, but this isn't used for minidump strings.  It looks like it's for these 
Linux proc status fields.


https://reviews.llvm.org/D24385



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

Re: [Lldb-commits] [PATCH] D23545: Minidump parsing

[Adrian McCarthy via lldb-commits]

amccarth added a comment.

LGTM.


https://reviews.llvm.org/D23545



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

Re: [Lldb-commits] [PATCH] D23545: Minidump parsing

[Adrian McCarthy via lldb-commits]

amccarth added a comment.

In https://reviews.llvm.org/D23545#519675, @dvlahovski wrote:

> In https://reviews.llvm.org/D23545#516808, @amccarth wrote:
>
> > Are we putting this code in the right place?  I wouldn't expect minidump 
> > parsing to fall under Plugins/Process.
> >
> > I assume the eventual intent is to turn the Windows-specific code into a 
> > user of your new code?  I look forward to seeing that happen.
>
>
> My idea was that the new plugin will live in Plugins/Process/minidump.
>  Do you have a suggestion where the parsing code to be, if not in the same 
> directory?


The parsing code will end up being used by multiple plugins--the new one you're 
building for Linux and the existing one that's Windows-specific.

What I thought would happen would be that you'd write the parsing code first, 
hook up the Windows-minidump plugin to use it (since the Windows-minidump 
plugin tests would help validate that your parsing is correct/complete) and 
then either add a new plugin for non-Windows.  That would argue for the 
minidump parsing to be in a common location that could be used by both plugins. 
 I don't have a good idea of where that would go.

Another approach is to make the Windows plugin platform agnostic once you 
replace the limited use of the minidump APIs with your own parsing code.

Food for thought.  No need to hold up this patch for that.  The code can be 
moved later if appropriate.



Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:22
@@ +21,3 @@
+llvm::Optional
+MinidumpHeader::Parse(llvm::ArrayRef )
+{

I find it amusing that all these `Parse` methods are in MinidumpTypes.cpp 
rather than MinidumpParser.cpp.  Just an observation--not a request to change 
it.


Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:133
@@ +132,3 @@
+// This matches the Linux kernel definitions from 
+enum MinidumpPCPUInformationARMElfHwCaps
+{

zturner wrote:
> Should this be `enum class`?
These look like individual bits that will be bitwise-ORed together, which is 
trickier to do with an enum class.

But you may still want to specify the underlying type, like `uint32_t`.


Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:258
@@ +257,3 @@
+llvm::support::ulittle16_t processor_level;
+llvm::support::ulittle16_t processor_revision;
+

dvlahovski wrote:
> The idea of this is to be sure that the compiler didn't align stuff 
> wrongly/not in the way we expect.
OK.


Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:260
@@ +259,3 @@
+
+uint8_t number_of_processors;
+uint8_t product_type;

I'll concede that the static_assert is useful.  Given that, showing the 
arithmetic explicitly will be helpful when one of these assertions trips, 
because you'll be able to see how it's supposed to correspond to the struct.


https://reviews.llvm.org/D23545



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

Re: [Lldb-commits] [PATCH] D23545: Minidump parsing

[Adrian McCarthy via lldb-commits]

amccarth added a comment.

Are we putting this code in the right place?  I wouldn't expect minidump 
parsing to fall under Plugins/Process.

I assume the eventual intent is to turn the Windows-specific code into a user 
of your new code?  I look forward to seeing that happen.



Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:13
@@ +12,3 @@
+#include "MinidumpParser.h"
+#include "MinidumpTypes.h"
+

Include MinidumpTypes.h first in MinidumpTypes.cpp.  This helps ensure that 
headers can stand alone.


Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:14
@@ +13,3 @@
+// C includes
+#include 
+

Maybe I'm missing it, but it doesn't seem like this header is using anything 
from `` (which should be under C++ includes).


Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:40
@@ +39,3 @@
+
+enum MinidumpStreamType
+{

// Reference:  
https://msdn.microsoft.com/en-us/library/windows/desktop/ms680394.aspx


Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:257
@@ +256,3 @@
+};
+const int MINIDUMP_SYSTEM_INFO_SIZE = 3 * 2 + 2 * 1 + 4 * 4 + sizeof(RVA) + 2 
* 2 + MINIDUMP_CPU_INFO_SIZE;
+static_assert(sizeof(MinidumpSystemInfo) == MINIDUMP_SYSTEM_INFO_SIZE, "sizeof 
MinidumpSystemInfo is not correct!");

Why do the arithmetic and static_assert?  Why not use 
`sizeof(MinidumpSystemInfo)`?


https://reviews.llvm.org/D23545



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

Re: [Lldb-commits] [PATCH] D22950: Centralize all calls to select() into a single class so we always call select properly

[Adrian McCarthy via lldb-commits]

I patched it in this morning, but it doesn't compile on Windows.  I was
able to make it work with a few tweaks at the top of SelectHelper.cpp:

// C Includes
#include 
#if defined(_WIN32)
#define NOMINMAX
#include 
#else
#include 
#endif

The key bits:  don't include  on Windows and #define NOMINMAX
on Windows to avoid stupid macros that conflict with std::min and std::max.

I'd probably put the #if under "// Other libraries and framework includes"
rather than "// C Includes", but that's a minor style nit.  Either way,
this change makes it compile on Windows.

Adrian.

On Mon, Aug 8, 2016 at 5:05 PM, Greg Clayton  wrote:

> clayborg updated this revision to Diff 67253.
> clayborg added a comment.
>
> Updated with all comments taken into account. Zach, please try this out
> and let me know if it compiles.
>
>
> https://reviews.llvm.org/D22950
>
> Files:
>   include/lldb/Utility/SelectHelper.h
>   lldb.xcodeproj/project.pbxproj
>   source/Host/common/Editline.cpp
>   source/Host/posix/ConnectionFileDescriptorPosix.cpp
>   source/Host/posix/PipePosix.cpp
>   source/Target/Process.cpp
>   source/Utility/SelectHelper.cpp
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D22352: Implement GetMemoryRegions() for Windows Minidumps and live processes.

[Adrian McCarthy via lldb-commits]

amccarth accepted this revision.
amccarth added a reviewer: amccarth.
amccarth added a comment.

LGTM.  Thanks!



Comment at: source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp:338
@@ -315,4 +337,3 @@
 // truncated.
-error.SetErrorString("address is not in a known range");
 return error;
 }

hhellyer wrote:
> amccarth wrote:
> > hhellyer wrote:
> > > Asking for an address outside a known range is not actually an error, you 
> > > just get back an unmapped range that specifies how far it is to the next 
> > > mapped range.
> > > The original discussion about that happened here: 
> > > https://reviews.llvm.org/D21751 - GetMemoryRegionInfo should only need to 
> > > return an error if it is unimplemented. (I should probably have 
> > > referenced that in the summary.)
> > Got it.  Thanks for the explanation!
> > 
> > Given that, should the condition on 284 ("the mini dump contains no memory 
> > range information") actually return an error or just an unmapped range?
> I thought the most likely cause of having no memory info would be not passing 
> the MiniDumpWithFullMemoryInfo flag to MiniDumpWriteDump will create a dump 
> without the MemoryInfoListStream present. Saying that memory range 
> information from a dump like that is "unsupported" seemed like the right 
> option. I think MiniDumpWithFullMemoryInfo is part of the default set of 
> flags used when you create a dump through task manager so most dumps should 
> have the stream.
> 
> I won't land the patch immediately, I'm happy to change it if you'd prefer it 
> to return an unmapped range. I think it would have to be one unmapped range 
> that ran from load_addr to LLDB_INVALID_ADDRESS to match the expected 
> behaviour.
Sounds like you've thought it through more thoroughly than I, and I agree with 
your conclusions, so I have no objections.  Thanks for the explanations.


https://reviews.llvm.org/D22352



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

Re: [Lldb-commits] [PATCH] D22352: Implement GetMemoryRegions() for Windows Minidumps and live processes.

[Adrian McCarthy via lldb-commits]

amccarth added a subscriber: amccarth.


Comment at: source/Plugins/Process/Windows/MiniDump/ProcessWinMiniDump.cpp:338
@@ -315,4 +337,3 @@
 // truncated.
-error.SetErrorString("address is not in a known range");
 return error;
 }

I'm not clear why you deleted the SetErrorString call.


https://reviews.llvm.org/D22352



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

[Lldb-commits] [lldb] r274277 - Fix for Windows builds.

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Thu Jun 30 15:55:50 2016
New Revision: 274277

URL: http://llvm.org/viewvc/llvm-project?rev=274277=rev
Log:
Fix for Windows builds.

Modified:
lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp

Modified: lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp?rev=274277=274276=274277=diff
==
--- lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/Android/AdbClient.cpp Thu Jun 30 
15:55:50 2016
@@ -29,6 +29,12 @@
 #include 
 #include 
 
+// On Windows, transitive dependencies pull in , which defines a
+// macro that clashes with a method name.
+#ifdef SendMessage
+#undef SendMessage
+#endif
+
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::platform_android;


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

Re: [Lldb-commits] [PATCH] D19943: XFail TestEnumTypes.py on Windows

[Adrian McCarthy via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL268574: XFail TestEnumTypes.py on Windows (authored by 
amccarth).

Changed prior to commit:
  http://reviews.llvm.org/D19943?vs=56216=56222#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D19943

Files:
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/main.c

Index: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py
@@ -19,6 +19,7 @@
 # Find the line number to break inside main().
 self.line = line_number('main.c', '// Set break point at this line.')
 
+@expectedFailAll(oslist=['windows'])  // derefing the null pointer "works" 
on Windows
 def test(self):
 """Test 'image lookup -t days' and check for correct display and enum 
value printing."""
 self.build()
@@ -53,24 +54,24 @@
'kNumDays',
'}'])
 
-enum_values = [ '-4', 
-'Monday', 
-'Tuesday', 
-'Wednesday', 
+enum_values = [ '-4',
+'Monday',
+'Tuesday',
+'Wednesday',
 'Thursday',
 'Friday',
 'Saturday',
 'Sunday',
 'kNumDays',
 '5'];
-   
  
+
 # Make sure a pointer to an anonymous enum type does crash LLDB and 
displays correctly using
 # frame variable and expression commands
 self.expect('frame variable f.op', DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs = ['ops *', 'f.op'], patterns = ['0x0+$'])
 self.expect('frame variable *f.op', DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs = ['ops', '*f.op', ''])
 self.expect('expr f.op', DATA_TYPES_DISPLAYED_CORRECTLY, substrs = 
['ops *', '$'], patterns = ['0x0+$'])
 self.expect('expr *f.op', DATA_TYPES_DISPLAYED_CORRECTLY, substrs = 
['error:'], error = True)
-
+
 bkpt = self.target().FindBreakpointByID(bkpt_id)
 for enum_value in enum_values:
 self.expect("frame variable day", 'check for valid enumeration 
value',
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/main.c
@@ -31,7 +31,7 @@
 };
 enum days day;
 struct foo f;
-   f.op = NULL;
+f.op = NULL;
 for (day = Monday - 1; day <= kNumDays + 1; day++)
 {
 printf("day as int is %i\n", (int)day); // Set break point at this 
line.


Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py
@@ -19,6 +19,7 @@
 # Find the line number to break inside main().
 self.line = line_number('main.c', '// Set break point at this line.')
 
+@expectedFailAll(oslist=['windows'])  // derefing the null pointer "works" on Windows
 def test(self):
 """Test 'image lookup -t days' and check for correct display and enum value printing."""
 self.build()
@@ -53,24 +54,24 @@
'kNumDays',
'}'])
 
-enum_values = [ '-4', 
-'Monday', 
-'Tuesday', 
-'Wednesday', 
+enum_values = [ '-4',
+'Monday',
+'Tuesday',
+'Wednesday',
 'Thursday',
 'Friday',
 'Saturday',
 'Sunday',
 'kNumDays',
 '5'];
- 
+
 # Make sure a pointer to an anonymous enum type does crash LLDB and displays correctly using
 # frame variable and expression commands
 self.expect('frame variable f.op', DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ['ops *', 'f.op'], patterns = ['0x0+$'])
 self.expect('frame variable *f.op', DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ['ops', 

[Lldb-commits] [lldb] r268574 - XFail TestEnumTypes.py on Windows

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Wed May  4 18:33:19 2016
New Revision: 268574

URL: http://llvm.org/viewvc/llvm-project?rev=268574=rev
Log:
XFail TestEnumTypes.py on Windows

Differential Revision: http://reviews.llvm.org/D19943

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py
lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/main.c

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py?rev=268574=268573=268574=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/TestEnumTypes.py 
Wed May  4 18:33:19 2016
@@ -19,6 +19,7 @@ class EnumTypesTestCase(TestBase):
 # Find the line number to break inside main().
 self.line = line_number('main.c', '// Set break point at this line.')
 
+@expectedFailAll(oslist=['windows'])  // derefing the null pointer "works" 
on Windows
 def test(self):
 """Test 'image lookup -t days' and check for correct display and enum 
value printing."""
 self.build()
@@ -53,24 +54,24 @@ class EnumTypesTestCase(TestBase):
'kNumDays',
'}'])
 
-enum_values = [ '-4', 
-'Monday', 
-'Tuesday', 
-'Wednesday', 
+enum_values = [ '-4',
+'Monday',
+'Tuesday',
+'Wednesday',
 'Thursday',
 'Friday',
 'Saturday',
 'Sunday',
 'kNumDays',
 '5'];
-   
  
+
 # Make sure a pointer to an anonymous enum type does crash LLDB and 
displays correctly using
 # frame variable and expression commands
 self.expect('frame variable f.op', DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs = ['ops *', 'f.op'], patterns = ['0x0+$'])
 self.expect('frame variable *f.op', DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs = ['ops', '*f.op', ''])
 self.expect('expr f.op', DATA_TYPES_DISPLAYED_CORRECTLY, substrs = 
['ops *', '$'], patterns = ['0x0+$'])
 self.expect('expr *f.op', DATA_TYPES_DISPLAYED_CORRECTLY, substrs = 
['error:'], error = True)
-
+
 bkpt = self.target().FindBreakpointByID(bkpt_id)
 for enum_value in enum_values:
 self.expect("frame variable day", 'check for valid enumeration 
value',

Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/main.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/main.c?rev=268574=268573=268574=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/main.c 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/enum_types/main.c Wed May  
4 18:33:19 2016
@@ -31,7 +31,7 @@ int main (int argc, char const *argv[])
 };
 enum days day;
 struct foo f;
-   f.op = NULL;
+f.op = NULL;
 for (day = Monday - 1; day <= kNumDays + 1; day++)
 {
 printf("day as int is %i\n", (int)day); // Set break point at this 
line.


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

Re: [Lldb-commits] [PATCH] D19606: XFail TestLambdas.py on Windows after fixing some of the problems

[Adrian McCarthy via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL268573: XFail TestLambdas.py on Windows after fixing some of 
the problems (authored by amccarth).

Changed prior to commit:
  http://reviews.llvm.org/D19606?vs=55263=56221#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D19606

Files:
  lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/TestLambdas.py
  lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp
  lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
  lldb/trunk/source/Expression/IRInterpreter.cpp

Index: lldb/trunk/source/Expression/IRInterpreter.cpp
===
--- lldb/trunk/source/Expression/IRInterpreter.cpp
+++ lldb/trunk/source/Expression/IRInterpreter.cpp
@@ -477,6 +477,7 @@
 static const char *memory_read_error= "Interpreter couldn't read from memory";
 static const char *infinite_loop_error  = "Interpreter ran for too many cycles";
 //static const char *bad_result_error = "Result of expression is in bad memory";
+static const char *too_many_functions_error = "Interpreter doesn't handle modules with multiple function bodies.";
 
 static bool
 CanResolveConstant (llvm::Constant *constant)
@@ -506,7 +507,7 @@
 Constant *base = dyn_cast(*op_cursor);
 if (!base)
 return false;
-
+
 return CanResolveConstant(base);
 }
 }
@@ -535,7 +536,13 @@
 if (fi->begin() != fi->end())
 {
 if (saw_function_with_body)
+{
+if (log)
+log->Printf("More than one function in the module has a body");
+error.SetErrorToGenericError();
+error.SetErrorString(too_many_functions_error);
 return false;
+}
 saw_function_with_body = true;
 }
 }
@@ -664,7 +671,7 @@
 return false;
 }
 }
-
+
 if (Constant *constant = llvm::dyn_cast(operand))
 {
 if (!CanResolveConstant(constant))
@@ -680,7 +687,8 @@
 
 }
 
-return true;}
+return true;
+}
 
 bool
 IRInterpreter::Interpret (llvm::Module ,
Index: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
@@ -186,7 +186,7 @@
 elif hasattr(decorators, '__call__'):
 tmp = decorators(tmp)
 return tmp
-
+
 
 def MakeInlineTest(__file, __globals, decorators=None):
 # Adjust the filename if it ends in .pyc.  We want filenames to
@@ -200,13 +200,17 @@
 InlineTest.mydir = TestBase.compute_mydir(__file)
 
 test_name, _ = os.path.splitext(file_basename)
-# Build the test case 
+# Build the test case
 test = type(test_name, (InlineTest,), {'using_dsym': None})
 test.name = test_name
 
-test.test_with_dsym = ApplyDecoratorsToFunction(test._InlineTest__test_with_dsym, decorators)
-test.test_with_dwarf = ApplyDecoratorsToFunction(test._InlineTest__test_with_dwarf, decorators)
-test.test_with_dwo = ApplyDecoratorsToFunction(test._InlineTest__test_with_dwo, decorators)
+target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
+if test_categories.is_supported_on_platform("dsym", target_platform):
+test.test_with_dsym = ApplyDecoratorsToFunction(test._InlineTest__test_with_dsym, decorators)
+if test_categories.is_supported_on_platform("dwarf", target_platform):
+test.test_with_dwarf = ApplyDecoratorsToFunction(test._InlineTest__test_with_dwarf, decorators)
+if test_categories.is_supported_on_platform("dwo", target_platform):
+test.test_with_dwo = ApplyDecoratorsToFunction(test._InlineTest__test_with_dwo, decorators)
 
 # Add the test case to the globals, and hide InlineTest
 __globals.update({test_name : test})
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp
@@ -11,7 +11,7 @@
 
 int main (int argc, char const *argv[])
 {
-printf("Stop here\n"); //% self.runCmd("expression auto $add = [](int a, int b) { return a + b };")
-   //% self.expect("expression $add(2,3)", substrs = ['= 5'])
-return 0; 
+printf("Stop here\n"); //% self.runCmd("expression auto $add = [](int a, int b) { return a + b; }")
+   //% self.expect("expression $add(2,3)", substrs = ['= 5'])
+return 0;
 }
Index: 

[Lldb-commits] [lldb] r268573 - XFail TestLambdas.py on Windows after fixing some of the problems

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Wed May  4 18:32:35 2016
New Revision: 268573

URL: http://llvm.org/viewvc/llvm-project?rev=268573=rev
Log:
XFail TestLambdas.py on Windows after fixing some of the problems

1. Fixed semicolon placement in the lambda in the test itself.

2. Fixed lldbinline tests in general so that we don't attempt tests on 
platforms that don't use the given type of debug info. (For example, no DWO 
tests on Windows.) This fixes one of the two failures on Windows. 
(TestLambdas.py was the only inline test that wasn't XFailed or skipped on 
Windows.)

3. Set the error string in IRInterpreter::CanInterpret so that the caller 
doesn't print (null) instead of an explanation. I don't entirely understand the 
error, so feel free to suggest a better wording.

4. XFailed the test on Windows. The interpreter won't evaluate the lambda 
because the module has multiple function bodies. I don't exactly understand why 
that's a problem for the interpreter nor why the problem arises only on Windows.

Differential Revision: http://reviews.llvm.org/D19606

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/TestLambdas.py
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp
lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
lldb/trunk/source/Expression/IRInterpreter.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/TestLambdas.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/TestLambdas.py?rev=268573=268572=268573=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/TestLambdas.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/TestLambdas.py 
Wed May  4 18:32:35 2016
@@ -1,4 +1,4 @@
 from lldbsuite.test import lldbinline
 from lldbsuite.test import decorators
 
-lldbinline.MakeInlineTest(__file__, globals(), [])
+lldbinline.MakeInlineTest(__file__, globals(), 
[lldbinline.expectedFailureAll(oslist=["windows"])])

Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp?rev=268573=268572=268573=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp Wed May 
 4 18:32:35 2016
@@ -11,7 +11,7 @@
 
 int main (int argc, char const *argv[])
 {
-printf("Stop here\n"); //% self.runCmd("expression auto $add = [](int 
a, int b) { return a + b };")
-   //% self.expect("expression $add(2,3)", substrs 
= ['= 5'])
-return 0; 
+printf("Stop here\n"); //% self.runCmd("expression auto $add = [](int a, 
int b) { return a + b; }")
+   //% self.expect("expression $add(2,3)", substrs = 
['= 5'])
+return 0;
 }

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py?rev=268573=268572=268573=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py Wed May  4 18:32:35 
2016
@@ -186,7 +186,7 @@ def ApplyDecoratorsToFunction(func, deco
 elif hasattr(decorators, '__call__'):
 tmp = decorators(tmp)
 return tmp
-
+
 
 def MakeInlineTest(__file, __globals, decorators=None):
 # Adjust the filename if it ends in .pyc.  We want filenames to
@@ -200,13 +200,17 @@ def MakeInlineTest(__file, __globals, de
 InlineTest.mydir = TestBase.compute_mydir(__file)
 
 test_name, _ = os.path.splitext(file_basename)
-# Build the test case 
+# Build the test case
 test = type(test_name, (InlineTest,), {'using_dsym': None})
 test.name = test_name
 
-test.test_with_dsym = 
ApplyDecoratorsToFunction(test._InlineTest__test_with_dsym, decorators)
-test.test_with_dwarf = 
ApplyDecoratorsToFunction(test._InlineTest__test_with_dwarf, decorators)
-test.test_with_dwo = 
ApplyDecoratorsToFunction(test._InlineTest__test_with_dwo, decorators)
+target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
+if test_categories.is_supported_on_platform("dsym", target_platform):
+test.test_with_dsym = 
ApplyDecoratorsToFunction(test._InlineTest__test_with_dsym, decorators)
+if test_categories.is_supported_on_platform("dwarf", target_platform):
+test.test_with_dwarf = 
ApplyDecoratorsToFunction(test._InlineTest__test_with_dwarf, decorators)
+if test_categories.is_supported_on_platform("dwo", target_platform):
+test.test_with_dwo = 

Re: [Lldb-commits] [PATCH] D19751: Fix TestEnumTypes.py for 32 bit platforms.

[Adrian McCarthy via lldb-commits]

amccarth added a comment.

The test expects `expr *f.op` to fail because f is a null pointer, but on 
Windows it yields `(ops) $2 = 0`.  I suspect this is a latent expression 
evaluation bug exposed by the new test, as it also happens for other types of 
pointers and not just pointers to enums.


Repository:
  rL LLVM

http://reviews.llvm.org/D19751



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

Re: [Lldb-commits] [PATCH] D19751: Fix TestEnumTypes.py for 32 bit platforms.

[Adrian McCarthy via lldb-commits]

amccarth added a subscriber: amccarth.
amccarth added a comment.

Chaoren:  Did this completely fix the test for you?  It's still failing for me 
on Windows, but for a reason not addressed here.


Repository:
  rL LLVM

http://reviews.llvm.org/D19751



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

Re: [Lldb-commits] [PATCH] D19606: XFail TestLambdas.py on Windows after fixing some of the problems

[Adrian McCarthy via lldb-commits]

amccarth added a comment.

Zach is hoping to enable tests on our Windows build bot this week, so I'd like 
to land this in the next day or two if possible.  If not, I can just xfail this 
test on Windows and postpone the other fixes until there's more time for a 
review.

Thanks.


http://reviews.llvm.org/D19606



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

Re: [Lldb-commits] [PATCH] D18017: Eliminate the TestStarted-XXX and TestFinished-XXX files from the test traces

[Adrian McCarthy via lldb-commits]

amccarth closed this revision.
amccarth added a comment.

This was submitted last month.  r263122


http://reviews.llvm.org/D18017



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

Re: [Lldb-commits] [PATCH] D19626: XFail TestIRInterpreter on Windows

[Adrian McCarthy via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL267800: XFail TestIRInterpreter on Windows (authored by 
amccarth).

Changed prior to commit:
  http://reviews.llvm.org/D19626?vs=55326=55333#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D19626

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py

Index: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
@@ -38,6 +38,7 @@
 self.runCmd("run", RUN_SUCCEEDED)
 
 @add_test_categories(['pyapi'])
+@expectedFailureAll(oslist=['windows'], bugnumber="21765")  # getpid() is 
POSIX, among other problems, see bug
 def test_ir_interpreter(self):
 self.build_and_run()
 


Index: lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
+++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
@@ -38,6 +38,7 @@
 self.runCmd("run", RUN_SUCCEEDED)
 
 @add_test_categories(['pyapi'])
+@expectedFailureAll(oslist=['windows'], bugnumber="21765")  # getpid() is POSIX, among other problems, see bug
 def test_ir_interpreter(self):
 self.build_and_run()
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r267800 - XFail TestIRInterpreter on Windows

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Wed Apr 27 16:53:19 2016
New Revision: 267800

URL: http://llvm.org/viewvc/llvm-project?rev=267800=rev
Log:
XFail TestIRInterpreter on Windows

There's an open bug with calling functions in the inferior.  And Windows 
doesn't have the POSIX function getpid().

Differential Revision: http://reviews.llvm.org/D19626

Modified:

lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py?rev=267800=267799=267800=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
 Wed Apr 27 16:53:19 2016
@@ -38,6 +38,7 @@ class IRInterpreterTestCase(TestBase):
 self.runCmd("run", RUN_SUCCEEDED)
 
 @add_test_categories(['pyapi'])
+@expectedFailureAll(oslist=['windows'], bugnumber="21765")  # getpid() is 
POSIX, among other problems, see bug
 def test_ir_interpreter(self):
 self.build_and_run()
 


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

[Lldb-commits] [PATCH] D19626: XFail TestIRInterpreter on Windows

[Adrian McCarthy via lldb-commits]

amccarth created this revision.
amccarth added a reviewer: spyffe.
amccarth added a subscriber: lldb-commits.

Test relies on a POSIX-only function `getpid()`, so the expression on Windows 
gets an undefined symbol.  If you substitute `_getpid()`, the interpreter 
complains that it uses an opcode it doesn't support.

http://reviews.llvm.org/D19626

Files:
  
packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py

Index: 
packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
===
--- 
packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
+++ 
packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
@@ -38,6 +38,7 @@
 self.runCmd("run", RUN_SUCCEEDED)
 
 @add_test_categories(['pyapi'])
+@expectedFailureAll(oslist=['windows'])  # depends on POSIX getpid (and 
ir-interpreter can run substitute _getpid)
 def test_ir_interpreter(self):
 self.build_and_run()
 


Index: packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
===
--- packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
+++ packages/Python/lldbsuite/test/expression_command/ir-interpreter/TestIRInterpreter.py
@@ -38,6 +38,7 @@
 self.runCmd("run", RUN_SUCCEEDED)
 
 @add_test_categories(['pyapi'])
+@expectedFailureAll(oslist=['windows'])  # depends on POSIX getpid (and ir-interpreter can run substitute _getpid)
 def test_ir_interpreter(self):
 self.build_and_run()
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D19606: XFail TestLambdas.py on Windows after fixing some of the problems

[Adrian McCarthy via lldb-commits]

amccarth created this revision.
amccarth added a reviewer: spyffe.
amccarth added a subscriber: lldb-commits.

1.  Fixed semicolon placement in the lambda in the test itself.

2.  Fixed lldbinline tests in general so that we don't attempt tests on 
platforms that don't use the given type of debug info.  (For example, no DWO 
tests on Windows.)  This fixes one of the two failures on Windows.  
(TestLambdas.py was the only inline test that wasn't XFailed or skipped on 
Windows.)

3.  Set the error string in `IRInterpreter::CanInterpret` so that the caller 
doesn't print `(null)` instead of an explanation.  I don't entirely understand 
the error, so feel free to suggest a better wording.

4.  XFailed the test on Windows.  The interpreter won't evaluate the lambda 
because the module has multiple function bodies.  I don't exactly understand 
why that's a problem for the interpreter nor why the problem arises only on 
Windows.




http://reviews.llvm.org/D19606

Files:
  packages/Python/lldbsuite/test/lang/cpp/lambdas/TestLambdas.py
  packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp
  packages/Python/lldbsuite/test/lldbinline.py
  source/Expression/IRInterpreter.cpp

Index: source/Expression/IRInterpreter.cpp
===
--- source/Expression/IRInterpreter.cpp
+++ source/Expression/IRInterpreter.cpp
@@ -477,6 +477,7 @@
 static const char *memory_read_error= "Interpreter couldn't read from memory";
 static const char *infinite_loop_error  = "Interpreter ran for too many cycles";
 //static const char *bad_result_error = "Result of expression is in bad memory";
+static const char *too_many_functions_error = "Interpreter doesn't handle modules with multiple function bodies.";
 
 static bool
 CanResolveConstant (llvm::Constant *constant)
@@ -506,7 +507,7 @@
 Constant *base = dyn_cast(*op_cursor);
 if (!base)
 return false;
-
+
 return CanResolveConstant(base);
 }
 }
@@ -535,7 +536,13 @@
 if (fi->begin() != fi->end())
 {
 if (saw_function_with_body)
+{
+if (log)
+log->Printf("More than one function in the module has a body");
+error.SetErrorToGenericError();
+error.SetErrorString(too_many_functions_error);
 return false;
+}
 saw_function_with_body = true;
 }
 }
@@ -664,7 +671,7 @@
 return false;
 }
 }
-
+
 if (Constant *constant = llvm::dyn_cast(operand))
 {
 if (!CanResolveConstant(constant))
@@ -680,7 +687,8 @@
 
 }
 
-return true;}
+return true;
+}
 
 bool
 IRInterpreter::Interpret (llvm::Module ,
Index: packages/Python/lldbsuite/test/lldbinline.py
===
--- packages/Python/lldbsuite/test/lldbinline.py
+++ packages/Python/lldbsuite/test/lldbinline.py
@@ -186,7 +186,7 @@
 elif hasattr(decorators, '__call__'):
 tmp = decorators(tmp)
 return tmp
-
+
 
 def MakeInlineTest(__file, __globals, decorators=None):
 # Adjust the filename if it ends in .pyc.  We want filenames to
@@ -200,13 +200,17 @@
 InlineTest.mydir = TestBase.compute_mydir(__file)
 
 test_name, _ = os.path.splitext(file_basename)
-# Build the test case 
+# Build the test case
 test = type(test_name, (InlineTest,), {'using_dsym': None})
 test.name = test_name
 
-test.test_with_dsym = ApplyDecoratorsToFunction(test._InlineTest__test_with_dsym, decorators)
-test.test_with_dwarf = ApplyDecoratorsToFunction(test._InlineTest__test_with_dwarf, decorators)
-test.test_with_dwo = ApplyDecoratorsToFunction(test._InlineTest__test_with_dwo, decorators)
+target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
+if test_categories.is_supported_on_platform("dsym", target_platform):
+test.test_with_dsym = ApplyDecoratorsToFunction(test._InlineTest__test_with_dsym, decorators)
+if test_categories.is_supported_on_platform("dwarf", target_platform):
+test.test_with_dwarf = ApplyDecoratorsToFunction(test._InlineTest__test_with_dwarf, decorators)
+if test_categories.is_supported_on_platform("dwo", target_platform):
+test.test_with_dwo = ApplyDecoratorsToFunction(test._InlineTest__test_with_dwo, decorators)
 
 # Add the test case to the globals, and hide InlineTest
 __globals.update({test_name : test})
Index: packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp
===
--- packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp
+++ packages/Python/lldbsuite/test/lang/cpp/lambdas/main.cpp
@@ -11,7 

Re: [Lldb-commits] [PATCH] D19548: Fix TestRegisterVariables.py on Windows

[Adrian McCarthy via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL267616: Fix TestRegisterVariables.py on Windows (authored by 
amccarth).

Changed prior to commit:
  http://reviews.llvm.org/D19548?vs=55050=55112#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D19548

Files:
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c

Index: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c
@@ -5,20 +5,20 @@
   int m2;
 };
 
-void f1(int a, struct bar *b) __attribute__ ((noinline));
+void f1(int a, struct bar *b) __attribute__((noinline)) 
__attribute__((regparm(2)));
 void f1(int a, struct bar *b)
 {
   b->m2 = b->m1 + a; // set breakpoint here
 }
 
-void f2(struct bar *b) __attribute__ ((noinline));
+void f2(struct bar *b) __attribute__((noinline)) __attribute__((regparm(1)));
 void f2(struct bar *b)
 {
   int c = b->m2;
   printf("%d\n", c); // set breakpoint here
 }
 
-float f3() __attribute__ ((noinline));
+float f3() __attribute__((noinline));
 float f3() {
   return 3.14f;
 }


Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c
@@ -5,20 +5,20 @@
   int m2;
 };
 
-void f1(int a, struct bar *b) __attribute__ ((noinline));
+void f1(int a, struct bar *b) __attribute__((noinline)) __attribute__((regparm(2)));
 void f1(int a, struct bar *b)
 {
   b->m2 = b->m1 + a; // set breakpoint here
 }
 
-void f2(struct bar *b) __attribute__ ((noinline));
+void f2(struct bar *b) __attribute__((noinline)) __attribute__((regparm(1)));
 void f2(struct bar *b)
 {
   int c = b->m2;
   printf("%d\n", c); // set breakpoint here
 }
 
-float f3() __attribute__ ((noinline));
+float f3() __attribute__((noinline));
 float f3() {
   return 3.14f;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r267616 - Fix TestRegisterVariables.py on Windows

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Tue Apr 26 17:25:40 2016
New Revision: 267616

URL: http://llvm.org/viewvc/llvm-project?rev=267616=rev
Log:
Fix TestRegisterVariables.py on Windows

Use __attribute__((regparm(x))) to ensure the compiler enregisters at least 
some arguments when calling functions.

Differential Revision: http://reviews.llvm.org/D19548

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c?rev=267616=267615=267616=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/register_variables/test.c 
Tue Apr 26 17:25:40 2016
@@ -5,20 +5,20 @@ struct bar {
   int m2;
 };
 
-void f1(int a, struct bar *b) __attribute__ ((noinline));
+void f1(int a, struct bar *b) __attribute__((noinline)) 
__attribute__((regparm(2)));
 void f1(int a, struct bar *b)
 {
   b->m2 = b->m1 + a; // set breakpoint here
 }
 
-void f2(struct bar *b) __attribute__ ((noinline));
+void f2(struct bar *b) __attribute__((noinline)) __attribute__((regparm(1)));
 void f2(struct bar *b)
 {
   int c = b->m2;
   printf("%d\n", c); // set breakpoint here
 }
 
-float f3() __attribute__ ((noinline));
+float f3() __attribute__((noinline));
 float f3() {
   return 3.14f;
 }


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

[Lldb-commits] [PATCH] D19548: Fix TestRegisterVariables.py on Windows

[Adrian McCarthy via lldb-commits]

amccarth created this revision.
amccarth added a reviewer: tfiala.
amccarth added a subscriber: lldb-commits.
Herald added a subscriber: aemerson.

32-bit Windows calling conventions, by default, don't pass arguments in 
registers, but this test expects at least one of them to be.

By adding __attribute__((regparm(x))) to the functions, we can get the compiler 
to enregister some of the parameters, and the test passes.

I don't think this should have a negative impact on other platforms, but I 
haven't tried.

http://reviews.llvm.org/D19548

Files:
  packages/Python/lldbsuite/test/lang/c/register_variables/test.c

Index: packages/Python/lldbsuite/test/lang/c/register_variables/test.c
===
--- packages/Python/lldbsuite/test/lang/c/register_variables/test.c
+++ packages/Python/lldbsuite/test/lang/c/register_variables/test.c
@@ -5,20 +5,20 @@
   int m2;
 };
 
-void f1(int a, struct bar *b) __attribute__ ((noinline));
+void f1(int a, struct bar *b) __attribute__((noinline)) 
__attribute__((regparm(2)));
 void f1(int a, struct bar *b)
 {
   b->m2 = b->m1 + a; // set breakpoint here
 }
 
-void f2(struct bar *b) __attribute__ ((noinline));
+void f2(struct bar *b) __attribute__((noinline)) __attribute__((regparm(1)));
 void f2(struct bar *b)
 {
   int c = b->m2;
   printf("%d\n", c); // set breakpoint here
 }
 
-float f3() __attribute__ ((noinline));
+float f3() __attribute__((noinline));
 float f3() {
   return 3.14f;
 }


Index: packages/Python/lldbsuite/test/lang/c/register_variables/test.c
===
--- packages/Python/lldbsuite/test/lang/c/register_variables/test.c
+++ packages/Python/lldbsuite/test/lang/c/register_variables/test.c
@@ -5,20 +5,20 @@
   int m2;
 };
 
-void f1(int a, struct bar *b) __attribute__ ((noinline));
+void f1(int a, struct bar *b) __attribute__((noinline)) __attribute__((regparm(2)));
 void f1(int a, struct bar *b)
 {
   b->m2 = b->m1 + a; // set breakpoint here
 }
 
-void f2(struct bar *b) __attribute__ ((noinline));
+void f2(struct bar *b) __attribute__((noinline)) __attribute__((regparm(1)));
 void f2(struct bar *b)
 {
   int c = b->m2;
   printf("%d\n", c); // set breakpoint here
 }
 
-float f3() __attribute__ ((noinline));
+float f3() __attribute__((noinline));
 float f3() {
   return 3.14f;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D19510: Fix send and receive of ACK byte in test infrastructure for Python 3.5

[Adrian McCarthy via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL267562: Fix send and receive of ACK byte in test 
infrastructure for Python 3.5 (authored by amccarth).

Changed prior to commit:
  http://reviews.llvm.org/D19510?vs=54938=55005#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D19510

Files:
  lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py
  lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py

Index: lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py
===
--- lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py
+++ lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py
@@ -59,8 +59,7 @@
 # the initiators of the socket to await this to ensure
 # that this end is up and running (and therefore already
 # into the async map).
-ack_bytes = bytearray()
-ack_bytes.append(chr(42))
+ack_bytes = b'*'
 file_object.send(ack_bytes)
 
 def deserialize_payload(self):
Index: lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py
===
--- lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py
+++ lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py
@@ -40,9 +40,6 @@
 self.cleanup_func = cleanup_func
 
 
-SOCKET_ACK_BYTE_VALUE = b'*'  # ASCII for chr(42)
-
-
 def create_results_formatter(config):
 """Sets up a test results formatter.
 
@@ -78,7 +75,7 @@
 # listener socket gets spun up; otherwise,
 # we lose the test result info.
 read_bytes = sock.recv(1)
-if read_bytes is None or (len(read_bytes) < 1) or (read_bytes[0] != 
SOCKET_ACK_BYTE_VALUE):
+if read_bytes is None or (len(read_bytes) < 1) or (read_bytes != b'*'):
 raise Exception("listening socket did not respond with ack byte: 
response={}".format(read_bytes))
 
 return sock, lambda: socket_closer(sock)


Index: lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py
===
--- lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py
+++ lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py
@@ -59,8 +59,7 @@
 # the initiators of the socket to await this to ensure
 # that this end is up and running (and therefore already
 # into the async map).
-ack_bytes = bytearray()
-ack_bytes.append(chr(42))
+ack_bytes = b'*'
 file_object.send(ack_bytes)
 
 def deserialize_payload(self):
Index: lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py
===
--- lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py
+++ lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py
@@ -40,9 +40,6 @@
 self.cleanup_func = cleanup_func
 
 
-SOCKET_ACK_BYTE_VALUE = b'*'  # ASCII for chr(42)
-
-
 def create_results_formatter(config):
 """Sets up a test results formatter.
 
@@ -78,7 +75,7 @@
 # listener socket gets spun up; otherwise,
 # we lose the test result info.
 read_bytes = sock.recv(1)
-if read_bytes is None or (len(read_bytes) < 1) or (read_bytes[0] != SOCKET_ACK_BYTE_VALUE):
+if read_bytes is None or (len(read_bytes) < 1) or (read_bytes != b'*'):
 raise Exception("listening socket did not respond with ack byte: response={}".format(read_bytes))
 
 return sock, lambda: socket_closer(sock)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r267562 - Fix send and receive of ACK byte in test infrastructure for Python 3.5

[Adrian McCarthy via lldb-commits]

Author: amccarth
Date: Tue Apr 26 10:15:29 2016
New Revision: 267562

URL: http://llvm.org/viewvc/llvm-project?rev=267562=rev
Log:
Fix send and receive of ACK byte in test infrastructure for Python 3.5

Python 3.5 is pickier about the distinction between chars and bytes (and 
strings and bytearrays) than Python 2.7.

Differential Revision: http://reviews.llvm.org/D19510

Modified:
lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py
lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py

Modified: lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py?rev=267562=267561=267562=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test_event/dotest_channels.py Tue Apr 
26 10:15:29 2016
@@ -59,8 +59,7 @@ class UnpicklingForwardingReaderChannel(
 # the initiators of the socket to await this to ensure
 # that this end is up and running (and therefore already
 # into the async map).
-ack_bytes = bytearray()
-ack_bytes.append(chr(42))
+ack_bytes = b'*'
 file_object.send(ack_bytes)
 
 def deserialize_payload(self):

Modified: lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py?rev=267562=267561=267562=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test_event/formatter/__init__.py Tue 
Apr 26 10:15:29 2016
@@ -40,9 +40,6 @@ class CreatedFormatter(object):
 self.cleanup_func = cleanup_func
 
 
-SOCKET_ACK_BYTE_VALUE = b'*'  # ASCII for chr(42)
-
-
 def create_results_formatter(config):
 """Sets up a test results formatter.
 
@@ -78,7 +75,7 @@ def create_results_formatter(config):
 # listener socket gets spun up; otherwise,
 # we lose the test result info.
 read_bytes = sock.recv(1)
-if read_bytes is None or (len(read_bytes) < 1) or (read_bytes[0] != 
SOCKET_ACK_BYTE_VALUE):
+if read_bytes is None or (len(read_bytes) < 1) or (read_bytes != b'*'):
 raise Exception("listening socket did not respond with ack byte: 
response={}".format(read_bytes))
 
 return sock, lambda: socket_closer(sock)


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

[Lldb-commits] [PATCH] D19510: Fix send and receive of ACK byte in test infrastructure for Python 3.5

[Adrian McCarthy via lldb-commits]

amccarth created this revision.
amccarth added a reviewer: tfiala.
amccarth added a subscriber: lldb-commits.

Python 3.5 is pickier about the distinction between chars and bytes (and 
strings and bytearrays) than Python 2.7.

The call to ack_bytes.append(chr(42)) was causing a type error to be thrown 
(during init, so we didn't see a proper stack trace).

I've used the b'*' syntax which creates a bytearray in Python 3 and a plain 
string in Python 2.  I believe this should work in Python 2, but I'm not in a 
position to test that.  If you want to try patching this in to your build to be 
sure, it would be appreciated.

http://reviews.llvm.org/D19510

Files:
  packages/Python/lldbsuite/test_event/dotest_channels.py
  packages/Python/lldbsuite/test_event/formatter/__init__.py

Index: packages/Python/lldbsuite/test_event/formatter/__init__.py
===
--- packages/Python/lldbsuite/test_event/formatter/__init__.py
+++ packages/Python/lldbsuite/test_event/formatter/__init__.py
@@ -40,9 +40,6 @@
 self.cleanup_func = cleanup_func
 
 
-SOCKET_ACK_BYTE_VALUE = b'*'  # ASCII for chr(42)
-
-
 def create_results_formatter(config):
 """Sets up a test results formatter.
 
@@ -78,7 +75,7 @@
 # listener socket gets spun up; otherwise,
 # we lose the test result info.
 read_bytes = sock.recv(1)
-if read_bytes is None or (len(read_bytes) < 1) or (read_bytes[0] != 
SOCKET_ACK_BYTE_VALUE):
+if read_bytes is None or (len(read_bytes) < 1) or (read_bytes != b'*'):
 raise Exception("listening socket did not respond with ack byte: 
response={}".format(read_bytes))
 
 return sock, lambda: socket_closer(sock)
Index: packages/Python/lldbsuite/test_event/dotest_channels.py
===
--- packages/Python/lldbsuite/test_event/dotest_channels.py
+++ packages/Python/lldbsuite/test_event/dotest_channels.py
@@ -59,8 +59,7 @@
 # the initiators of the socket to await this to ensure
 # that this end is up and running (and therefore already
 # into the async map).
-ack_bytes = bytearray()
-ack_bytes.append(chr(42))
+ack_bytes = b'*'
 file_object.send(ack_bytes)
 
 def deserialize_payload(self):


Index: packages/Python/lldbsuite/test_event/formatter/__init__.py
===
--- packages/Python/lldbsuite/test_event/formatter/__init__.py
+++ packages/Python/lldbsuite/test_event/formatter/__init__.py
@@ -40,9 +40,6 @@
 self.cleanup_func = cleanup_func
 
 
-SOCKET_ACK_BYTE_VALUE = b'*'  # ASCII for chr(42)
-
-
 def create_results_formatter(config):
 """Sets up a test results formatter.
 
@@ -78,7 +75,7 @@
 # listener socket gets spun up; otherwise,
 # we lose the test result info.
 read_bytes = sock.recv(1)
-if read_bytes is None or (len(read_bytes) < 1) or (read_bytes[0] != SOCKET_ACK_BYTE_VALUE):
+if read_bytes is None or (len(read_bytes) < 1) or (read_bytes != b'*'):
 raise Exception("listening socket did not respond with ack byte: response={}".format(read_bytes))
 
 return sock, lambda: socket_closer(sock)
Index: packages/Python/lldbsuite/test_event/dotest_channels.py
===
--- packages/Python/lldbsuite/test_event/dotest_channels.py
+++ packages/Python/lldbsuite/test_event/dotest_channels.py
@@ -59,8 +59,7 @@
 # the initiators of the socket to await this to ensure
 # that this end is up and running (and therefore already
 # into the async map).
-ack_bytes = bytearray()
-ack_bytes.append(chr(42))
+ack_bytes = b'*'
 file_object.send(ack_bytes)
 
 def deserialize_payload(self):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D19214: fix a race is the LLDB test suite results collection

[Adrian McCarthy via lldb-commits]

amccarth added a comment.

We already limit threads on Windows to work around other problems.  
Technically, we force Windows to always use the multiprocessing-pool instead of 
the threading-pool, which I think has the effect of limiting the number of 
threads.

I suspect that the first several invocations are stuck waiting "forever" for 
the new ACK byte (perhaps because of buffering?) and that causes the rest of 
the invocations to fail.  I'm still investigating.

Having been broken for a week, it seems we've already collected 30-40 newly 
broken tests on Windows. :-(


http://reviews.llvm.org/D19214



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

[Lldb-commits] [PATCH] D19489: Use llvm_unreachable to quiet a VC++ compiler warning.

[Adrian McCarthy via lldb-commits]

amccarth created this revision.
amccarth added a reviewer: spyffe.
amccarth added a subscriber: lldb-commits.

http://reviews.llvm.org/D19489

Files:
  source/Expression/DiagnosticManager.cpp

Index: source/Expression/DiagnosticManager.cpp
===
--- source/Expression/DiagnosticManager.cpp
+++ source/Expression/DiagnosticManager.cpp
@@ -8,6 +8,9 @@
 
//===--===//
 
 #include "lldb/Expression/DiagnosticManager.h"
+
+#include "llvm/Support/ErrorHandling.h"
+
 #include "lldb/Core/Log.h"
 #include "lldb/Core/StreamString.h"
 
@@ -45,6 +48,7 @@
 case lldb_private::eDiagnosticSeverityRemark:
 return "";
 }
+llvm_unreachable("switch needs another case for DiagnosticSeverity enum");
 }
 
 std::string


Index: source/Expression/DiagnosticManager.cpp
===
--- source/Expression/DiagnosticManager.cpp
+++ source/Expression/DiagnosticManager.cpp
@@ -8,6 +8,9 @@
 //===--===//
 
 #include "lldb/Expression/DiagnosticManager.h"
+
+#include "llvm/Support/ErrorHandling.h"
+
 #include "lldb/Core/Log.h"
 #include "lldb/Core/StreamString.h"
 
@@ -45,6 +48,7 @@
 case lldb_private::eDiagnosticSeverityRemark:
 return "";
 }
+llvm_unreachable("switch needs another case for DiagnosticSeverity enum");
 }
 
 std::string
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D19214: fix a race is the LLDB test suite results collection

[Adrian McCarthy via lldb-commits]

amccarth added a subscriber: amccarth.
amccarth added a comment.

FYI:  This patch seems to have broken all LLDB testing on Windows.  I'll 
investigate why.

Lots of the socket calls now result in stack traces like this:

  Traceback (most recent call last):
File "C:\python_35\lib\multiprocessing\pool.py", line 119, in worker
  result = (True, func(*args, **kwds))
File "C:\python_35\lib\multiprocessing\pool.py", line 44, in mapstar
  return list(map(*args))
File "D:\src\llvm\llvm\tools\lldb\packages\Python\lldbsuite\test\dosep.py", 
line 495, in process_dir_worker_multiprocessing_pool
  return process_dir(*args)
File "D:\src\llvm\llvm\tools\lldb\packages\Python\lldbsuite\test\dosep.py", 
line 442, in process_dir
  command, timeout, base_name, inferior_pid_events, full_test_path))
File "D:\src\llvm\llvm\tools\lldb\packages\Python\lldbsuite\test\dosep.py", 
line 417, in call_with_timeout
  test_filename)
File "D:\src\llvm\llvm\tools\lldb\packages\Python\lldbsuite\test\dosep.py", 
line 372, in send_inferior_post_run_events
  send_events_to_collector(post_events, command)
File "D:\src\llvm\llvm\tools\lldb\packages\Python\lldbsuite\test\dosep.py", 
line 308, in send_events_to_collector
  formatter_spec = result_formatter.create_results_formatter(config)
File 
"D:\src\llvm\llvm\tools\lldb\packages\Python\lldbsuite\test\result_formatter.py",
 line 112, in create_results_formatter
  results_file_object, cleanup_func = create_socket(config.port)
File 
"D:\src\llvm\llvm\tools\lldb\packages\Python\lldbsuite\test\result_formatter.py",
 line 78, in create_socket
  sock.connect(("localhost", port))
  ConnectionRefusedError: [WinError 10061] No connection could be made because 
the target machine actively refused it

The changes to result_formatter.py are after the problematic line, so the root 
cause it not immediately obvious.  I have a guess that we're trying to open 
more sockets than before and are therefore exceeding a handle limit imposed by 
Python's use of ancient C RTL code.


http://reviews.llvm.org/D19214



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