date:20191030

[Lldb-commits] [PATCH] D69612: [lldb-vscod] fix build with NDEBUG on windows

2019-10-30 Thread Peiyuan Song via Phabricator via lldb-commits

SquallATF created this revision.
SquallATF added a reviewer: mstorsjo.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

_setmode in assert will not run when build with NDEBUG


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69612

Files:
  lldb/tools/lldb-vscode/VSCode.cpp


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -44,8 +44,10 @@
 // Windows opens stdout and stdin in text mode which converts \n to 13,10
 // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
 // fixes this.
-  assert(_setmode(fileno(stdout), _O_BINARY));
-  assert(_setmode(fileno(stdin), _O_BINARY));
+  int result = _setmode(fileno(stdout), _O_BINARY);
+  assert(result);
+  result = _setmode(fileno(stdin), _O_BINARY);
+  assert(result);
 #endif
   if (log_file_path)
 log.reset(new std::ofstream(log_file_path));
@@ -301,4 +303,3 @@
 }
 
 } // namespace lldb_vscode
-


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -44,8 +44,10 @@
 // Windows opens stdout and stdin in text mode which converts \n to 13,10
 // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
 // fixes this.
-  assert(_setmode(fileno(stdout), _O_BINARY));
-  assert(_setmode(fileno(stdin), _O_BINARY));
+  int result = _setmode(fileno(stdout), _O_BINARY);
+  assert(result);
+  result = _setmode(fileno(stdin), _O_BINARY);
+  assert(result);
 #endif
   if (log_file_path)
 log.reset(new std::ofstream(log_file_path));
@@ -301,4 +303,3 @@
 }
 
 } // namespace lldb_vscode
-
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69612: [lldb-vscod] fix build with NDEBUG on windows

2019-10-30 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo added reviewers: labath, amccarth.
mstorsjo added a comment.

Good catch, thanks! But would we need to add something like `(void)result;` to 
silence potential compiler warnings about an unused (or set but not read) 
variable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69612



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

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added reviewers: labath, mgorny, sgraenitz.
labath added a comment.

This looks reasonable to me. I'm just wondering, now that these are separate 
targets, I guess that means they can be run in random order, right? Will that 
cause any problems, for instance when creating a package and its subpackage 
(formatters and formatters/cpp maybe)?




Comment at: lldb/CMakeLists.txt:133
+  add_copy_file_target(lldb_python_init
+FILES  "${lldb_scripts_dir}/lldb.py"
+DEST_DIR   "${lldb_python_build_path}"

Would it be possible to just make the swig step place this file into the 
correct place directly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69589



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

[Lldb-commits] [PATCH] D69503: [LLDB] [Windows] Don't crash if the debugged process is unable to start up

2019-10-30 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo added a comment.

Yes, this definitely is a race condition. Without any changes, I triggered this 
condition on arm64, but not on x86_64.

The issue is in the subclass ProcessWindows, where OnExitProcess does this:

  void ProcessWindows::OnDebuggerError(const Status &error, uint32_t type) {
// [simplified]
if (m_session_data->m_initial_stop_received) {
} else {
  // If we haven't actually launched the process yet, this was an error
  // launching the process.  Set the internal error and signal the initial
  // stop event so that the DoLaunch method wakes up and returns a failure.
  m_session_data->m_launch_error = error;
  ::SetEvent(m_session_data->m_initial_stop_event);
  return;
}
  }   
  
  void ProcessWindows::OnExitProcess(uint32_t exit_code) {
// [irrelevant bits excluded]
  
// If the process exits before any initial stop then notify the debugger
// of the error otherwise WaitForDebuggerConnection() will be blocked.
// An example of this issue is when a process fails to load a dependent DLL.
if (m_session_data && !m_session_data->m_initial_stop_received) {
  Status error(exit_code, eErrorTypeWin32);
  OnDebuggerError(error, 0);
}
  
// Reset the session.
m_session_data.reset();
  }

So `ProcessWindows::OnExitProcess` signals to 
`ProcessDebugger::WaitForDebuggerConnection` to proceed, and then 
`ProcessWindows::OnExitProcess` resets `m_session_data`, which 
`WaitForDebuggerConnection` starts inspecting.

What's the correct course of action here? Remove the `m_session_data.reset()` 
from ProcessWindows::OnExitProcess, concluding that it's up to some other class 
to clear `m_session_data` in this case?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69503



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

[Lldb-commits] [PATCH] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

2019-10-30 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo added a comment.

In D69031#1714143 , @labath wrote:

> In D69031#1714107 , @mstorsjo wrote:
>
> > In D69031#1713900 , @aprantl wrote:
> >
> > > Check out  `lldb/test/Shell/helper/toolchain.py`, you probably need to 
> > > filter out some options for clang_cl specifically.
> >
> >
> > Yeah, I later found that. The issue is that it's passed to usr_clang in i 
> > llvm/utils/lit/lit/llvm/config.py, which sets all the provided additional 
> > flags on both %clang, %clangxx, %clang_cc1 and %clang_cl.
> >
> > Maybe the additional_flags param needs to be split, into one common for 
> > all, one for gcc-style driver, one for clang_cl, and maybe also a separate 
> > one for cc1 (where not all driver level flags may fit either)?
>
>
> Actually, it seems to be that these arguments should not be added to the 
> command line by default. All of the existing tests that do "%clang -target 
> whatever" don't need the arguments, and they "only" work because the 
> arguments do no harm because the tests don't do anything which would depend 
> on them. I think we should leave `additional_flags` argument mostly empty, 
> and instead have a separate way of invoking clang when building for the host. 
> Either %clang_host (achievable with a recursive substitution %clang_host -> 
> %clang ), or using something like %clang %host_cflags.
>
> I can have a stab at that, if you like, but I'm not sure I'll get around to 
> that before the llvm conference is over...


Do you have time to look into this now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69031



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

[Lldb-commits] [PATCH] D69612: [lldb-vscod] fix build with NDEBUG on windows

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

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

LGTM, modulo the `(void) result` stuff. Do you need someone to commit this for 
you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69612



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

[Lldb-commits] [PATCH] D69532: [LLDB][PythonFile] fix dangerous borrow semantics on python2

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

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

In D69532#1725772 , @lawrence_danna 
wrote:

> > It shouldn't be really needed -- if everything goes through the same FILE* 
> > object, the C library will make sure the writes are available to anyone who 
> > tries to read through that object.
> >  I don't buy the argument that holding onto a FD-backed object after the FD 
> > has been closed is somehow "safer" than holding onto a FILE*. They both 
> > produce undefined behavior,
>
> But this is a fundamental difference between python and C++.   In C++ there's 
> only one level of UB.   If you do something illegal, your program can crash, 
> demons come out of your nose, whatever.   In python, unless you're using 
> something like ctypes, UB means your program misbehaves, but should never 
> mean that the interpreter crashes.   An interpreter crash in python is not 
> the moral equivalent of a segfault in C++, it's the equivalent of a kernel 
> panic.   If unprivileged C++ code crashes the kernel, that's a bug in the 
> kernel, whether UB was involved or not.   If a non-ctypes python program 
> crashes the interpreter, that's a bug in Python or some loadable module, no 
> matter how incorrect the python program was.

Yes, I get that. What I was trying to say that using a fd-based python file 
after it has been closed introduces exactly the same level of UB as for a 
FILE*-based one. I mean, for all you know, the fd might be recycled to point to 
/dev/sda, and writing to it can make your whole system unbootable.

The difference was that in the FILE* case the mere act of closing constituted a 
"use", while for a fd file we just happily did nothing. Not fflushing makes 
this aspect of the two implementations equal. It is true that this introduces 
some potential difference in behavior as a FILE* involves userspace buffers, 
whereas for an fd everything would go to the kernel straight away (at least on 
posix systems). However, cloning a bunch of FILE*s would introduce other kinds 
of inconsistencies, so I think this is a better tradeoff.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69532

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

[Lldb-commits] [PATCH] D69503: [LLDB] [Windows] Don't crash if the debugged process is unable to start up

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D69503#1726823 , @mstorsjo wrote:

> What's the correct course of action here? Remove the `m_session_data.reset()` 
> from ProcessWindows::OnExitProcess, concluding that it's up to some other 
> class to clear `m_session_data` in this case?


I'm not really familiar with this code, but removing the reset() call does not 
sound completely unreasonable to me. I think the only one who may have enough 
context here is @amccarth, so maybe let's wait if he has to say anything?

(Ideally, I'd just delete ProcessWindows entirely, and move everything to 
lldb-server based model -- it's much easier to reason about threads there)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69503



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

[Lldb-commits] [PATCH] D69612: [lldb-vscod] fix build with NDEBUG on windows

2019-10-30 Thread Peiyuan Song via Phabricator via lldb-commits

SquallATF added a comment.

In D69612#1726829 , @labath wrote:

> LGTM, modulo the `(void) result` stuff. Do you need someone to commit this 
> for you?


Yes I need.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69612



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

[Lldb-commits] [PATCH] D69502: [LLDB] [PECOFF] Don't crash in ReadImageDataByRVA for addresses out of range

2019-10-30 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo added a comment.

In D69502#1725155 , @mstorsjo wrote:

> In D69502#1725146 , @labath wrote:
>
> > In D69502#1723715 , @mstorsjo 
> > wrote:
> >
> > > In D69502#1723549 , @labath 
> > > wrote:
> > >
> > > > Any way to get a test for this? Maybe creating a object file with a 
> > > > bogus unwind RVA via yaml2obj ?
> > >
> > >
> > > Do we have a suitable test as basis for it? I'm not quite sure which way 
> > > is the most compact way of achieving that. A small couple function exe 
> > > with SEH or dwarf (eh_frame) unwind info, without debug info, with a 
> > > crash/int3 in a nested function? Or just some image unwind commands so it 
> > > doesn't need executing?
> >
> >
> > We have some files that might be usable as a basis for this, but I don't 
> > know which one would be the best, as I don't know what you need here. What 
> > do you need to do in order to reproduce the crash? Would it be possible to 
> > just set the export table RVA to some bogus value? That should be trigerred 
> > by just constructing the module symbol table...
>
>
> Ok, I'll look at it later to see if I can make some broken file to trigger 
> this condition.


I gave this some amount of tries, but my files with broken unwind info doesn't 
trigger it. It happened originally unreliably on arm64.

Ok to proceed with it without a testcase?

I did test crafting a file with a bogus export table RVA as well, and that 
crashes lldb elsewhere, due to an unchecked Expected<> which contained an 
error. Will try to look into that one separately later...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69502



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

[Lldb-commits] [PATCH] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I'm afraid I haven't been able to write a single line of code since I got back 
from the devmtg, but I have this on my radar, and I fully intend to get back to 
it. :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69031



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

[Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

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

This looks fine to me. Thanks for your patience. Do you still need someone to 
commit this for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62931



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

[Lldb-commits] [PATCH] D69555: [zorg] Fix LLDBCmakeBuildFactory

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rZORGa342b84f8576: [zorg] Fix LLDBCmakeBuildFactory (authored 
by labath).

Repository:
  rZORG LLVM Github Zorg

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

https://reviews.llvm.org/D69555

Files:
  zorg/buildbot/builders/LLDBBuilder.py


Index: zorg/buildbot/builders/LLDBBuilder.py
===
--- zorg/buildbot/builders/LLDBBuilder.py
+++ zorg/buildbot/builders/LLDBBuilder.py
@@ -9,8 +9,9 @@
 from buildbot.steps.slave import RemoveDirectory
 from buildbot.process.properties import WithProperties, Property
 from buildbot.steps import trigger
-import zorg.buildbot.commands.BatchFileDownload as batch_file_download
 from zorg.buildbot.commands.LitTestCommand import LitTestCommand
+from zorg.buildbot.commands.CmakeCommand import CmakeCommand
+from zorg.buildbot.conditions.FileConditions import FileDoesNotExist
 from zorg.buildbot.builders.Util import getVisualStudioEnvironment
 from zorg.buildbot.builders.Util import extractSlaveEnvironment
 from zorg.buildbot.process.factory import LLVMBuildFactory
@@ -106,24 +107,27 @@
 doStepIf=cleanBuildRequested
 ))
 
-cmake_cmd = [
-"cmake", "-G", "Ninja", os.path.join(os.pardir, f.monorepo_dir, 
"llvm"),
+path = os.path.join(os.pardir, f.monorepo_dir, "llvm")
+cmake_options = [
+"-G", "Ninja",
 "-DCMAKE_BUILD_TYPE=" + config,
-"-DCMAKE_INSTALL_PREFIX=../install"
+"-DCMAKE_INSTALL_PREFIX=../install",
+"-DLLVM_ENABLE_PROJECTS=" + ";".join(f.depends_on_projects),
 ]
 if python_source_dir:
-cmake_cmd.append("-DPYTHON_HOME=" + python_source_dir)
+cmake_options.append("-DPYTHON_HOME=" + python_source_dir)
 if extra_cmake_args:
-cmake_cmd += extra_cmake_args
-# Note: ShellCommand does not pass the params with special symbols right.
-# The " ".join is a workaround for this bug.
-f.addStep(ShellCommand(name="cmake-configure",
+cmake_options += extra_cmake_args
+
+f.addStep(CmakeCommand(name="cmake-configure",
description=["cmake configure"],
-   command=WithProperties(" ".join(cmake_cmd)),
haltOnFailure=True,
-   warnOnWarnings=True,
+   options=cmake_options,
+   path="../%s" % f.llvm_srcdir,
+   env=Property('slave_env'),
workdir=build_dir,
-   env=Property('slave_env')))
+   doStepIf=FileDoesNotExist(
+"./%s/CMakeCache.txt" % build_dir)))
 
 f.addStep(WarningCountingShellCommand(name='build',
   command=build_cmd,


Index: zorg/buildbot/builders/LLDBBuilder.py
===
--- zorg/buildbot/builders/LLDBBuilder.py
+++ zorg/buildbot/builders/LLDBBuilder.py
@@ -9,8 +9,9 @@
 from buildbot.steps.slave import RemoveDirectory
 from buildbot.process.properties import WithProperties, Property
 from buildbot.steps import trigger
-import zorg.buildbot.commands.BatchFileDownload as batch_file_download
 from zorg.buildbot.commands.LitTestCommand import LitTestCommand
+from zorg.buildbot.commands.CmakeCommand import CmakeCommand
+from zorg.buildbot.conditions.FileConditions import FileDoesNotExist
 from zorg.buildbot.builders.Util import getVisualStudioEnvironment
 from zorg.buildbot.builders.Util import extractSlaveEnvironment
 from zorg.buildbot.process.factory import LLVMBuildFactory
@@ -106,24 +107,27 @@
 doStepIf=cleanBuildRequested
 ))
 
-cmake_cmd = [
-"cmake", "-G", "Ninja", os.path.join(os.pardir, f.monorepo_dir, "llvm"),
+path = os.path.join(os.pardir, f.monorepo_dir, "llvm")
+cmake_options = [
+"-G", "Ninja",
 "-DCMAKE_BUILD_TYPE=" + config,
-"-DCMAKE_INSTALL_PREFIX=../install"
+"-DCMAKE_INSTALL_PREFIX=../install",
+"-DLLVM_ENABLE_PROJECTS=" + ";".join(f.depends_on_projects),
 ]
 if python_source_dir:
-cmake_cmd.append("-DPYTHON_HOME=" + python_source_dir)
+cmake_options.append("-DPYTHON_HOME=" + python_source_dir)
 if extra_cmake_args:
-cmake_cmd += extra_cmake_args
-# Note: ShellCommand does not pass the params with special symbols right.
-# The " ".join is a workaround for this bug.
-f.addStep(ShellCommand(name="cmake-configure",
+cmake_options += extra_cmake_args
+
+f.addStep(CmakeCommand(name="cmake-configure",
description=["cmake configure"],
-   command=WithProperties(" ".join(cmake_cmd)),
haltOnFailure=True,
-   warnOnWarnings=

[Lldb-commits] [PATCH] D69502: [LLDB] [PECOFF] Don't crash in ReadImageDataByRVA for addresses out of range

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Try this for patch for size: F10586804: a.patch 
. It needs to me moved into a separate test 
case, and cleaned up a bit, but I believe it should trigger the bug you're 
fixing here.

The tricky part about unwind info is that you currently need a Process instance 
around even to just inspect (= parse) it. I have ideas how to fix that, but no 
time to implement them :(. Until then we need to create a process somehow to 
test unwind info parsing.

Fortunately, these days we have a minidump files and their yaml representation, 
which gives us a relatively easy (though there definitely are ways to improve 
that too) method to create Processes from core files. I consider something like 
this to be the future of testing unwinding, because it's usually very hard to 
reliably drive a real process to a point which will exercise some specific 
unwind corner case. OTOH, with a minidump(core) file, you can just freeze a 
process in the state that you're intersted in, and the test will always test 
the exact same thing.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69502



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

[Lldb-commits] [lldb] f1e0ae3 - COFF: Set section permissions

2019-10-30 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2019-10-30T14:13:21+01:00
New Revision: f1e0ae3420b6cd554a240274c5ec77ccc4392ad3

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

LOG: COFF: Set section permissions

Summary:
This enables us to reason about whether a given address can be
executable, for instance during unwinding.

Reviewers: amccarth, mstorsjo

Subscribers: lldb-commits

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

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/test/Shell/ObjectFile/PECOFF/sections.yaml

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 8fa9e2000cfe..fbc5be17fa64 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -797,6 +797,7 @@ void ObjectFilePECOFF::CreateSections(SectionList 
&unified_section_list) {
 /*file_offset*/ 0, m_coff_header_opt.header_size,
 m_coff_header_opt.sect_alignment,
 /*flags*/ 0);
+header_sp->SetPermissions(ePermissionsReadable);
 m_sections_up->AddSection(header_sp);
 unified_section_list.AddSection(header_sp);
 
@@ -919,6 +920,15 @@ void ObjectFilePECOFF::CreateSections(SectionList 
&unified_section_list) {
   m_coff_header_opt.sect_alignment, // Section alignment
   m_sect_headers[idx].flags));  // Flags for this section
 
+  uint32_t permissions = 0;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_EXECUTE)
+permissions |= ePermissionsExecutable;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_READ)
+permissions |= ePermissionsReadable;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_WRITE)
+permissions |= ePermissionsWritable;
+  section_sp->SetPermissions(permissions);
+
   m_sections_up->AddSection(section_sp);
   unified_section_list.AddSection(section_sp);
 }

diff  --git a/lldb/test/Shell/ObjectFile/PECOFF/sections.yaml 
b/lldb/test/Shell/ObjectFile/PECOFF/sections.yaml
index 715ef523c13c..d41427eed0aa 100644
--- a/lldb/test/Shell/ObjectFile/PECOFF/sections.yaml
+++ b/lldb/test/Shell/ObjectFile/PECOFF/sections.yaml
@@ -7,7 +7,7 @@
 # CHECK-NEXT:   ID: 0x
 # CHECK-NEXT:   Name: PECOFF header
 # CHECK-NEXT:   Type: regular
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r--
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x4000
 # CHECK-NEXT:   VM size: 512
@@ -17,7 +17,7 @@
 # CHECK-NEXT:   ID: 0x1
 # CHECK-NEXT:   Name: .text
 # CHECK-NEXT:   Type: code
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r-x
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x40001000
 # CHECK-NEXT:   VM size: 64
@@ -27,7 +27,7 @@
 # CHECK-NEXT:   ID: 0x2
 # CHECK-NEXT:   Name: .data
 # CHECK-NEXT:   Type: data
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r--
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x40002000
 # CHECK-NEXT:   VM size: 64



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

[Lldb-commits] [lldb] 2dbcfad - [lldb-vscod] fix build with NDEBUG on windows

2019-10-30 Thread Pavel Labath via lldb-commits


Author: SquallATF
Date: 2019-10-30T14:20:22+01:00
New Revision: 2dbcfad35de6a1c86e794d911304ed50257d6d87

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

LOG: [lldb-vscod] fix build with NDEBUG on windows

Summary: _setmode in assert will not run when build with NDEBUG

Reviewers: mstorsjo, labath, amccarth

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/tools/lldb-vscode/VSCode.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/VSCode.cpp 
b/lldb/tools/lldb-vscode/VSCode.cpp
index 283798eb7aba..74c3088f4c78 100644
--- a/lldb/tools/lldb-vscode/VSCode.cpp
+++ b/lldb/tools/lldb-vscode/VSCode.cpp
@@ -44,8 +44,10 @@ VSCode::VSCode()
 // Windows opens stdout and stdin in text mode which converts \n to 13,10
 // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
 // fixes this.
-  assert(_setmode(fileno(stdout), _O_BINARY));
-  assert(_setmode(fileno(stdin), _O_BINARY));
+  int result = _setmode(fileno(stdout), _O_BINARY);
+  assert(result);
+  result = _setmode(fileno(stdin), _O_BINARY);
+  assert(result);
 #endif
   if (log_file_path)
 log.reset(new std::ofstream(log_file_path));
@@ -301,4 +303,3 @@ void VSCode::RunExitCommands() {
 }
 
 } // namespace lldb_vscode
-



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

[Lldb-commits] [PATCH] D69102: COFF: Set section permissions

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf1e0ae3420b6: COFF: Set section permissions (authored by 
labath).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D69102?vs=225403&id=227077#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69102

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/test/Shell/ObjectFile/PECOFF/sections.yaml


Index: lldb/test/Shell/ObjectFile/PECOFF/sections.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/sections.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/sections.yaml
@@ -7,7 +7,7 @@
 # CHECK-NEXT:   ID: 0x
 # CHECK-NEXT:   Name: PECOFF header
 # CHECK-NEXT:   Type: regular
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r--
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x4000
 # CHECK-NEXT:   VM size: 512
@@ -17,7 +17,7 @@
 # CHECK-NEXT:   ID: 0x1
 # CHECK-NEXT:   Name: .text
 # CHECK-NEXT:   Type: code
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r-x
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x40001000
 # CHECK-NEXT:   VM size: 64
@@ -27,7 +27,7 @@
 # CHECK-NEXT:   ID: 0x2
 # CHECK-NEXT:   Name: .data
 # CHECK-NEXT:   Type: data
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r--
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x40002000
 # CHECK-NEXT:   VM size: 64
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -797,6 +797,7 @@
 /*file_offset*/ 0, m_coff_header_opt.header_size,
 m_coff_header_opt.sect_alignment,
 /*flags*/ 0);
+header_sp->SetPermissions(ePermissionsReadable);
 m_sections_up->AddSection(header_sp);
 unified_section_list.AddSection(header_sp);
 
@@ -919,6 +920,15 @@
   m_coff_header_opt.sect_alignment, // Section alignment
   m_sect_headers[idx].flags));  // Flags for this section
 
+  uint32_t permissions = 0;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_EXECUTE)
+permissions |= ePermissionsExecutable;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_READ)
+permissions |= ePermissionsReadable;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_WRITE)
+permissions |= ePermissionsWritable;
+  section_sp->SetPermissions(permissions);
+
   m_sections_up->AddSection(section_sp);
   unified_section_list.AddSection(section_sp);
 }


Index: lldb/test/Shell/ObjectFile/PECOFF/sections.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/sections.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/sections.yaml
@@ -7,7 +7,7 @@
 # CHECK-NEXT:   ID: 0x
 # CHECK-NEXT:   Name: PECOFF header
 # CHECK-NEXT:   Type: regular
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r--
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x4000
 # CHECK-NEXT:   VM size: 512
@@ -17,7 +17,7 @@
 # CHECK-NEXT:   ID: 0x1
 # CHECK-NEXT:   Name: .text
 # CHECK-NEXT:   Type: code
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r-x
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x40001000
 # CHECK-NEXT:   VM size: 64
@@ -27,7 +27,7 @@
 # CHECK-NEXT:   ID: 0x2
 # CHECK-NEXT:   Name: .data
 # CHECK-NEXT:   Type: data
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r--
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x40002000
 # CHECK-NEXT:   VM size: 64
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -797,6 +797,7 @@
 /*file_offset*/ 0, m_coff_header_opt.header_size,
 m_coff_header_opt.sect_alignment,
 /*flags*/ 0);
+header_sp->SetPermissions(ePermissionsReadable);
 m_sections_up->AddSection(header_sp);
 unified_section_list.AddSection(header_sp);
 
@@ -919,6 +920,15 @@
   m_coff_header_opt.sect_alignment, // Section alignment
   m_sect_headers[idx].flags));  // Flags for this section
 
+  uint32_t permissions = 0;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_EXECUTE)
+permissions |= ePermissionsExecutable;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_READ)
+permissions |= ePermissionsReadable;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_WRITE)
+

[Lldb-commits] [PATCH] D69612: [lldb-vscod] fix build with NDEBUG on windows

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2dbcfad35de6: [lldb-vscod] fix build with NDEBUG on windows 
(authored by SquallATF, committed by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69612

Files:
  lldb/tools/lldb-vscode/VSCode.cpp


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -44,8 +44,10 @@
 // Windows opens stdout and stdin in text mode which converts \n to 13,10
 // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
 // fixes this.
-  assert(_setmode(fileno(stdout), _O_BINARY));
-  assert(_setmode(fileno(stdin), _O_BINARY));
+  int result = _setmode(fileno(stdout), _O_BINARY);
+  assert(result);
+  result = _setmode(fileno(stdin), _O_BINARY);
+  assert(result);
 #endif
   if (log_file_path)
 log.reset(new std::ofstream(log_file_path));
@@ -301,4 +303,3 @@
 }
 
 } // namespace lldb_vscode
-


Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -44,8 +44,10 @@
 // Windows opens stdout and stdin in text mode which converts \n to 13,10
 // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
 // fixes this.
-  assert(_setmode(fileno(stdout), _O_BINARY));
-  assert(_setmode(fileno(stdin), _O_BINARY));
+  int result = _setmode(fileno(stdout), _O_BINARY);
+  assert(result);
+  result = _setmode(fileno(stdin), _O_BINARY);
+  assert(result);
 #endif
   if (log_file_path)
 log.reset(new std::ofstream(log_file_path));
@@ -301,4 +303,3 @@
 }
 
 } // namespace lldb_vscode
-
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 2d1a0df - lldb-vscode: Add a forgotten cast to void

2019-10-30 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2019-10-30T14:27:55+01:00
New Revision: 2d1a0dfe4c7c470ad8792eaba669115dfe8eff06

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

LOG: lldb-vscode: Add a forgotten cast to void

"git push" works even with a dirty working tree. :/

Added: 


Modified: 
lldb/tools/lldb-vscode/VSCode.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/VSCode.cpp 
b/lldb/tools/lldb-vscode/VSCode.cpp
index 74c3088f4c78..2f85627da4b5 100644
--- a/lldb/tools/lldb-vscode/VSCode.cpp
+++ b/lldb/tools/lldb-vscode/VSCode.cpp
@@ -47,6 +47,7 @@ VSCode::VSCode()
   int result = _setmode(fileno(stdout), _O_BINARY);
   assert(result);
   result = _setmode(fileno(stdin), _O_BINARY);
+  (void)result;
   assert(result);
 #endif
   if (log_file_path)



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

[Lldb-commits] [PATCH] D69612: [lldb-vscod] fix build with NDEBUG on windows

2019-10-30 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo added a comment.

I presume you applied the patch with `arc patch`? It seems to apply the 
Phabricator username as git user, which is a bit odd, as it is customary to use 
realnames in the git author field. Earlier, when everything ended up mangled by 
svn, everything ended up normalized to the committer's name anyway, but as git 
preserves the author names, it would probably be good if the arc workflow could 
use the realname instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69612



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

[Lldb-commits] [PATCH] D69612: [lldb-vscod] fix build with NDEBUG on windows

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Yes, I used "arc patch". I did check that it preserved the authorship 
information, but I did not go into the details of what's in the author field 
TBH. I agree it would be better to have the full name there, but I don't know 
if there's a way to change that (short of people doing "git commit --amend 
--author=XXX" manually, which everyone is going to forget).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69612



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

[Lldb-commits] [lldb] 83a55c6 - minidump: Rename some architecture constants

2019-10-30 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2019-10-30T14:46:00+01:00
New Revision: 83a55c6a575806eec78062dfe128c095c26ab5e2

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

LOG: minidump: Rename some architecture constants

The architecture enum contains two kinds of contstants: the "official" ones
defined by Microsoft, and unofficial constants added by breakpad to cover the
architectures not described by the first ones.

Up until now, there was no big need to differentiate between the two. However,
now that Microsoft has defined
https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/ns-sysinfoapi-system_info
a constant for ARM64, we have a name clash.

This patch renames all breakpad-defined constants with to include the prefix
"BP_". This frees up the name "ARM64", which I'll re-introduce with the new
"official" value in a follow-up patch.

Reviewers: amccarth, clayborg

Subscribers: lldb-commits, llvm-commits

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

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm64-macos.yaml

lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/regions-linux-map.yaml
lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/test/Shell/Minidump/dump-all.test
lldb/test/Shell/Minidump/fb-dump.test
llvm/include/llvm/BinaryFormat/MinidumpConstants.def
llvm/lib/ObjectYAML/MinidumpYAML.cpp
llvm/test/tools/obj2yaml/basic-minidump.yaml
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm64-macos.yaml
 
b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm64-macos.yaml
index 9114424e4702..70817f14da5e 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm64-macos.yaml
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm64-macos.yaml
@@ -1,7 +1,7 @@
 --- !minidump
 Streams: 
   - Type:SystemInfo
-Processor Arch:  ARM64
+Processor Arch:  BP_ARM64
 Platform ID: MacOSX
 CSD Version: '15E216'
 CPU: 

diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/regions-linux-map.yaml
 
b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/regions-linux-map.yaml
index 3c0961eba077..680ad623361e 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/regions-linux-map.yaml
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/regions-linux-map.yaml
@@ -1,7 +1,7 @@
 --- !minidump
 Streams: 
   - Type:SystemInfo
-Processor Arch:  ARM64
+Processor Arch:  BP_ARM64
 Platform ID: Linux
 CSD Version: '15E216'
 CPU: 

diff  --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp 
b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index 47cfd5bd2730..99717e7fe34a 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -173,7 +173,7 @@ ArchSpec MinidumpParser::GetArchitecture() {
   case ProcessorArchitecture::ARM:
 triple.setArch(llvm::Triple::ArchType::arm);
 break;
-  case ProcessorArchitecture::ARM64:
+  case ProcessorArchitecture::BP_ARM64:
 triple.setArch(llvm::Triple::ArchType::aarch64);
 break;
   default:

diff  --git a/lldb/test/Shell/Minidump/dump-all.test 
b/lldb/test/Shell/Minidump/dump-all.test
index 92f2be24e173..507c1688bb8b 100644
--- a/lldb/test/Shell/Minidump/dump-all.test
+++ b/lldb/test/Shell/Minidump/dump-all.test
@@ -57,7 +57,7 @@
 --- !minidump
 Streams: 
   - Type:SystemInfo
-Processor Arch:  ARM64
+Processor Arch:  BP_ARM64
 Platform ID: Linux
 CSD Version: '15E216'
 CPU: 

diff  --git a/lldb/test/Shell/Minidump/fb-dump.test 
b/lldb/test/Shell/Minidump/fb-dump.test
index 1cd777c6d718..319db2f3d368 100644
--- a/lldb/test/Shell/Minidump/fb-dump.test
+++ b/lldb/test/Shell/Minidump/fb-dump.test
@@ -61,7 +61,7 @@
 --- !minidump
 Streams: 
   - Type:SystemInfo
-Processor Arch:  ARM64
+Processor Arch:  BP_ARM64
 Platform ID: Linux
 CSD Version: '15E216'
 CPU: 

diff  --git a/llvm/include/llvm/BinaryFormat/MinidumpConstants.def 
b/llvm/include/llvm/BinaryFormat/MinidumpConstants.def
index aeef399af7a4..c04a10d30d4c 100644
--- a/llvm/include/llvm/BinaryFormat/MinidumpConstants.def
+++ b/llvm/include/llvm/BinaryFormat/MinidumpConstants.def
@@ -85,21 +85,21 @@ HANDLE_MDMP_STREAM_TYPE(0xFACE

[Lldb-commits] [PATCH] D69285: minidump: Rename some architecture constants

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG83a55c6a5758: minidump: Rename some architecture constants 
(authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D69285?vs=225971&id=227084#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69285

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm64-macos.yaml
  
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/regions-linux-map.yaml
  lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
  lldb/test/Shell/Minidump/dump-all.test
  lldb/test/Shell/Minidump/fb-dump.test
  llvm/include/llvm/BinaryFormat/MinidumpConstants.def
  llvm/lib/ObjectYAML/MinidumpYAML.cpp
  llvm/test/tools/obj2yaml/basic-minidump.yaml
  llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -33,7 +33,7 @@
 --- !minidump
 Streams:
   - Type:SystemInfo
-Processor Arch:  ARM64
+Processor Arch:  BP_ARM64
 Platform ID: Linux
 CPU:
   CPUID:   0x05060708
@@ -53,7 +53,7 @@
   auto ExpectedSysInfo = File.getSystemInfo();
   ASSERT_THAT_EXPECTED(ExpectedSysInfo, Succeeded());
   const SystemInfo &SysInfo = *ExpectedSysInfo;
-  EXPECT_EQ(ProcessorArchitecture::ARM64, SysInfo.ProcessorArch);
+  EXPECT_EQ(ProcessorArchitecture::BP_ARM64, SysInfo.ProcessorArch);
   EXPECT_EQ(OSPlatform::Linux, SysInfo.PlatformId);
   EXPECT_EQ(0x05060708u, SysInfo.CPU.Arm.CPUID);
 
Index: llvm/test/tools/obj2yaml/basic-minidump.yaml
===
--- llvm/test/tools/obj2yaml/basic-minidump.yaml
+++ llvm/test/tools/obj2yaml/basic-minidump.yaml
@@ -3,7 +3,7 @@
 --- !minidump
 Streams:
   - Type:SystemInfo
-Processor Arch:  ARM64
+Processor Arch:  BP_ARM64
 Platform ID: Linux
 CSD Version: Linux 3.13.0-91-generic
 CPU:
@@ -92,7 +92,7 @@
 # CHECK:  --- !minidump
 # CHECK-NEXT: Streams:
 # CHECK-NEXT:   - Type:SystemInfo
-# CHECK-NEXT: Processor Arch:  ARM64
+# CHECK-NEXT: Processor Arch:  BP_ARM64
 # CHECK-NEXT: Platform ID: Linux
 # CHECK-NEXT: CSD Version: Linux 3.13.0-91-generic
 # CHECK-NEXT: CPU:
Index: llvm/lib/ObjectYAML/MinidumpYAML.cpp
===
--- llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -336,7 +336,7 @@
 IO.mapOptional("CPU", Info.CPU.X86);
 break;
   case ProcessorArchitecture::ARM:
-  case ProcessorArchitecture::ARM64:
+  case ProcessorArchitecture::BP_ARM64:
 IO.mapOptional("CPU", Info.CPU.Arm);
 break;
   default:
Index: llvm/include/llvm/BinaryFormat/MinidumpConstants.def
===
--- llvm/include/llvm/BinaryFormat/MinidumpConstants.def
+++ llvm/include/llvm/BinaryFormat/MinidumpConstants.def
@@ -85,21 +85,21 @@
 HANDLE_MDMP_STREAM_TYPE(0xFACEDEAD, FacebookAbortReason)
 HANDLE_MDMP_STREAM_TYPE(0xFACEE000, FacebookThreadName)
 
-HANDLE_MDMP_ARCH(0x, X86)  // PROCESSOR_ARCHITECTURE_INTEL
-HANDLE_MDMP_ARCH(0x0001, MIPS) // PROCESSOR_ARCHITECTURE_MIPS
-HANDLE_MDMP_ARCH(0x0002, Alpha)// PROCESSOR_ARCHITECTURE_ALPHA
-HANDLE_MDMP_ARCH(0x0003, PPC)  // PROCESSOR_ARCHITECTURE_PPC
-HANDLE_MDMP_ARCH(0x0004, SHX)  // PROCESSOR_ARCHITECTURE_SHX (Super-H)
-HANDLE_MDMP_ARCH(0x0005, ARM)  // PROCESSOR_ARCHITECTURE_ARM
-HANDLE_MDMP_ARCH(0x0006, IA64) // PROCESSOR_ARCHITECTURE_IA64
-HANDLE_MDMP_ARCH(0x0007, Alpha64)  // PROCESSOR_ARCHITECTURE_ALPHA64
-HANDLE_MDMP_ARCH(0x0008, MSIL) // PROCESSOR_ARCHITECTURE_MSIL
-HANDLE_MDMP_ARCH(0x0009, AMD64)// PROCESSOR_ARCHITECTURE_AMD64
-HANDLE_MDMP_ARCH(0x000a, X86Win64) // PROCESSOR_ARCHITECTURE_IA32_ON_WIN64
-HANDLE_MDMP_ARCH(0x8001, SPARC)// Breakpad-defined value for SPARC
-HANDLE_MDMP_ARCH(0x8002, PPC64)// Breakpad-defined value for PPC64
-HANDLE_MDMP_ARCH(0x8003, ARM64)// Breakpad-defined value for ARM64
-HANDLE_MDMP_ARCH(0x8004, MIPS64)   // Breakpad-defined value for MIPS64
+HANDLE_MDMP_ARCH(0x, X86)   // PROCESSOR_ARCHITECTURE_INTEL
+HANDLE_MDMP_ARCH(0x0001, MIPS)  // PROCESSOR_ARCHITECTURE_MIPS
+HANDLE_MDMP_ARCH(0x0002, Alpha) // PROCESSOR_ARCHITECTURE_ALPHA
+HANDLE_MDMP_ARCH(0x0003, PPC)   // PROCESSOR_ARCHITECTURE_PPC
+HANDLE_MDMP_ARCH(0x0004, SHX)   // PROCESSOR_ARCHITECTURE_SHX (Super-H)
+HANDLE_MDMP_ARCH(0x0005, ARM)   // PROCESSOR_ARCHITECTURE_ARM
+HANDLE_MDMP_ARCH(0x0006, IA64)  // PROCESSOR_ARCHITECTURE_IA64
+HANDLE_MDMP_ARCH(0x0007, Alpha64)   // PROCESSOR_ARCHITECTURE_ALPHA64
+HANDLE_MDMP_ARCH(0x0008

[Lldb-commits] [PATCH] D69535: build: improve python check for Windows

2019-10-30 Thread Adrian McCarthy via Phabricator via lldb-commits

amccarth added a comment.

In D69535#1724797 , @labath wrote:

> .. Given that this code is python3 specific and other platforms still support 
> python2, maybe we can make this bump windows-specific at first, and then 
> extend it to other platforms once we officially stop supporting python2 ?


Yes, please.  It's a mess on Windows, so I'll support a Windows-specific 
version bump.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69535



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

[Lldb-commits] [PATCH] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

BTW, as I started working on this patch, I realized that there is a (somewhat 
surprising) workaround for this problem. If we change the isysroot argument to 
be passed as `-isysroot/foo` (instead of `-isysroot /foo`), then clang-cl will 
properly ignore this argument (as it already knows how to ignore most 
arguments). The main surprising thing here is that the argument is 
`-isysroot/foo` and not `--isysroot=/foo`, but maybe that could also be changed 
for the sake of consistency with `--sysroot`...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69031



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

[Lldb-commits] [PATCH] D69619: [lldb/lit] Introduce %clang_host substitutions

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

labath created this revision.
labath added reviewers: JDevlieghere, aprantl, mstorsjo.
Herald added subscribers: jfb, MaskRay, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLDB.

This patch addresses an ambiguity in how our existing tests invoke the
compiler. Roughly two thirds of our current "shell" tests invoke the
compiler to build the executables for the host. However, there is also
a significant number of tests which don't build a host binary (because
they don't need to run it) and instead they hardcode a certain target.

We also have code which adds a bunch of default arguments to the %clang
substitutions. However, most of these arguments only really make sense
for the host compilation. So far, this has worked mostly ok, because the
arguments we were adding were not conflicting with the target-hardcoding
tests (though they did provoke an occasional "argument unused" warning).

However, this started to break down when we wanted to use
target-hardcoding clang-cl tests (D69031 ) 
because clang-cl has a
substantially different command line, and it was getting very confused
by some of the arguments we were adding on non-windows hosts.

This patch avoid this problem by creating separate %clang(xx,_cl)_host
substutitions, which are specifically meant to be used for compiling
host binaries. All funny host-specific options are moved there. To
ensure that the regular %clang substitutions are not used for compiling
host binaries (skipping the extra arguments) I employ a little
hac^H^H^Htrick -- I add an invalid --target argument to the %clang
substitution, which means that one has to use an explicit --target in
order for the compilation to succeed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69619

Files:
  lldb/test/Shell/Commands/command-script-import.test
  lldb/test/Shell/Driver/TestSingleQuote.test
  lldb/test/Shell/Driver/TestTarget.test
  lldb/test/Shell/ExecControl/StopHook/stop-hook-threads.test
  lldb/test/Shell/ExecControl/StopHook/stop-hook.test
  lldb/test/Shell/Expr/TestIRMemoryMap.test
  lldb/test/Shell/Expr/TestIRMemoryMapWindows.test
  lldb/test/Shell/Heap/heap-cstr.test
  lldb/test/Shell/Host/TestCustomShell.test
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-set-and-hit-breakpoint.test
  lldb/test/Shell/Process/TestEnvironment.test
  lldb/test/Shell/Register/aarch64-fp-read.test
  lldb/test/Shell/Register/aarch64-gp-read.test
  lldb/test/Shell/Register/arm-fp-read.test
  lldb/test/Shell/Register/arm-gp-read.test
  lldb/test/Shell/Register/x86-64-gp-read.test
  lldb/test/Shell/Register/x86-64-gp-write.test
  lldb/test/Shell/Register/x86-64-read.test
  lldb/test/Shell/Register/x86-64-write.test
  lldb/test/Shell/Register/x86-64-xmm16-read.test
  lldb/test/Shell/Register/x86-64-xmm16-write.test
  lldb/test/Shell/Register/x86-64-ymm-read.test
  lldb/test/Shell/Register/x86-64-ymm-write.test
  lldb/test/Shell/Register/x86-64-ymm16-read.test
  lldb/test/Shell/Register/x86-64-ymm16-write.test
  lldb/test/Shell/Register/x86-64-zmm-read.test
  lldb/test/Shell/Register/x86-64-zmm-write.test
  lldb/test/Shell/Register/x86-gp-read.test
  lldb/test/Shell/Register/x86-gp-write.test
  lldb/test/Shell/Register/x86-mm-xmm-read.test
  lldb/test/Shell/Register/x86-mm-xmm-write.test
  lldb/test/Shell/Register/x86-ymm-read.test
  lldb/test/Shell/Register/x86-ymm-write.test
  lldb/test/Shell/Register/x86-zmm-read.test
  lldb/test/Shell/Register/x86-zmm-write.test
  lldb/test/Shell/Reproducer/Functionalities/TestDataFormatter.test
  lldb/test/Shell/Reproducer/Functionalities/TestImageList.test
  lldb/test/Shell/Reproducer/Functionalities/TestStepping.test
  lldb/test/Shell/Reproducer/Modules/TestModuleCXX.test
  lldb/test/Shell/Reproducer/TestDump.test
  lldb/test/Shell/Reproducer/TestFileRepro.test
  lldb/test/Shell/Reproducer/TestGDBRemoteRepro.test
  lldb/test/Shell/Reproducer/TestRelativePath.test
  lldb/test/Shell/Reproducer/TestReuseDirectory.test
  lldb/test/Shell/Reproducer/TestWorkingDir.test
  lldb/test/Shell/Settings/TestFrameFormatColor.test
  lldb/test/Shell/Settings/TestFrameFormatNoColor.test
  lldb/test/Shell/SymbolFile/DWARF/anon_class_w_and_wo_export_symbols.ll
  
lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug-types-expressions.test
  lldb/test/Shell/SymbolFile/DWARF/deterministic-build.cpp
  lldb/test/Shell/SymbolFile/PDB/function-level-linking.test
  lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test
  lldb/test/Shell/Unwind/eh-frame-dwarf-unwind.test
  lldb/test/Shell/Unwind/eh-frame-small-fde.test
  lldb/test/Shell/Unwind/prefer-debug-over-eh-frame.test
  lldb/test/Shell/Unwind/trap_frame_sym_ctx.test
  lldb/test/Shell/Unwind/unwind-plan-dwarf-dump.test
  lldb/test/Shell/Watchpoint/SetErrorCases.test
  lldb/test/Shell/helper/toolchain.py

Index: lldb/test/Shell/helper/toolchain.py
=

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

BTW, I was bored on the plane, so I created this proof of concept F10589216: 
dense.cc . It needs a lot of cleanup of 
course, but it demonstrates one way to generically store some extra data with 
some type. It gives each type the option to define a bunch of unused (the patch 
calls them "free") values (which can be useful for for Optional<>s, DenseMaps, 
etc.). Also each type can say that it is able to store some extra unrelated 
bits (like PointerIntPair does), and with a bit of care, one can even nest 
these types arbitrarily). the main thing I haven't yet figured out is the 
relationship with DenseMapInfo. Theoretically DenseMapInfo could be implemented 
on top of DenseInfo, but I am still wondering whether we could not make that a 
single type (trait) somehow...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230



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

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

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

That is interesting. Can you send me the binary produced by this test? I'd like 
to compare it with what I get locally. (I can also send you mine if you're 
interested)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68130



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

[Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

2019-10-30 Thread Guilherme Andrade via Phabricator via lldb-commits

guiandrade added a comment.

In D62931#1726865 , @labath wrote:

> This looks fine to me. Thanks for your patience. Do you still need someone to 
> commit this for you?


Np. Yes, I do. Could you please do it for me?

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62931



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

[Lldb-commits] [PATCH] D69619: [lldb/lit] Introduce %clang_host substitutions

2019-10-30 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added a comment.

this looks like a reasonable incremental improvement. That said, I'm wondering 
how useful `clang_host` is as a concept in the LIT tests. In my own mental 
model the tests are grouped into

1. end-to-end tests that test lldb against a variety of user-configurable 
compilers and with different debug info formats
2. unit-tests/internal consistency tests where we may use a compiler for the 
sake of not having to check in a binary, but where we expect a specific output 
from the compiler and thus should always use the just-built clang.

The `packages` subdirectory covers category (1) and the `lit` subdirectory 
covers category (2).

Based on that I would expect that we either always hard-code a triple in the 
lit tests (may result in less coverage if the target isn't available), or we 
run them for each available target. Giving the host target preference seems 
arbitrary, given how extensively LLDB is used on an x86_64 host, debugging 
AArch64 targets, for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69619



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

[Lldb-commits] [lldb] 3071ebf - [LLDB][PythonFile] fix dangerous borrow semantics on python2

2019-10-30 Thread Lawrence D'Anna via lldb-commits


Author: Lawrence D'Anna
Date: 2019-10-30T09:46:51-07:00
New Revision: 3071ebf7b38341e89be04aa64c257c4643e0648c

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

LOG: [LLDB][PythonFile] fix dangerous borrow semantics on python2

Summary:
It is inherently unsafe to allow a python program to manipulate borrowed
memory from a python object's destructor. It would be nice to
flush a borrowed file when python is finished with it, but it's not safe
to do on python 2.

Python 3 does not suffer from this issue.

Reviewers: labath, mgorny

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py 
b/lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
index f7f1ad08500a..5d025a46dced 100644
--- 
a/lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ 
b/lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -851,10 +851,6 @@ def i(sbf):
 yield sbf
 sbf.Write(str(i).encode('ascii') + b"\n")
 files = list(i(sbf))
-# delete them in reverse order, again because each is a borrow
-# of the previous.
-while files:
-files.pop()
 with open(self.out_filename, 'r') as f:
 self.assertEqual(list(range(10)), list(map(int, 
f.read().strip().split(
 

diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
index df8bac951fc4..ef5eb7a57d9c 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1500,21 +1500,23 @@ Expected PythonFile::FromFile(File &file, 
const char *mode) {
   PyObject *file_obj;
 #if PY_MAJOR_VERSION >= 3
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
-   "ignore", nullptr, 0);
+   "ignore", nullptr, /*closefd=*/0);
 #else
-  // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.   NetBSD does not allow
-  // input files to be flushed, so we have to check for that case too.
-  int (*closer)(FILE *);
-  auto opts = file.GetOptions();
-  if (!opts)
-return opts.takeError();
-  if (opts.get() & File::eOpenOptionWrite)
-closer = ::fflush;
-  else
-closer = [](FILE *) { return 0; };
+  // I'd like to pass ::fflush here if the file is writable,  so that
+  // when the python side destructs the file object it will be flushed.
+  // However, this would be dangerous.It can cause fflush to be called
+  // after fclose if the python program keeps a reference to the file after
+  // the original lldb_private::File has been destructed.
+  //
+  // It's all well and good to ask a python program not to use a closed file
+  // but asking a python program to make sure objects get released in a
+  // particular order is not safe.
+  //
+  // The tradeoff here is that if a python 2 program wants to make sure this
+  // file gets flushed, they'll have to do it explicitly or wait untill the
+  // original lldb File itself gets flushed.
   file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
- py2_const_cast(mode), closer);
+ py2_const_cast(mode), [](FILE *) { return 0; });
 #endif
 
   if (!file_obj)



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

[Lldb-commits] [lldb] fb01c01 - [LLDB][Python] warning fix for LLDBSwigPythonBreakpointCallbackFunction

2019-10-30 Thread Lawrence D'Anna via lldb-commits


Author: Lawrence D'Anna
Date: 2019-10-30T09:47:27-07:00
New Revision: fb01c01bf3f60d1d229126ea4088519adae5c015

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

LOG: [LLDB][Python] warning fix for LLDBSwigPythonBreakpointCallbackFunction

This is a quick followup to this commit:

https://reviews.llvm.org/rGa69bbe02a2352271e8b14542073f177e24c499c1

In that, I #pragma-squelch this warning in `ScriptInterpreterPython.cpp`
but we get the same warning in `PythonTestSuite.cpp`.

This patch squelches the same warning in the same way as the
reviweed commit.   I'm submitting it without review under the
"obviously correct" rule.

At least if this is incorrect the main commit was also incorrect.

By the way, as far as I can tell, these functions are extern "C" because
SWIG does that to everything, not because they particularly need to be.

Added: 


Modified: 
lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Removed: 




diff  --git a/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp 
b/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
index 2ac631eba424..12ffdfe79ec3 100644
--- a/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ b/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -59,6 +59,9 @@ extern "C" void init_lldb(void) {}
 #define LLDBSwigPyInit init_lldb
 #endif
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+
 extern "C" llvm::Expected LLDBSwigPythonBreakpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
 const lldb::StackFrameSP &sb_frame,
@@ -67,6 +70,8 @@ extern "C" llvm::Expected 
LLDBSwigPythonBreakpointCallbackFunction(
   return false;
 }
 
+#pragma clang diagnostic pop
+
 extern "C" bool LLDBSwigPythonWatchpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
 const lldb::StackFrameSP &sb_frame, const lldb::WatchpointSP &sb_wp) {



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

[Lldb-commits] [PATCH] D69532: [LLDB][PythonFile] fix dangerous borrow semantics on python2

2019-10-30 Thread Lawrence D'Anna via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3071ebf7b383: [LLDB][PythonFile] fix dangerous borrow 
semantics on python2 (authored by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69532

Files:
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1500,21 +1500,23 @@
   PyObject *file_obj;
 #if PY_MAJOR_VERSION >= 3
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
-   "ignore", nullptr, 0);
+   "ignore", nullptr, /*closefd=*/0);
 #else
-  // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.   NetBSD does not allow
-  // input files to be flushed, so we have to check for that case too.
-  int (*closer)(FILE *);
-  auto opts = file.GetOptions();
-  if (!opts)
-return opts.takeError();
-  if (opts.get() & File::eOpenOptionWrite)
-closer = ::fflush;
-  else
-closer = [](FILE *) { return 0; };
+  // I'd like to pass ::fflush here if the file is writable,  so that
+  // when the python side destructs the file object it will be flushed.
+  // However, this would be dangerous.It can cause fflush to be called
+  // after fclose if the python program keeps a reference to the file after
+  // the original lldb_private::File has been destructed.
+  //
+  // It's all well and good to ask a python program not to use a closed file
+  // but asking a python program to make sure objects get released in a
+  // particular order is not safe.
+  //
+  // The tradeoff here is that if a python 2 program wants to make sure this
+  // file gets flushed, they'll have to do it explicitly or wait untill the
+  // original lldb File itself gets flushed.
   file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
- py2_const_cast(mode), closer);
+ py2_const_cast(mode), [](FILE *) { return 0; });
 #endif
 
   if (!file_obj)
Index: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -851,10 +851,6 @@
 yield sbf
 sbf.Write(str(i).encode('ascii') + b"\n")
 files = list(i(sbf))
-# delete them in reverse order, again because each is a borrow
-# of the previous.
-while files:
-files.pop()
 with open(self.out_filename, 'r') as f:
 self.assertEqual(list(range(10)), list(map(int, 
f.read().strip().split(
 


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1500,21 +1500,23 @@
   PyObject *file_obj;
 #if PY_MAJOR_VERSION >= 3
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
-   "ignore", nullptr, 0);
+   "ignore", nullptr, /*closefd=*/0);
 #else
-  // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.   NetBSD does not allow
-  // input files to be flushed, so we have to check for that case too.
-  int (*closer)(FILE *);
-  auto opts = file.GetOptions();
-  if (!opts)
-return opts.takeError();
-  if (opts.get() & File::eOpenOptionWrite)
-closer = ::fflush;
-  else
-closer = [](FILE *) { return 0; };
+  // I'd like to pass ::fflush here if the file is writable,  so that
+  // when the python side destructs the file object it will be flushed.
+  // However, this would be dangerous.It can cause fflush to be called
+  // after fclose if the python program keeps a reference to the file after
+  // the original lldb_private::File has been destructed.
+  //
+  // It's all well and good to ask a python program not to use a closed file
+  // but asking a python program to make sure objects get released in a
+  // particular order is not safe.
+  //
+  // The tradeoff here is that if a python 2 program wants to make sure this
+  // file gets flushed, they'll have to do it explicitly or wait untill the
+  // original lldb File itself gets flushed.
   fil

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

2019-10-30 Thread Haibo Huang via Phabricator via lldb-commits

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

In D69589#1726812 , @labath wrote:

> This looks reasonable to me. I'm just wondering, now that these are separate 
> targets, I guess that means they can be run in random order, right? Will that 
> cause any problems, for instance when creating a package and its subpackage 
> (formatters and formatters/cpp maybe)?

I try to avoid that problem by creating directory every time. Other than that I 
don't think there's any dependencies between two subpackages...

But in general, yes, that's something we should be careful of...

Comment at: lldb/CMakeLists.txt:133
+  add_copy_file_target(lldb_python_init
+FILES  "${lldb_scripts_dir}/lldb.py"
+DEST_DIR   "${lldb_python_build_path}"

labath wrote:
> Would it be possible to just make the swig step place this file into the 
> correct place directly?
The difficulty is that, there is a config time path dependency like this:

swig_wrapper -> liblldb -> finish_swig (renamed to lldb_python_packages in this 
change)

Where liblldb uses the path of LLDBWrapPython.cpp in swig_wrapper. And here we 
reference the path of liblldb, to calculate the right path for python packages.

That means when swig_wrapper is created, we don't have a good way to figure out 
the right path for python packages and put the file there directly.

The best way to solve this is to hard code liblldb path for framework build. 
I.e. replace line 92 here:

```
get_target_property(liblldb_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
```

With something more constant. This removes the liblldb -> finish_swig 
dependency. Then all the code here can be moved to scripts/CMakeLists.txt (I 
think they belong there better anyway). And swig_wrapper can get access to 
python package path.

The only question is whether we can / should "predict" liblldb path for 
framework... I'll look more into this. But it probably deserves a separate 
change.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69589

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

[Lldb-commits] [PATCH] D69630: [lldb] Record framework build path and use it everywhere

2019-10-30 Thread Haibo Huang via Phabricator via lldb-commits

hhb created this revision.
hhb added a reviewer: labath.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.

This avoids config time dependencies on liblldb. And enables other
refactoring.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69630

Files:
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/cmake/modules/LLDBFramework.cmake
  lldb/test/API/CMakeLists.txt
  lldb/tools/debugserver/source/CMakeLists.txt
  lldb/tools/driver/CMakeLists.txt
  lldb/tools/lldb-vscode/CMakeLists.txt

Index: lldb/tools/lldb-vscode/CMakeLists.txt
===
--- lldb/tools/lldb-vscode/CMakeLists.txt
+++ lldb/tools/lldb-vscode/CMakeLists.txt
@@ -33,10 +33,9 @@
 if(LLDB_BUILD_FRAMEWORK)
   # In the build-tree, we know the exact path to the framework directory.
   # The installed framework can be in different locations.
-  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
   lldb_setup_rpaths(lldb-vscode
 BUILD_RPATH
-  "${framework_build_dir}"
+  "${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}"
 INSTALL_RPATH
   "@loader_path/../../../SharedFrameworks"
   "@loader_path/../../System/Library/PrivateFrameworks"
Index: lldb/tools/driver/CMakeLists.txt
===
--- lldb/tools/driver/CMakeLists.txt
+++ lldb/tools/driver/CMakeLists.txt
@@ -33,10 +33,9 @@
 if(LLDB_BUILD_FRAMEWORK)
   # In the build-tree, we know the exact path to the framework directory.
   # The installed framework can be in different locations.
-  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
   lldb_setup_rpaths(lldb
 BUILD_RPATH
-  "${framework_build_dir}"
+  "${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}"
 INSTALL_RPATH
   "@loader_path/../../../SharedFrameworks"
   "@loader_path/../../System/Library/PrivateFrameworks"
Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -219,8 +219,7 @@
 set(pass_entitlements --entitlements ${entitlements})
   endif()
 
-  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
-  set(copy_location ${framework_build_dir}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Resources/debugserver)
+  set(copy_location ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Resources/debugserver)
 
   add_custom_command(TARGET debugserver POST_BUILD
 COMMAND ${CMAKE_COMMAND} -E
Index: lldb/test/API/CMakeLists.txt
===
--- lldb/test/API/CMakeLists.txt
+++ lldb/test/API/CMakeLists.txt
@@ -107,8 +107,7 @@
 
 if(CMAKE_HOST_APPLE)
   if(LLDB_BUILD_FRAMEWORK)
-get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
-list(APPEND LLDB_TEST_COMMON_ARGS --framework ${framework_build_dir}/LLDB.framework)
+list(APPEND LLDB_TEST_COMMON_ARGS --framework ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework)
   endif()
 
   # Use the same identity for testing
Index: lldb/cmake/modules/LLDBFramework.cmake
===
--- lldb/cmake/modules/LLDBFramework.cmake
+++ lldb/cmake/modules/LLDBFramework.cmake
@@ -1,10 +1,4 @@
-# Path relative to the root binary directory
-get_filename_component(
-  framework_target_dir ${LLDB_FRAMEWORK_BUILD_DIR} ABSOLUTE
-  BASE_DIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}
-)
-
-message(STATUS "LLDB.framework: build path is '${framework_target_dir}'")
+message(STATUS "LLDB.framework: build path is '${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}'")
 message(STATUS "LLDB.framework: install path is '${LLDB_FRAMEWORK_INSTALL_DIR}'")
 message(STATUS "LLDB.framework: resources subdirectory is 'Versions/${LLDB_FRAMEWORK_VERSION}/Resources'")
 
@@ -15,7 +9,7 @@
 
   OUTPUT_NAME LLDB
   VERSION ${LLDB_VERSION}
-  LIBRARY_OUTPUT_DIRECTORY ${framework_target_dir}
+  LIBRARY_OUTPUT_DIRECTORY ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}
 
   # Compatibility version
   SOVERSION "1.0.0"
@@ -29,8 +23,8 @@
 # Used in llvm_add_library() to set default output directories for multi-config
 # generators. Overwrite to account for special framework output directory.
 set_output_directory(liblldb
-  BINARY_DIR ${framework_target_dir}
-  LIBRARY_DIR ${framework_target_dir}
+  BINARY_DIR ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}
+  LIBRARY_DIR ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}
 )
 
 lldb_add_post_install_steps_darwin(liblldb ${LLDB_FRAMEWORK_INSTALL_DIR})
@@ -51,7 +45,7 @@
 add_custom_command(TARGET liblldb POST_BUILD
   COMMAND ${CMAKE_COMMAND} -E create_symlink
   Versions/Current/Headers
-  ${framework_target_dir}/LLDB.framework/Headers
+  ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework/Headers
   COMMENT "

[Lldb-commits] [PATCH] D69612: [lldb-vscod] fix build with NDEBUG on windows

2019-10-30 Thread Adrian McCarthy via Phabricator via lldb-commits

amccarth added a comment.

In D69612#1726849 , @SquallATF wrote:

> In D69612#1726829 , @labath wrote:
>
> > LGTM, modulo the `(void) result` stuff. Do you need someone to commit this 
> > for you?
>
>
> Yes I need.


Was the `(void) result` fix applied before this was committed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69612



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

[Lldb-commits] [PATCH] D69619: [lldb/lit] Introduce %clang_host substitutions

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Yes, I understand what you're saying, but I am afraid the situation is not as 
clear-cut as that. I would be very happy if it was, but it's definitely not the 
current situation, and given the complexities involved, I'm not sure if we 
should even aim for that goal. I mean, a lot of times these things end up being 
down to the personal preference of the author (or reviewer), but there are 
cases where writing a test of the appropriate category (based on your above 
classification) would be very hard. Lemme give some examples:

- there is a subclass of "dotest" tests which test lldb's python api (most of 
them tagged with the "pyapi" category), which test lldb's python api. These 
definitely don't benefit from being run with multiple compilers (in fact, a lot 
of them don't run the compiler at all). I don't think it makes sense to change 
these into "lit" tests.
- another class of "dotest" tests are those which test some interactive 
behavior of lldb (inferior control, thread plans, stepping and stuff). While 
these may touch debug info peripherally, it's usually not their main focus, and 
I don't think we should be relying on them on testing debug info. However, 
being able to run them in a more "interactive" manner is usually more 
convenient.
- "pexpect" tests definitely don't/shouldn't test debug info, but they 
currently exist only in the "dotest" suite (though they are fairly independent, 
and they could be extracted if we wanted to)
- on the other side of the fence, there are "lit" tests which do not test debug 
info, or even "debugging", but they still need a host executable, because 
debugging something is the only way to trigger some functionality. For instance 
the "register" tests need a functional executable to check that they can 
read/write registers from it. Unwind tests need a running executable because we 
can't test even basic unwind without a running process now. Reproducers need a 
debuggable executable too, but they don't really care about it's contents, etc.

So overall, while there definitely are tests which would be better off moved 
from one category or the other, I am doubtful that we'll be able to enforce a 
separation where all "configurable" tests are dotest tests, and all "lit" tests 
hardcode a triple. The one barrier we have right now is that it is not possible 
to run the "lit" suite with a different compiler, and that's something I think 
we should keep. And we can definitely change some %clang_hosts to a hard coded 
triple (I think that would remove some "UNSUPPORTED: windows" stanzas) and/or 
be stricter about using %clang_host in the future. In fact I am happy that you 
spoke out against %clang_host, because when I was  going through these tests, I 
got the impression that the only tests which have a specific triple hard-coded 
are those that I have written myself. :)

For the test which don't need a host executable, I don't think that running 
them for each available target would be useful, as they generally don't care 
about those details. What they sometimes care about is the object file format 
(because of dwarf differences in elf vs. macho), so it may make sense to 
introduce some substitution like  %elf_triple and %macho_triple, which would 
choose some elf flavour of one of the configured targets. However, I am afraid 
that even that would be of limited usefulness, as it's pretty hard to make a 
specific test which is target agnostic (I tried) -- there are differences in 
pointer sizes, and even the different assemblers can't agree on what they use 
as a comment character.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69619



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

[Lldb-commits] [PATCH] D69630: [lldb] Record framework build path and use it everywhere

2019-10-30 Thread Haibo Huang via Phabricator via lldb-commits

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

Update one more place


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69630

Files:
  lldb/CMakeLists.txt
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/cmake/modules/LLDBFramework.cmake
  lldb/test/API/CMakeLists.txt
  lldb/tools/debugserver/source/CMakeLists.txt
  lldb/tools/driver/CMakeLists.txt
  lldb/tools/lldb-vscode/CMakeLists.txt

Index: lldb/tools/lldb-vscode/CMakeLists.txt
===
--- lldb/tools/lldb-vscode/CMakeLists.txt
+++ lldb/tools/lldb-vscode/CMakeLists.txt
@@ -33,10 +33,9 @@
 if(LLDB_BUILD_FRAMEWORK)
   # In the build-tree, we know the exact path to the framework directory.
   # The installed framework can be in different locations.
-  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
   lldb_setup_rpaths(lldb-vscode
 BUILD_RPATH
-  "${framework_build_dir}"
+  "${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}"
 INSTALL_RPATH
   "@loader_path/../../../SharedFrameworks"
   "@loader_path/../../System/Library/PrivateFrameworks"
Index: lldb/tools/driver/CMakeLists.txt
===
--- lldb/tools/driver/CMakeLists.txt
+++ lldb/tools/driver/CMakeLists.txt
@@ -33,10 +33,9 @@
 if(LLDB_BUILD_FRAMEWORK)
   # In the build-tree, we know the exact path to the framework directory.
   # The installed framework can be in different locations.
-  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
   lldb_setup_rpaths(lldb
 BUILD_RPATH
-  "${framework_build_dir}"
+  "${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}"
 INSTALL_RPATH
   "@loader_path/../../../SharedFrameworks"
   "@loader_path/../../System/Library/PrivateFrameworks"
Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -219,8 +219,7 @@
 set(pass_entitlements --entitlements ${entitlements})
   endif()
 
-  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
-  set(copy_location ${framework_build_dir}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Resources/debugserver)
+  set(copy_location ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Resources/debugserver)
 
   add_custom_command(TARGET debugserver POST_BUILD
 COMMAND ${CMAKE_COMMAND} -E
Index: lldb/test/API/CMakeLists.txt
===
--- lldb/test/API/CMakeLists.txt
+++ lldb/test/API/CMakeLists.txt
@@ -107,8 +107,7 @@
 
 if(CMAKE_HOST_APPLE)
   if(LLDB_BUILD_FRAMEWORK)
-get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
-list(APPEND LLDB_TEST_COMMON_ARGS --framework ${framework_build_dir}/LLDB.framework)
+list(APPEND LLDB_TEST_COMMON_ARGS --framework ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework)
   endif()
 
   # Use the same identity for testing
Index: lldb/cmake/modules/LLDBFramework.cmake
===
--- lldb/cmake/modules/LLDBFramework.cmake
+++ lldb/cmake/modules/LLDBFramework.cmake
@@ -1,10 +1,4 @@
-# Path relative to the root binary directory
-get_filename_component(
-  framework_target_dir ${LLDB_FRAMEWORK_BUILD_DIR} ABSOLUTE
-  BASE_DIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}
-)
-
-message(STATUS "LLDB.framework: build path is '${framework_target_dir}'")
+message(STATUS "LLDB.framework: build path is '${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}'")
 message(STATUS "LLDB.framework: install path is '${LLDB_FRAMEWORK_INSTALL_DIR}'")
 message(STATUS "LLDB.framework: resources subdirectory is 'Versions/${LLDB_FRAMEWORK_VERSION}/Resources'")
 
@@ -15,7 +9,7 @@
 
   OUTPUT_NAME LLDB
   VERSION ${LLDB_VERSION}
-  LIBRARY_OUTPUT_DIRECTORY ${framework_target_dir}
+  LIBRARY_OUTPUT_DIRECTORY ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}
 
   # Compatibility version
   SOVERSION "1.0.0"
@@ -29,8 +23,8 @@
 # Used in llvm_add_library() to set default output directories for multi-config
 # generators. Overwrite to account for special framework output directory.
 set_output_directory(liblldb
-  BINARY_DIR ${framework_target_dir}
-  LIBRARY_DIR ${framework_target_dir}
+  BINARY_DIR ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}
+  LIBRARY_DIR ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}
 )
 
 lldb_add_post_install_steps_darwin(liblldb ${LLDB_FRAMEWORK_INSTALL_DIR})
@@ -51,7 +45,7 @@
 add_custom_command(TARGET liblldb POST_BUILD
   COMMAND ${CMAKE_COMMAND} -E create_symlink
   Versions/Current/Headers
-  ${framework_target_dir}/LLDB.framework/Headers
+  ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework/Headers
   COMMENT "LLDB.framework: create Headers symlink"
 )

[Lldb-commits] [PATCH] D69612: [lldb-vscod] fix build with NDEBUG on windows

2019-10-30 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

It ended up being a separate commit (2d1a0dfe 
) because 
I don't know how to use git. :P As far as change attribution is concerned, that 
may be even for the better. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69612



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

[Lldb-commits] [PATCH] D69503: [LLDB] [Windows] Don't crash if the debugged process is unable to start up

2019-10-30 Thread Adrian McCarthy via Phabricator via lldb-commits

amccarth added a comment.

In D69503#1726841 , @labath wrote:

> (Ideally, I'd just delete ProcessWindows entirely, and move everything to 
> lldb-server based model -- it's much easier to reason about threads there)

I believe there are people working on moving the Windows bits to the 
lldb-server model, but I don't think it's far enough along to get rid of 
ProcessWindows just yet.

The `m_mutex` is supposed to protect `m_session_data`.   `OnExitProcess` says:

  // No need to acquire the lock since m_session_data isn't accessed.

but it actually does access m_session_data.

So I think you've identified the cause of the race condition, but I'm not sure 
whether this is the best fix.  I need a little time to look at this more 
closely.

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69503

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

[Lldb-commits] [lldb] 5e029c4 - DebugServer: be more lenient about the target triple

2019-10-30 Thread Saleem Abdulrasool via lldb-commits


Author: Saleem Abdulrasool
Date: 2019-10-30T12:08:29-07:00
New Revision: 5e029c4cfd7b8db269b6db475ecd420311fbd7d1

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

LOG: DebugServer: be more lenient about the target triple

When building standalone, `LLVM_DEFAULT_TARGET_TRIPLE` may be undefined.
Matching against an empty string does not work as desired in CMake, so,
fallback to the old behaviour, defaulting `LLDB_DEBUGSERVER_ARCH` to
`CMAKE_OSX_ARCHITECTURES`.

Added: 


Modified: 
lldb/tools/debugserver/source/MacOSX/CMakeLists.txt

Removed: 




diff  --git a/lldb/tools/debugserver/source/MacOSX/CMakeLists.txt 
b/lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
index 59812b27dff2..cf08985ed6f2 100644
--- a/lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
+++ b/lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
@@ -10,7 +10,11 @@
 # CFLAGS etc explicitly. Switching on LLVM_HOST_TRIPLE is also an option,
 # but it breaks down when cross-compiling.
 
-string(REGEX MATCH "^[^-]*" LLDB_DEBUGSERVER_ARCH 
"${LLVM_DEFAULT_TARGET_TRIPLE}")
+if(LLVM_DEFAULT_TARGET_TRIPLE)
+  string(REGEX MATCH "^[^-]*" LLDB_DEBUGSERVER_ARCH 
${LLVM_DEFAULT_TARGET_TRIPLE})
+else()
+  set(LLDB_DEBUGSERVER_ARCH ${CMAKE_OSX_ARCHITECTURES})
+endif()
 
 if("${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*arm.*")
   list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)



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

[Lldb-commits] [PATCH] D69619: [lldb/lit] Introduce %clang_host substitutions

2019-10-30 Thread Adrian Prantl via Phabricator via lldb-commits

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

> The one barrier we have right now is that it is not possible to run the "lit" 
> suite with a different compiler, and that's something I think we should keep.

Very much agreed.

> For the test which don't need a host executable, I don't think that running 
> them for each available target would be useful, as they generally don't care 
> about those details.

That's plausible.

Thanks for sharing your thoughts!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69619



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

[Lldb-commits] [PATCH] D69619: [lldb/lit] Introduce %clang_host substitutions

2019-10-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.

All my thoughts have already been brought up and discussed. :-)

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69619



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

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

2019-10-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Let me know if you want me to commit this for you.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68868



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

[Lldb-commits] [PATCH] D69630: [lldb] Record framework build path and use it everywhere

2019-10-30 Thread Haibo Huang via Phabricator via lldb-commits

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

Fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69630

Files:
  lldb/CMakeLists.txt
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/cmake/modules/LLDBFramework.cmake
  lldb/test/API/CMakeLists.txt
  lldb/tools/debugserver/source/CMakeLists.txt
  lldb/tools/driver/CMakeLists.txt
  lldb/tools/lldb-vscode/CMakeLists.txt

Index: lldb/tools/lldb-vscode/CMakeLists.txt
===
--- lldb/tools/lldb-vscode/CMakeLists.txt
+++ lldb/tools/lldb-vscode/CMakeLists.txt
@@ -33,10 +33,9 @@
 if(LLDB_BUILD_FRAMEWORK)
   # In the build-tree, we know the exact path to the framework directory.
   # The installed framework can be in different locations.
-  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
   lldb_setup_rpaths(lldb-vscode
 BUILD_RPATH
-  "${framework_build_dir}"
+  "${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}"
 INSTALL_RPATH
   "@loader_path/../../../SharedFrameworks"
   "@loader_path/../../System/Library/PrivateFrameworks"
Index: lldb/tools/driver/CMakeLists.txt
===
--- lldb/tools/driver/CMakeLists.txt
+++ lldb/tools/driver/CMakeLists.txt
@@ -33,10 +33,9 @@
 if(LLDB_BUILD_FRAMEWORK)
   # In the build-tree, we know the exact path to the framework directory.
   # The installed framework can be in different locations.
-  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
   lldb_setup_rpaths(lldb
 BUILD_RPATH
-  "${framework_build_dir}"
+  "${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}"
 INSTALL_RPATH
   "@loader_path/../../../SharedFrameworks"
   "@loader_path/../../System/Library/PrivateFrameworks"
Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -219,8 +219,7 @@
 set(pass_entitlements --entitlements ${entitlements})
   endif()
 
-  get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
-  set(copy_location ${framework_build_dir}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Resources/debugserver)
+  set(copy_location ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework/Versions/${LLDB_FRAMEWORK_VERSION}/Resources/debugserver)
 
   add_custom_command(TARGET debugserver POST_BUILD
 COMMAND ${CMAKE_COMMAND} -E
Index: lldb/test/API/CMakeLists.txt
===
--- lldb/test/API/CMakeLists.txt
+++ lldb/test/API/CMakeLists.txt
@@ -107,8 +107,7 @@
 
 if(CMAKE_HOST_APPLE)
   if(LLDB_BUILD_FRAMEWORK)
-get_target_property(framework_build_dir liblldb LIBRARY_OUTPUT_DIRECTORY)
-list(APPEND LLDB_TEST_COMMON_ARGS --framework ${framework_build_dir}/LLDB.framework)
+list(APPEND LLDB_TEST_COMMON_ARGS --framework ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework)
   endif()
 
   # Use the same identity for testing
Index: lldb/cmake/modules/LLDBFramework.cmake
===
--- lldb/cmake/modules/LLDBFramework.cmake
+++ lldb/cmake/modules/LLDBFramework.cmake
@@ -1,10 +1,4 @@
-# Path relative to the root binary directory
-get_filename_component(
-  framework_target_dir ${LLDB_FRAMEWORK_BUILD_DIR} ABSOLUTE
-  BASE_DIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}
-)
-
-message(STATUS "LLDB.framework: build path is '${framework_target_dir}'")
+message(STATUS "LLDB.framework: build path is '${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}'")
 message(STATUS "LLDB.framework: install path is '${LLDB_FRAMEWORK_INSTALL_DIR}'")
 message(STATUS "LLDB.framework: resources subdirectory is 'Versions/${LLDB_FRAMEWORK_VERSION}/Resources'")
 
@@ -15,7 +9,7 @@
 
   OUTPUT_NAME LLDB
   VERSION ${LLDB_VERSION}
-  LIBRARY_OUTPUT_DIRECTORY ${framework_target_dir}
+  LIBRARY_OUTPUT_DIRECTORY ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}
 
   # Compatibility version
   SOVERSION "1.0.0"
@@ -29,8 +23,8 @@
 # Used in llvm_add_library() to set default output directories for multi-config
 # generators. Overwrite to account for special framework output directory.
 set_output_directory(liblldb
-  BINARY_DIR ${framework_target_dir}
-  LIBRARY_DIR ${framework_target_dir}
+  BINARY_DIR ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}
+  LIBRARY_DIR ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}
 )
 
 lldb_add_post_install_steps_darwin(liblldb ${LLDB_FRAMEWORK_INSTALL_DIR})
@@ -51,7 +45,7 @@
 add_custom_command(TARGET liblldb POST_BUILD
   COMMAND ${CMAKE_COMMAND} -E create_symlink
   Versions/Current/Headers
-  ${framework_target_dir}/LLDB.framework/Headers
+  ${LLDB_FRAMEWORK_ABSOLUTE_BUILD_DIR}/LLDB.framework/Headers
   COMMENT "LLDB.framework: create Headers symlink"
 )
 
Index: lldb/cma

[Lldb-commits] [PATCH] D69641: [Symbol] Change ClangASTContext::GetCXXClassName return type

2019-10-30 Thread Alex Langford via Phabricator via lldb-commits

xiaobai created this revision.
xiaobai added reviewers: JDevlieghere, labath, compnerd.
Herald added a project: LLDB.

Instead of filling out a std::string and returning a bool to indicate
success, returning a std::string directly and testing to see if it's
empty seems like a cleaner solution overall.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69641

Files:
  lldb/include/lldb/Symbol/ClangASTContext.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Symbol/ClangASTContext.cpp


Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -3850,20 +3850,19 @@
   return ClangASTContextSupportsLanguage(language);
 }
 
-bool ClangASTContext::GetCXXClassName(const CompilerType &type,
-  std::string &class_name) {
-  if (type) {
-clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
-if (!qual_type.isNull()) {
-  clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
-  if (cxx_record_decl) {
-class_name.assign(cxx_record_decl->getIdentifier()->getNameStart());
-return true;
-  }
-}
-  }
-  class_name.clear();
-  return false;
+std::string ClangASTContext::GetCXXClassName(const CompilerType &type) {
+  if (!type)
+return std::string();
+
+  clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
+  if (qual_type.isNull())
+return std::string();
+
+  clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
+  if (!cxx_record_decl)
+return std::string();
+
+  return cxx_record_decl->getIdentifier()->getNameStart();
 }
 
 bool ClangASTContext::IsCXXClassType(const CompilerType &type) {
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2023,15 +2023,14 @@
 bool parent_had_base_class =
 GetParent() && GetParent()->GetBaseClassPath(s);
 CompilerType compiler_type = GetCompilerType();
-std::string cxx_class_name;
-bool this_had_base_class =
-ClangASTContext::GetCXXClassName(compiler_type, cxx_class_name);
-if (this_had_base_class) {
+std::string cxx_class_name =
+ClangASTContext::GetCXXClassName(compiler_type);
+if (!cxx_class_name.empty()) {
   if (parent_had_base_class)
 s.PutCString("::");
   s.PutCString(cxx_class_name);
 }
-return parent_had_base_class || this_had_base_class;
+return parent_had_base_class || !cxx_class_name.empty();
   }
   return false;
 }
Index: lldb/include/lldb/Symbol/ClangASTContext.h
===
--- lldb/include/lldb/Symbol/ClangASTContext.h
+++ lldb/include/lldb/Symbol/ClangASTContext.h
@@ -601,8 +601,7 @@
 
   bool SupportsLanguage(lldb::LanguageType language) override;
 
-  static bool GetCXXClassName(const CompilerType &type,
-  std::string &class_name);
+  static std::string GetCXXClassName(const CompilerType &type);
 
   // Type Completion
 


Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -3850,20 +3850,19 @@
   return ClangASTContextSupportsLanguage(language);
 }
 
-bool ClangASTContext::GetCXXClassName(const CompilerType &type,
-  std::string &class_name) {
-  if (type) {
-clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
-if (!qual_type.isNull()) {
-  clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
-  if (cxx_record_decl) {
-class_name.assign(cxx_record_decl->getIdentifier()->getNameStart());
-return true;
-  }
-}
-  }
-  class_name.clear();
-  return false;
+std::string ClangASTContext::GetCXXClassName(const CompilerType &type) {
+  if (!type)
+return std::string();
+
+  clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
+  if (qual_type.isNull())
+return std::string();
+
+  clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
+  if (!cxx_record_decl)
+return std::string();
+
+  return cxx_record_decl->getIdentifier()->getNameStart();
 }
 
 bool ClangASTContext::IsCXXClassType(const CompilerType &type) {
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2023,15 +2023,14 @@
 bool parent_had_base_class =
 GetParent() && GetParent()->GetBaseClassPath(s);
 CompilerType compiler_type = GetCompilerType();
-std::string cxx_class_name;
-bool this_had_base_class =
-ClangASTContext::GetCXXClassName(compiler_type,

[Lldb-commits] [PATCH] D69641: [Symbol] Change ClangASTContext::GetCXXClassName return type

2019-10-30 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

Could this return an `Optional` or `Expected`? It's 
not clear from a `std::string` return value that the method can fail and it 
will return an empty string on error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69641



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

[Lldb-commits] [PATCH] D69641: [Symbol] Change ClangASTContext::GetCXXClassName return type

2019-10-30 Thread Alex Langford via Phabricator via lldb-commits

xiaobai added a comment.

In D69641#1727787 , @teemperor wrote:

> Could this return an `Optional` or `Expected`? It's 
> not clear from a `std::string` return value that the method can fail and it 
> will return an empty string on error.


That sounds like a better idea to me. Will do that instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69641



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

[Lldb-commits] [PATCH] D69502: [LLDB] [PECOFF] Don't crash in ReadImageDataByRVA for addresses out of range

2019-10-30 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo updated this revision to Diff 227183.
mstorsjo added a comment.

Added a testcase based on @labath 's patch. (Thanks! That managed to trigger 
the condition!)


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

https://reviews.llvm.org/D69502

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/test/Shell/Minidump/Windows/Inputs/broken-unwind.dmp.yaml
  lldb/test/Shell/Minidump/Windows/Inputs/broken-unwind.exe.yaml
  lldb/test/Shell/Minidump/Windows/broken-unwind.test

Index: lldb/test/Shell/Minidump/Windows/broken-unwind.test
===
--- /dev/null
+++ lldb/test/Shell/Minidump/Windows/broken-unwind.test
@@ -0,0 +1,7 @@
+Test that we can cope with broken unwind information that suggests
+reading out of bounds.
+
+RUN: yaml2obj %S/Inputs/broken-unwind.exe.yaml > %T/broken-unwind.exe
+RUN: yaml2obj %S/Inputs/broken-unwind.dmp.yaml > %T/broken-unwind.dmp
+RUN: %lldb -O "settings set target.exec-search-paths %T" \
+RUN:   -c %T/broken-unwind.dmp -o "image show-unwind -a 0xb1000" -o exit
Index: lldb/test/Shell/Minidump/Windows/Inputs/broken-unwind.exe.yaml
===
--- /dev/null
+++ lldb/test/Shell/Minidump/Windows/Inputs/broken-unwind.exe.yaml
@@ -0,0 +1,84 @@
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4224
+  ImageBase:   4194304
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_NO_SEH, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  ExportTable: 
+RelativeVirtualAddress: 8327
+Size:90
+  ImportTable: 
+RelativeVirtualAddress: 0
+Size:0
+  ResourceTable:   
+RelativeVirtualAddress: 0
+Size:0
+  ExceptionTable:  
+RelativeVirtualAddress: 12303
+Size:12
+  CertificateTable: 
+RelativeVirtualAddress: 0
+Size:0
+  BaseRelocationTable: 
+RelativeVirtualAddress: 0
+Size:0
+  Debug:   
+RelativeVirtualAddress: 8192
+Size:28
+  Architecture:
+RelativeVirtualAddress: 0
+Size:0
+  GlobalPtr:   
+RelativeVirtualAddress: 0
+Size:0
+  TlsTable:
+RelativeVirtualAddress: 0
+Size:0
+  LoadConfigTable: 
+RelativeVirtualAddress: 0
+Size:0
+  BoundImport: 
+RelativeVirtualAddress: 0
+Size:0
+  IAT: 
+RelativeVirtualAddress: 0
+Size:0
+  DelayImportDescriptor: 
+RelativeVirtualAddress: 0
+Size:0
+  ClrRuntimeHeader: 
+RelativeVirtualAddress: 0
+Size:0
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 22
+SectionData: 50894C24048B4C24040FAF4C2404890C248B042459C3
+  - Name:.rdata
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 236
+SectionData: A565B65C02006B001C201C0652534453E092B2141AD8F1B44C4C44205044422E0100443A5C757073747265616D5C6275696C645C746F6F6C735C6C6C64625C6C69745C4D6F64756C65735C5045434F46465C4F75747075745C6578706F72742D646C6C66756E632E79616D6C2E746D702E70646200AF2002000100CB20D320D7206578706F72742D646C6C66756E632E79616D6C2E746D702E646C6C10D9200100446C6C46756E63010101000102
+  - Name:.pdata
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  12288
+VirtualSize: 12
+SectionData: '00101610E420'
+symbols: []
+...
Index: lldb/test/Shell/Minidump/Windows/Inputs/broken-unwind.dmp.yaml
===
--- /dev/null
+++ lldb/test/Shell/Minidump/Windows/Inputs/broken-unwind.dmp.yaml
@@ -0,0 +1,35 @@
+--- !minidump
+Streams:
+  - Type:ModuleList
+Modules:
+  - Base of Image:   0x000B
+Size of Image:   0x5000
+Module Name: 'find-module.exe'
+CodeView Record: 52534453E092B2141AD8F1B44C4C44205044422E0100433A5C70726F6A656374735C746573745F6170705C436F6E736F6C654170706C6

[Lldb-commits] [PATCH] D69555: [zorg] Fix LLDBCmakeBuildFactory

2019-10-30 Thread Stella Stamenova via Phabricator via lldb-commits

stella.stamenova added a comment.

Sadly, I think this broke the windows bot now (assuming it got applied to 
staging, otherwise I have to figure out what did):

http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/46/steps/cmake-configure/logs/stdio


Repository:
  rZORG LLVM Github Zorg

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

https://reviews.llvm.org/D69555



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

[Lldb-commits] [PATCH] D69646: [LLDB] [PECOFF] Fix error handling for executables that object::createBinary error out on

2019-10-30 Thread Martin Storsjö via Phabricator via lldb-commits

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

llvm::object::createBinary returns an Expected<>, which requires not only 
checking the object for success, but also requires consuming the Error, if one 
was set.

For another similar existing case, move consuming of the Error object to a 
standalone std::string variable. If the Error only is consumed within a 
LLDB_LOGF() statement, it actually doesn't get consumed unless that log channel 
is enabled.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D69646

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml
@@ -0,0 +1,80 @@
+## Test that errors in loading an exe doesn't crash lldb.
+
+# RUN: yaml2obj %s > %t.exe
+# RUN: %lldb %t.exe 2>&1 | FileCheck %s
+
+# CHECK: error: '{{.*}}' doesn't contain any {{.*}} platform architectures
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4096
+  ImageBase:   1073741824
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  ExportTable:
+RelativeVirtualAddress: 12345678
+Size:100
+  ImportTable:
+RelativeVirtualAddress: 0
+Size:0
+  ResourceTable:
+RelativeVirtualAddress: 0
+Size:0
+  ExceptionTable:
+RelativeVirtualAddress: 0
+Size:0
+  CertificateTable:
+RelativeVirtualAddress: 0
+Size:0
+  BaseRelocationTable:
+RelativeVirtualAddress: 0
+Size:0
+  Debug:
+RelativeVirtualAddress: 0
+Size:0
+  Architecture:
+RelativeVirtualAddress: 0
+Size:0
+  GlobalPtr:
+RelativeVirtualAddress: 0
+Size:0
+  TlsTable:
+RelativeVirtualAddress: 0
+Size:0
+  LoadConfigTable:
+RelativeVirtualAddress: 0
+Size:0
+  BoundImport:
+RelativeVirtualAddress: 0
+Size:0
+  IAT:
+RelativeVirtualAddress: 0
+Size:0
+  DelayImportDescriptor:
+RelativeVirtualAddress: 0
+Size:0
+  ClrRuntimeHeader:
+RelativeVirtualAddress: 0
+Size:0
+header:
+  Machine: IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 1
+SectionData: C3
+symbols: []
+...
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -167,9 +167,18 @@
   if (!data_sp || !ObjectFilePECOFF::MagicBytesMatch(data_sp))
 return initial_count;
 
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
+
   auto binary = llvm::object::createBinary(file.GetPath());
-  if (!binary)
+
+  if (!binary) {
+std::string msg = errorToErrorCode(binary.takeError()).message();
+LLDB_LOGF(log,
+  "ObjectFilePECOFF::GetModuleSpecifications() - failed to create binary "
+  "for file (%s): %s",
+  file ? file.GetPath().c_str() : "", msg.c_str());
 return initial_count;
+  }
 
   if (!binary->getBinary()->isCOFF() &&
   !binary->getBinary()->isCOFFImportFile())
@@ -242,11 +251,11 @@
 
   auto binary = llvm::object::createBinary(m_file.GetPath());
   if (!binary) {
+std::string msg = errorToErrorCode(binary.takeError()).message();
 LLDB_LOGF(log,
   "ObjectFilePECOFF::CreateBinary() - failed to create binary "
   "for file (%s): %s",
-  m_file ? m_file.GetPath().c_str() : "",
-  errorToErrorCode(binary.takeError()).message().c_str());
+  m_file ? m_file.GetPath().c_str() : "", msg.c_str());
 return false;
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69641: [Symbol] Change ClangASTContext::GetCXXClassName return type

2019-10-30 Thread Alex Langford via Phabricator via lldb-commits

xiaobai updated this revision to Diff 227188.
xiaobai added a comment.

Use llvm::Optional


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69641

Files:
  lldb/include/lldb/Symbol/ClangASTContext.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Symbol/ClangASTContext.cpp


Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -3850,20 +3850,20 @@
   return ClangASTContextSupportsLanguage(language);
 }
 
-bool ClangASTContext::GetCXXClassName(const CompilerType &type,
-  std::string &class_name) {
-  if (type) {
-clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
-if (!qual_type.isNull()) {
-  clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
-  if (cxx_record_decl) {
-class_name.assign(cxx_record_decl->getIdentifier()->getNameStart());
-return true;
-  }
-}
-  }
-  class_name.clear();
-  return false;
+Optional
+ClangASTContext::GetCXXClassName(const CompilerType &type) {
+  if (!type)
+return llvm::None;
+
+  clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
+  if (qual_type.isNull())
+return llvm::None;
+
+  clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
+  if (!cxx_record_decl)
+return llvm::None;
+
+  return std::string(cxx_record_decl->getIdentifier()->getNameStart());
 }
 
 bool ClangASTContext::IsCXXClassType(const CompilerType &type) {
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2023,15 +2023,14 @@
 bool parent_had_base_class =
 GetParent() && GetParent()->GetBaseClassPath(s);
 CompilerType compiler_type = GetCompilerType();
-std::string cxx_class_name;
-bool this_had_base_class =
-ClangASTContext::GetCXXClassName(compiler_type, cxx_class_name);
-if (this_had_base_class) {
+llvm::Optional cxx_class_name =
+ClangASTContext::GetCXXClassName(compiler_type);
+if (cxx_class_name) {
   if (parent_had_base_class)
 s.PutCString("::");
-  s.PutCString(cxx_class_name);
+  s.PutCString(cxx_class_name.getValue());
 }
-return parent_had_base_class || this_had_base_class;
+return parent_had_base_class || cxx_class_name;
   }
   return false;
 }
Index: lldb/include/lldb/Symbol/ClangASTContext.h
===
--- lldb/include/lldb/Symbol/ClangASTContext.h
+++ lldb/include/lldb/Symbol/ClangASTContext.h
@@ -601,8 +601,7 @@
 
   bool SupportsLanguage(lldb::LanguageType language) override;
 
-  static bool GetCXXClassName(const CompilerType &type,
-  std::string &class_name);
+  static llvm::Optional GetCXXClassName(const CompilerType &type);
 
   // Type Completion
 


Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -3850,20 +3850,20 @@
   return ClangASTContextSupportsLanguage(language);
 }
 
-bool ClangASTContext::GetCXXClassName(const CompilerType &type,
-  std::string &class_name) {
-  if (type) {
-clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
-if (!qual_type.isNull()) {
-  clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
-  if (cxx_record_decl) {
-class_name.assign(cxx_record_decl->getIdentifier()->getNameStart());
-return true;
-  }
-}
-  }
-  class_name.clear();
-  return false;
+Optional
+ClangASTContext::GetCXXClassName(const CompilerType &type) {
+  if (!type)
+return llvm::None;
+
+  clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
+  if (qual_type.isNull())
+return llvm::None;
+
+  clang::CXXRecordDecl *cxx_record_decl = qual_type->getAsCXXRecordDecl();
+  if (!cxx_record_decl)
+return llvm::None;
+
+  return std::string(cxx_record_decl->getIdentifier()->getNameStart());
 }
 
 bool ClangASTContext::IsCXXClassType(const CompilerType &type) {
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2023,15 +2023,14 @@
 bool parent_had_base_class =
 GetParent() && GetParent()->GetBaseClassPath(s);
 CompilerType compiler_type = GetCompilerType();
-std::string cxx_class_name;
-bool this_had_base_class =
-ClangASTContext::GetCXXClassName(compiler_type, cxx_class_name);
-if (this_had_base_class) {
+llvm::Optional cxx_class_name =
+ClangASTCon

[Lldb-commits] [PATCH] D69641: [Symbol] Change ClangASTContext::GetCXXClassName return type

2019-10-30 Thread Raphael Isemann via Phabricator via lldb-commits

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69641



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

[Lldb-commits] [lldb] a925974 - Run clang-format on lldb/source/Commands (NFC)

2019-10-30 Thread Adrian Prantl via lldb-commits


Author: Adrian Prantl
Date: 2019-10-30T16:03:00-07:00
New Revision: a925974bf166a83cfd45b0ca89c5c65d17f275cc

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

LOG: Run clang-format on lldb/source/Commands (NFC)

These files had a lot of whitespace errors in them which was a
constant source of merge conflicts downstream.

Added: 


Modified: 
lldb/source/Commands/CommandObjectBreakpoint.cpp
lldb/source/Commands/CommandObjectBreakpoint.h
lldb/source/Commands/CommandObjectBreakpointCommand.cpp
lldb/source/Commands/CommandObjectBreakpointCommand.h
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Commands/CommandObjectDisassemble.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectHelp.cpp
lldb/source/Commands/CommandObjectLanguage.h
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Commands/CommandObjectPlugin.h
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/CommandObjectSettings.cpp
lldb/source/Commands/CommandObjectSource.cpp
lldb/source/Commands/CommandObjectStats.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Commands/CommandObjectType.cpp
lldb/source/Commands/CommandObjectType.h
lldb/source/Commands/CommandObjectWatchpoint.h
lldb/source/Commands/CommandObjectWatchpointCommand.h

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp 
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index 042fc06a2bf0..799066e07a29 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -48,13 +48,10 @@ static void AddBreakpointDescription(Stream *s, Breakpoint 
*bp,
 #define LLDB_OPTIONS_breakpoint_modify
 #include "CommandOptions.inc"
 
-class lldb_private::BreakpointOptionGroup : public OptionGroup
-{
+class lldb_private::BreakpointOptionGroup : public OptionGroup {
 public:
-  BreakpointOptionGroup() :
-  OptionGroup(),
-  m_bp_opts(false) {}
-  
+  BreakpointOptionGroup() : OptionGroup(), m_bp_opts(false) {}
+
   ~BreakpointOptionGroup() override = default;
 
   llvm::ArrayRef GetDefinitions() override {
@@ -62,9 +59,10 @@ class lldb_private::BreakpointOptionGroup : public 
OptionGroup
   }
 
   Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
-  ExecutionContext *execution_context) override {
+ExecutionContext *execution_context) override {
 Status error;
-const int short_option = 
g_breakpoint_modify_options[option_idx].short_option;
+const int short_option =
+g_breakpoint_modify_options[option_idx].short_option;
 
 switch (short_option) {
 case 'c':
@@ -91,18 +89,15 @@ class lldb_private::BreakpointOptionGroup : public 
OptionGroup
 error.SetErrorStringWithFormat(
 "invalid boolean value '%s' passed for -G option",
 option_arg.str().c_str());
-}
-break;
-case 'i':
-{
+} break;
+case 'i': {
   uint32_t ignore_count;
   if (option_arg.getAsInteger(0, ignore_count))
 error.SetErrorStringWithFormat("invalid ignore count '%s'",
option_arg.str().c_str());
   else
 m_bp_opts.SetIgnoreCount(ignore_count);
-}
-break;
+} break;
 case 'o': {
   bool value, success;
   value = OptionArgParser::ToBoolean(option_arg, false, &success);
@@ -113,8 +108,7 @@ class lldb_private::BreakpointOptionGroup : public 
OptionGroup
 "invalid boolean value '%s' passed for -o option",
 option_arg.str().c_str());
 } break;
-case 't':
-{
+case 't': {
   lldb::tid_t thread_id = LLDB_INVALID_THREAD_ID;
   if (option_arg[0] != '\0') {
 if (option_arg.getAsInteger(0, thread_id))
@@ -122,16 +116,14 @@ class lldb_private::BreakpointOptionGroup : public 
OptionGroup
  option_arg.str().c_str());
   }
   m_bp_opts.SetThreadID(thread_id);
-}
-break;
+} break;
 case 'T':
   m_bp_opts.GetThreadSpec()->SetName(option_arg.str().c_str());
   break;
 case 'q':
   m_bp_opts.GetThreadSpec()->SetQueueName(option_arg.str().c_str());
   break;
-case 'x':
-{
+case 'x': {
   uint32_t thread_index = UINT32_MAX;
   if (option_arg[0] != '\n') {
 if (option_arg.getAsInteger(0, thread_index))
@@ -139,8 +131,7 @@ class lldb_private::BreakpointOptionGroup : public 
OptionGroup
  option_arg.str().c

[Lldb-commits] [PATCH] D69503: [LLDB] [Windows] Don't crash if the debugged process is unable to start up

2019-10-30 Thread Adrian McCarthy via Phabricator via lldb-commits

amccarth requested changes to this revision.
amccarth added a comment.
This revision now requires changes to proceed.

Thanks for identifying this race condition.

After some poking around, I think there's a cleaner way to address it.

First, `m_session_state`'s lifetime is currently managed mostly by 
`ProcessDebugger` (base class) but for one exception in `ProcessWindows` 
(derived class).  There doesn't seem to be a good reason for that 
inconsistency, so my first step was to eliminate it and make `ProcessDebugger` 
responsible for its lifetime in all cases. This is done by moving the 
`m_session_state.reset()` to `ProcessDebugger::OnExitProcess` and having 
`ProcessWindows::OnExitProcess` call the other.  This has the nice additional 
benefit of eliminating some duplicate code and comments.

But I'm not sure we need to clear `m_session_state` even in 
`ProcessDebugger::OnExitProcess`.  There are two places where `m_session_state` 
is created:  `ProcessDebugger::AttachProcess` and 
`ProcessDebugger::LaunchProcess`.  We could put `m_session_state.reset()`s in 
those functions, where it handles failure of `WaitForDebuggerConnection`.  But 
I'm not even sure if _that_ is necessary.

By adding a virtual destructor to the base class, it seems everything cleans up 
naturally before starting the next debugging session.

Here's what it would look like:  https://reviews.llvm.org/P8168

(If I'm wrong, and it _is_ important for us to clear the session state 
explicitly, then we'd need to find a place after `OnExitProcess`.  I don't 
think such a hook exists right now, so perhaps we'd have to create one.  That 
hook could be the one place we reset `m_session_state` for both the successful 
and unsuccessful debug sessions.)

FYI:  I'll be offline until November 4.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69503



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

[Lldb-commits] [PATCH] D69555: [zorg] Fix LLDBCmakeBuildFactory

2019-10-30 Thread Galina via Phabricator via lldb-commits

gkistanova added a comment.

The cmake command redefinition effectively is not supported any more, but the 
'cmake' arg is left behind.
+ inline comments.

These have been addressed by https://reviews.llvm.org/rZORG425eeb1bf21b.




Comment at: zorg/buildbot/builders/LLDBBuilder.py:110
 
-cmake_cmd = [
-"cmake", "-G", "Ninja", os.path.join(os.pardir, f.monorepo_dir, 
"llvm"),
+path = os.path.join(os.pardir, f.monorepo_dir, "llvm")
+cmake_options = [

This would build a path per notations on master. Which could be different than 
what a builder expects. Think of master running on Windows, for example.

Better use a "/" path separator as these days it seems supported by all 
platforms we care about.



Comment at: zorg/buildbot/builders/LLDBBuilder.py:130
+   doStepIf=FileDoesNotExist(
+"./%s/CMakeCache.txt" % build_dir)))
 

It is better to always run the cmake configure step. It doesn't take long but 
handles some dependencies.


Repository:
  rZORG LLVM Github Zorg

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

https://reviews.llvm.org/D69555



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

[Lldb-commits] [PATCH] D69589: [lldb] Refactor all POST_BUILD commands into targets

2019-10-30 Thread Haibo Huang via Phabricator via lldb-commits

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

When creating symlink, make the target depends on relative target path. So that 
if the target file doesn't exist, the build will fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69589

Files:
  lldb/CMakeLists.txt
  lldb/scripts/CMakeLists.txt

Index: lldb/scripts/CMakeLists.txt
===
--- lldb/scripts/CMakeLists.txt
+++ lldb/scripts/CMakeLists.txt
@@ -54,4 +54,5 @@
   ${CMAKE_CURRENT_BINARY_DIR}/LLDBWrapPython.cpp
   ${CMAKE_CURRENT_BINARY_DIR}/lldb.py
 )
+set_target_properties(swig_wrapper PROPERTIES FOLDER "lldb misc")
 
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -103,77 +103,105 @@
 
   # Add a Post-Build Event to copy over Python files and create the symlink
   # to liblldb.so for the Python API(hardlink on Windows).
-  add_custom_target(finish_swig ALL VERBATIM
-COMMAND ${CMAKE_COMMAND} -E make_directory ${lldb_python_build_path}
-DEPENDS ${lldb_scripts_dir}/lldb.py
-COMMENT "Python script sym-linking LLDB Python API")
+  add_custom_target(lldb_python_packages ALL
+COMMENT "Copy over Python files and create symlinks for LLDB Python API.")
+  set_target_properties(lldb_python_packages PROPERTIES FOLDER "lldb misc")
+
+  function(add_copy_file_target Name)
+cmake_parse_arguments(ARG "" "DEST_DIR;DEST_FILE_NAME" "FILES" ${ARGN})
+add_custom_command(OUTPUT ${ARG_DEST_DIR} VERBATIM
+  COMMAND ${CMAKE_COMMAND} -E make_directory ${ARG_DEST_DIR})
+foreach(src_file ${ARG_FILES})
+  if(ARG_DEST_FILE_NAME)
+set(file_name ${ARG_DEST_FILE_NAME})
+  else()
+get_filename_component(file_name ${src_file} NAME)
+  endif()
+  set(dest_file ${ARG_DEST_DIR}/${file_name})
+  list(APPEND DEST_FILES ${dest_file})
+  add_custom_command(OUTPUT ${dest_file} VERBATIM
+COMMAND ${CMAKE_COMMAND} -E copy ${src_file} ${dest_file}
+DEPENDS ${ARG_DEST_DIR} ${src_file})
+endforeach()
+add_custom_target(${Name} DEPENDS ${DEST_FILES} ${ARG_DEST_DIR})
+  endfunction()
 
   if(NOT LLDB_USE_SYSTEM_SIX)
-add_custom_command(TARGET finish_swig POST_BUILD VERBATIM
-  COMMAND ${CMAKE_COMMAND} -E copy
-"${LLDB_SOURCE_DIR}/third_party/Python/module/six/six.py"
-"${lldb_python_build_path}/../six.py")
+add_copy_file_target(lldb_python_six
+  FILES"${LLDB_SOURCE_DIR}/third_party/Python/module/six/six.py"
+  DEST_DIR "${lldb_python_build_path}/..")
+add_dependencies(lldb_python_packages lldb_python_six)
   endif()
 
-  add_custom_command(TARGET finish_swig POST_BUILD VERBATIM
-COMMAND ${CMAKE_COMMAND} -E copy
-  "${lldb_scripts_dir}/lldb.py"
-  "${lldb_python_build_path}/__init__.py")
+  add_copy_file_target(lldb_python_init
+FILES  "${lldb_scripts_dir}/lldb.py"
+DEST_DIR   "${lldb_python_build_path}"
+DEST_FILE_NAME "__init__.py")
+  add_dependencies(lldb_python_packages lldb_python_init)
 
   if(APPLE)
-SET(lldb_python_heap_dir "${lldb_python_build_path}/macosx/heap")
-add_custom_command(TARGET finish_swig POST_BUILD VERBATIM
-  COMMAND ${CMAKE_COMMAND} -E make_directory ${lldb_python_heap_dir}
-  COMMAND ${CMAKE_COMMAND} -E copy
-"${LLDB_SOURCE_DIR}/examples/darwin/heap_find/heap/heap_find.cpp"
-"${LLDB_SOURCE_DIR}/examples/darwin/heap_find/heap/Makefile"
-${lldb_python_heap_dir})
+add_copy_file_target(lldb_python_heap
+  FILES"${LLDB_SOURCE_DIR}/examples/darwin/heap_find/heap/heap_find.cpp"
+   "${LLDB_SOURCE_DIR}/examples/darwin/heap_find/heap/Makefile"
+  DEST_DIR "${lldb_python_build_path}/macosx/heap")
+add_dependencies(lldb_python_packages lldb_python_heap)
   endif()
 
-  function(create_python_package target pkg_dir)
+  add_copy_file_target(lldb_python_embeded_interpreter
+FILES"${LLDB_SOURCE_DIR}/source/Interpreter/embedded_interpreter.py"
+DEST_DIR "${lldb_python_build_path}")
+  add_dependencies(lldb_python_packages lldb_python_embeded_interpreter)
+
+  function(add_lldb_python_package_target Name PKG_DIR)
 cmake_parse_arguments(ARG "" "" "FILES" ${ARGN})
-if(ARG_FILES)
-  set(copy_cmd COMMAND ${CMAKE_COMMAND} -E copy ${ARG_FILES} ${pkg_dir})
-endif()
-add_custom_command(TARGET ${target} POST_BUILD VERBATIM
-  COMMAND ${CMAKE_COMMAND} -E make_directory ${pkg_dir}
-  ${copy_cmd}
+set(ABS_PKG_DIR "${lldb_python_build_path}/${PKG_DIR}")
+add_copy_file_target("${Name}_srcs" FILES ${ARG_FILES} DEST_DIR ${ABS_PKG_DIR})
+add_custom_command(OUTPUT "${ABS_PKG_DIR}/__init__.py" VERBATIM
   COMMAND ${PYTHON_EXECUTABLE} "${LLDB_SOURCE_DIR}/scripts/Python/createPythonInit.py"
-${pkg_dir} ${ARG_FILES}
-  WORKING_DIRECTORY ${lldb

[Lldb-commits] [lldb] 29d5e27 - Only ask once if we have no commands. NFC.

2019-10-30 Thread Jim Ingham via lldb-commits


Author: Jim Ingham
Date: 2019-10-30T18:04:03-07:00
New Revision: 29d5e275f28723b3b36b00e91b535d776f5aa281

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

LOG: Only ask once if we have no commands.  NFC.

Added: 


Modified: 
lldb/source/Commands/CommandObjectBreakpoint.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp 
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index 799066e07a29..5d0cc3d9dcea 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -146,15 +146,13 @@ class lldb_private::BreakpointOptionGroup : public 
OptionGroup {
 
   Status OptionParsingFinished(ExecutionContext *execution_context) override {
 if (!m_commands.empty()) {
-  if (!m_commands.empty()) {
-auto cmd_data = std::make_unique();
+  auto cmd_data = std::make_unique();
 
-for (std::string &str : m_commands)
-  cmd_data->user_source.AppendString(str);
+  for (std::string &str : m_commands)
+cmd_data->user_source.AppendString(str);
 
-cmd_data->stop_on_error = true;
-m_bp_opts.SetCommandDataCallback(cmd_data);
-  }
+  cmd_data->stop_on_error = true;
+  m_bp_opts.SetCommandDataCallback(cmd_data);
 }
 return Status();
   }



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

[Lldb-commits] [PATCH] D69230: RFC: specialized Optional for T that can represent its own invalid state

2019-10-30 Thread David Blaikie via Phabricator via lldb-commits

dblaikie added a comment.

In D69230#1727209 , @labath wrote:

> BTW, I was bored on the plane, so I created this proof of concept F10589216: 
> dense.cc . It needs a lot of cleanup of 
> course, but it demonstrates one way to generically store some extra data with 
> some type. It gives each type the option to define a bunch of unused (the 
> patch calls them "free") values (which can be useful for for Optional<>s, 
> DenseMaps, etc.). Also each type can say that it is able to store some extra 
> unrelated bits (like PointerIntPair does), and with a bit of care, one can 
> even nest these types arbitrarily). the main thing I haven't yet figured out 
> is the relationship with DenseMapInfo. Theoretically DenseMapInfo could be 
> implemented on top of DenseInfo, but I am still wondering whether we could 
> not make that a single type (trait) somehow...

Neat - yeah, I'd certainly like to see something along those lines, including 
unification with/replacement of DenseMapInfo (though DenseMapInfo needs other 
things like hashing, so I'm not sure we'd necessarily want to replace them - I 
guess it could be a trait class with "optional" features - no reason an 
implementation necessarily has to have all 4 functions of DenseMapInfo - but 
maybe separating it would be better to express intent (DenseMapInfo builds on 
DenseInfo and adds the requirement for hashing and equality)).

I guess there's an open question to me as to whether DenseOptional should just 
be the only optional we use, and it could fallback to non-dense when the type 
didn't provide some extra states - or should it be like DenseMap where you just 
can't use it and have to use std::map or similar once your type doesn't have 
extra states? I don't know that I feel super strongyl either way - I don't 
think we have lots of hypergeneric code that wants to DenseOptional over 
some unknown T that might have spare bits of state and T that might not.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69230

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

59 matches

Mail list logo