[Lldb-commits] [PATCH] D69468: [LLDB][breakpoints] ArgInfo::count -> ArgInfo::max_positional_args

[Pavel Labath via Phabricator via lldb-commits]

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

Looks fine, though I'd try to remove the `extern "C"` thingy to avoid the need 
to supress warnings and stuff...




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:76-79
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+
+extern "C" llvm::Expected LLDBSwigPythonBreakpointCallbackFunction(

How sure are we that these functions need to be `extern "C"` ? AFAICT, the only 
requirement is that they match the declarations in ScriptInterpreterPython.cpp. 
That can be easily achieved by just removing `extern "C"` from both 
declarations.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2283
+[&](PythonException &E) {
+  debugger.GetErrorStream() << E.ReadBacktrace();
+},

Random idea: Should we make `PythonException::message()` include the backtrace?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69468



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added subscribers: aleksandr.urakov, amccarth, ted, labath.
labath added a comment.

Yeah, I'd still like to avoid having multiple paths for configuring python, as 
one of them is bound to end up broken sooner or later. However, given that 
there's already talk about bumping the minimum cmake version, maybe we could do 
an lldb-specific bump first?

Though I am generally against aggressive version bumps, this python+windows 
situation is sufficiently messy that I think the bump may be worth it. 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 ?

[+ other windows stakeholders: @amccarth, @ted, @aleksandr.urakov ]




Comment at: lldb/cmake/modules/LLDBConfig.cmake:225
+  if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.13 AND CMAKE_SYSTEM_NAME STREQUAL 
Windows)
+find_package(Python3 COMPONENTS Interpreter Development REQUIRED)
+if(Python3_VERSION VERSION_LESS 3.5)

What if I use a single-config generator and build a release configuration. Is 
the "development" thingy still required?


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] D69488: [LLDB][Python] fix another fflush issue on NetBSD

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Ok, let's get this thing fixed. We can continue the discussion on the other 
patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69488



___
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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

So, if I understand correctly. The problem here is the final call to `fflush`, 
which can end up referencing a closed FILE*. Can we just not call `fflush` 
then? 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, and given how FDs are recycled, it's very likely that this 
dangling object will end up writing to a random other open file -- that may be 
even worse than crashing. If we're not happy with that requirement then we can 
make `GetFile()` return a real python object, which will hold onto the 
lldb_private::File instance and handle these use-after-free cases in a reliable 
manner.


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] D62931: [lldb-server] Add setting to force 'g' packet use

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D62931#1724433 , @guiandrade wrote:

> In D62931#1719948 , @labath wrote:
>
> > I'm sorry, this dropped off my radar. The code looks fine, but it could use 
> > some more tests. For instance, one test when you set the setting value to 
> > the non-default , and then check that lldb does _not_ use the `g` packet .
>
>
> Yeah, more tests would be useful. They made me notice an issue btw. I was 
> simply sending a 'g' packet and checking if the server replied back nicely; 
> however, IIUC with 'G' packets it isn't so simple because we also need to 
> send the registers data in the same packet. I'm not sure how I could do that, 
> and I'm also afraid that check could get too expensive. Do you have any idea 
> what could be a nice way to handle that?


Well... I'd expect that a stub which does not support the `G` packet at all 
would return an "unsupported" response no matter what data you send it, whereas 
a stub which supports the `G` packet would return an "error" response to an 
incorrect `G` packet. But yeah, testing for the availability of packets which 
mutate the state is a bit tricky...

In D62931#1724451 , @jasonmolenda 
wrote:

> fwiw I can't imagine a stub that supports g but not G.


well.. lldb-server is in that bucket right now. :) However, one could 
definitely argue that this behavior is wrong and we shouldn't support that.  
However, that still leaves the question of what exactly should the 
`use-g-packet` setting do. In the previous version of this patch, it controlled 
both `g` and `G`. I think that was mostly an accident, because it was 
implemented by hooking into the code which was trying to support stubs which 
did not understand the `p`/`P` packet. But as the reason we're introducing it 
is performance, I think it'd make sense to separate these out, because writing 
is a much less common operation, and I expect wanting to write _all_ registers 
would be very rare. OTOH, wanting to _read_ all registers is not that uncommon, 
particularly as our stubs now expedite the most common registers, which means 
that the fact that one is attempting to read one of the non-expedited registers 
is a pretty good indication that he's going to read more than one.

So, one possibility would be to just have one setting 
(use-g-packet-for-reading), but make it so that it does not control the usage 
of the `G` packet -- the logic for that could stay as `if (p_supported) 
send_P() else send_G()`, while for reading we'd have something like `if 
(p_supported && !g_requested) send_p() else send_g();`.


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

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: jankratochvil, stella.stamenova, gkistanova.

The attempt in D69341  was not complete, as in 
the monorepo world, we
also need to pass LLVM_ENABLE_PROJECTS in order to build lldb (and
clang and lld).

This also updates the code to use the CmakeCommand class instead of the
generic ShellCommand.


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
@@ -11,6 +11,7 @@
 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.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
@@ -11,6 +11,7 @@
 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.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=Proper

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

[Jan Kratochvil via Phabricator via lldb-commits]

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

  2019-10-29 11:38:04+0100 [-] error while parsing config file
  2019-10-29 11:38:04+0100 [-] Unhandled Error
Traceback (most recent call last):
  File 
"/home/buildbot/.local/lib/python2.7/site-packages/buildbot-latest-py2.7.egg/buildbot/master.py",
 line 197, in loadTheConfigFile
d = self.loadConfig(f)
  File 
"/home/buildbot/.local/lib/python2.7/site-packages/buildbot-latest-py2.7.egg/buildbot/master.py",
 line 579, in loadConfig
d.addCallback(do_load)
  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", 
line 322, in addCallback
callbackKeywords=kw)
  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", 
line 311, in addCallbacks
self._runCallbacks()
---  ---
  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", 
line 654, in _runCallbacks
current.result = callback(current.result, *args, **kw)
  File 
"/home/buildbot/.local/lib/python2.7/site-packages/buildbot-latest-py2.7.egg/buildbot/master.py",
 line 226, in do_load
exec f in localDict
  File 
"/quad/home/buildbot/zorg-git/buildbot/osuosl/master/master.cfg", line 83, in 

c['builders'] = builders = list(config.builders.get_builders())
  File 
"/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 
1456, in get_builders
for b in _get_lldb_builders():
  File 
"/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 
829, in _get_lldb_builders
'-DLLVM_LIT_ARGS="-v"'])},
  File 
"/quad/home/buildbot/zorg-git/buildbot/osuosl/master/zorg/buildbot/builders/LLDBBuilder.py",
 line 129, in getLLDBCMakeBuildFactory
doStepIf=FileDoesNotExist(
exceptions.NameError: global name 'FileDoesNotExist' is not defined

  File 
"/quad/home/buildbot/zorg-git/buildbot/osuosl/master/master.cfg", line 83, in 

c['builders'] = builders = list(config.builders.get_builders())
  File 
"/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 
1456, in get_builders
for b in _get_lldb_builders():
  File 
"/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 
829, in _get_lldb_builders
'-DLLVM_LIT_ARGS="-v"'])},
  File 
"/quad/home/buildbot/zorg-git/buildbot/osuosl/master/zorg/buildbot/builders/LLDBBuilder.py",
 line 129, in getLLDBCMakeBuildFactory
doStepIf=FileDoesNotExist(
exceptions.NameError: global name 'FileDoesNotExist' is not defined


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] D68130: [lldb] Don't emit artificial constructor declarations as global functions

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Whoops, I meant skipIf'ing, not x-failing.

Anyway, I can reproduce the SIBABRT fail on my Arch Linux CI, so it seems that 
Stella's ubuntu failure isn't at fault here but instead some other issue. If I 
disable unwind-on-error I get this:

  lldb version 10.0.0 (g...@github.com:Teemperor/llvm-project revision 
94df949a1f120014e9c715fc43d87398ed3a880a)
clang revision 94df949a1f120014e9c715fc43d87398ed3a880a
llvm revision 94df949a1f120014e9c715fc43d87398ed3a880a
  warning: (x86_64) 
/home/teemperor/work/ci/build/lldb-test-build.noindex/commands/expression/call-overridden-method/TestCallOverriddenMethod.test_call_on_temporary_dwarf/a.out
 0x7fff0101: DW_AT_specification(0x00d5) has no decl
  inlinable function call in a function with debug info must have a !dbg 
location
call void @_ZN4BaseC2Ev(%class.Base* %this1) #4
  inlinable function call in a function with debug info must have a !dbg 
location
call void @_ZN4BaseC2Ev(%class.Base* %this1) #4
  LLVM ERROR: Broken module found, compilation aborted!


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] D69555: [zorg] Fix LLDBCmakeBuildFactory

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 226872.
labath added a comment.

import FileDoesNotExist


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=True,
+   options=cmake_options,
+   path="../%s" % f.llvm_sr

[Lldb-commits] [PATCH] D69366: [LLDB] [PECOFF] Use FindSectionByID to associate symbols to sections

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4394b5bee615: [LLDB] [PECOFF] Use FindSectionByID to 
associate symbols to sections (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69366

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

Index: lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml
@@ -0,0 +1,116 @@
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test symbols %t | FileCheck %s
+
+# CHECK: Type File Address/Value {{.*}} SizeFlags   Name
+# CHECK: Code 0x400010000x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} entry
+# CHECK:  0x400020000x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} variable
+
+--- !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: 0
+Size:0
+  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
+  - Name:.data
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+VirtualAddress:  8192
+VirtualSize: 4
+SectionData: ''
+symbols:
+  - Name:.text
+Value:   0
+SectionNumber:   1
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_NULL
+StorageClass:IMAGE_SYM_CLASS_STATIC
+  - Name:.data
+Value:   0
+SectionNumber:   2
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_NULL
+StorageClass:IMAGE_SYM_CLASS_STATIC
+  - Name:.bss
+Value:   0
+SectionNumber:   2
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_NULL
+StorageClass:IMAGE_SYM_CLASS_STATIC
+  - Name:entry
+Value:   0
+SectionNumber:   1
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_FUNCTION
+StorageClass:IMAGE_SYM_CLASS_EXTERNAL
+  - Name:variable
+Value:   0
+SectionNumber:   2
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_NULL
+StorageClass:IMAGE_SYM_CLASS_EXTERNAL
+...
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -685,7 +685,7 @@
 symbol.naux = symtab_data.GetU8(&offset);
 symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
 if ((int16_t)symbol.sect >= 1) {
-  Address symbol_addr(sect_list->GetSectionAtIndex(symbol.sect - 1),
+  Address symbol_addr(sect_list->FindSectionByID(symbol.sect),
   symbol.value);

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

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...


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] D69502: [LLDB] [PECOFF] Don't crash in ReadImageDataByRVA for addresses out of range

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

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.


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] D69555: [zorg] Fix LLDBCmakeBuildFactory

[Jan Kratochvil via Phabricator via lldb-commits]

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

It does run now, I did not test it with a slave.


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] [lldb] 3011c7e - [lldb][NFC] Make LLVMUserExpression::DoExecute return early

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-10-29T15:38:35+01:00
New Revision: 3011c7eb31c58526066841e84e7f0a6b9b733b57

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

LOG: [lldb][NFC] Make LLVMUserExpression::DoExecute return early

The giant if-else isn't conforming to LLVM code style.

Added: 


Modified: 
lldb/source/Expression/LLVMUserExpression.cpp

Removed: 




diff  --git a/lldb/source/Expression/LLVMUserExpression.cpp 
b/lldb/source/Expression/LLVMUserExpression.cpp
index 99e0c11df420..ee72e7ce6322 100644
--- a/lldb/source/Expression/LLVMUserExpression.cpp
+++ b/lldb/source/Expression/LLVMUserExpression.cpp
@@ -70,174 +70,172 @@ LLVMUserExpression::DoExecute(DiagnosticManager 
&diagnostic_manager,
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EXPRESSIONS |
   LIBLLDB_LOG_STEP));
 
-  if (m_jit_start_addr != LLDB_INVALID_ADDRESS || m_can_interpret) {
-lldb::addr_t struct_address = LLDB_INVALID_ADDRESS;
+  if (m_jit_start_addr == LLDB_INVALID_ADDRESS && !m_can_interpret) {
+diagnostic_manager.PutString(
+eDiagnosticSeverityError,
+"Expression can't be run, because there is no JIT compiled function");
+return lldb::eExpressionSetupError;
+  }
 
-if (!PrepareToExecuteJITExpression(diagnostic_manager, exe_ctx,
-   struct_address)) {
-  diagnostic_manager.Printf(
+  lldb::addr_t struct_address = LLDB_INVALID_ADDRESS;
+
+  if (!PrepareToExecuteJITExpression(diagnostic_manager, exe_ctx,
+ struct_address)) {
+diagnostic_manager.Printf(
+eDiagnosticSeverityError,
+"errored out in %s, couldn't PrepareToExecuteJITExpression",
+__FUNCTION__);
+return lldb::eExpressionSetupError;
+  }
+
+  lldb::addr_t function_stack_bottom = LLDB_INVALID_ADDRESS;
+  lldb::addr_t function_stack_top = LLDB_INVALID_ADDRESS;
+
+  if (m_can_interpret) {
+llvm::Module *module = m_execution_unit_sp->GetModule();
+llvm::Function *function = m_execution_unit_sp->GetFunction();
+
+if (!module || !function) {
+  diagnostic_manager.PutString(
   eDiagnosticSeverityError,
-  "errored out in %s, couldn't PrepareToExecuteJITExpression",
-  __FUNCTION__);
+  "supposed to interpret, but nothing is there");
   return lldb::eExpressionSetupError;
 }
 
-lldb::addr_t function_stack_bottom = LLDB_INVALID_ADDRESS;
-lldb::addr_t function_stack_top = LLDB_INVALID_ADDRESS;
+Status interpreter_error;
 
-if (m_can_interpret) {
-  llvm::Module *module = m_execution_unit_sp->GetModule();
-  llvm::Function *function = m_execution_unit_sp->GetFunction();
+std::vector args;
 
-  if (!module || !function) {
-diagnostic_manager.PutString(
-eDiagnosticSeverityError,
-"supposed to interpret, but nothing is there");
-return lldb::eExpressionSetupError;
-  }
+if (!AddArguments(exe_ctx, args, struct_address, diagnostic_manager)) {
+  diagnostic_manager.Printf(eDiagnosticSeverityError,
+"errored out in %s, couldn't AddArguments",
+__FUNCTION__);
+  return lldb::eExpressionSetupError;
+}
 
-  Status interpreter_error;
+function_stack_bottom = m_stack_frame_bottom;
+function_stack_top = m_stack_frame_top;
 
-  std::vector args;
+IRInterpreter::Interpret(*module, *function, args, *m_execution_unit_sp,
+ interpreter_error, function_stack_bottom,
+ function_stack_top, exe_ctx);
 
-  if (!AddArguments(exe_ctx, args, struct_address, diagnostic_manager)) {
-diagnostic_manager.Printf(eDiagnosticSeverityError,
-  "errored out in %s, couldn't AddArguments",
-  __FUNCTION__);
-return lldb::eExpressionSetupError;
-  }
+if (!interpreter_error.Success()) {
+  diagnostic_manager.Printf(eDiagnosticSeverityError,
+"supposed to interpret, but failed: %s",
+interpreter_error.AsCString());
+  return lldb::eExpressionDiscarded;
+}
+  } else {
+if (!exe_ctx.HasThreadScope()) {
+  diagnostic_manager.Printf(eDiagnosticSeverityError,
+"%s called with no thread selected",
+__FUNCTION__);
+  return lldb::eExpressionSetupError;
+}
 
-  function_stack_bottom = m_stack_frame_bottom;
-  function_stack_top = m_stack_frame_top;
+Address wrapper_address(m_jit_start_addr);
 
-  IRInterpreter::Interpret(*module, *function, args, *m_execut

[Lldb-commits] [lldb] 55eec2b - build: workaround stale caches (NFC)

[Saleem Abdulrasool via lldb-commits]

Author: Saleem Abdulrasool
Date: 2019-10-29T08:20:58-07:00
New Revision: 55eec2ba96bd9c19ccb5d4d13cb8c88d4abcebc6

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

LOG: build: workaround stale caches (NFC)

`LLVM_DEFAULT_TARGET_TRIPLE` is a cached variable, which means that it
may actually be unset.  Furthermore, in standalone builds, the variable
may be fully undefined.  Apply the regular expression over the empty
string in such a case.  This should improve the state of the green
dragon bot.

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 6d8e1ee449e3..59812b27dff2 100644
--- a/lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
+++ b/lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
@@ -10,7 +10,7 @@
 # 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})
+string(REGEX MATCH "^[^-]*" LLDB_DEBUGSERVER_ARCH 
"${LLVM_DEFAULT_TARGET_TRIPLE}")
 
 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] [lldb] e56ba37 - build: make standalone builds work again

[Saleem Abdulrasool via lldb-commits]

Author: Saleem Abdulrasool
Date: 2019-10-29T08:24:10-07:00
New Revision: e56ba3743bcc344c51be9d919c32ec8f88ddef44

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

LOG: build: make standalone builds work again

Apple's greendragon bot uses a standalone build of lldb which would
fail to build after a recent change to LLVM as it relied on LLVM setting
global flags for its build.  Attempt to repair the standalone build for
greendragon bot.

Added: 


Modified: 
lldb/CMakeLists.txt

Removed: 




diff  --git a/lldb/CMakeLists.txt b/lldb/CMakeLists.txt
index a5e505c67a13..511c75d76588 100644
--- a/lldb/CMakeLists.txt
+++ b/lldb/CMakeLists.txt
@@ -16,6 +16,10 @@ set(CMAKE_MODULE_PATH
 if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   project(lldb)
   include(LLDBStandalone)
+
+  set(CMAKE_CXX_STANDARD 14 CACHE STRING "C++ standard to conform to")
+  set(CMAKE_CXX_STANDARD_REQUIRED YES)
+  set(CMAKE_CXX_EXTENSIONS NO)
 endif()
 
 include(LLDBConfig)



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

[Lldb-commits] [lldb] 6a93a12 - [LLDB][Python] fix another fflush issue on NetBSD

[Lawrence D'Anna via lldb-commits]

Author: Lawrence D'Anna
Date: 2019-10-29T09:41:22-07:00
New Revision: 6a93a12a8dd98291225a282b5b8f3c97e68ebe49

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

LOG: [LLDB][Python] fix another fflush issue on NetBSD

Summary:
Here's another instance where we were calling fflush on an input
stream, which is illegal on NetBSD.

Reviewers: labath, mgorny

Reviewed By: mgorny

Subscribers: krytarowski, lldb-commits

Tags: #lldb

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

Added: 


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

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 5a75187262ab..f7f1ad08500a 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
@@ -845,11 +845,16 @@ def test_back_and_forth(self):
 def i(sbf):
 for i in range(10):
 f = sbf.GetFile()
+self.assertEqual(f.mode, "w")
 yield f
 sbf = lldb.SBFile.Create(f, borrow=True)
 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/Host/common/File.cpp 
b/lldb/source/Host/common/File.cpp
index 9dae24d766f6..7850222376f7 100644
--- a/lldb/source/Host/common/File.cpp
+++ b/lldb/source/Host/common/File.cpp
@@ -310,7 +310,7 @@ Status NativeFile::Close() {
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
-} else {
+} else if (m_options & eOpenOptionWrite) {
   if (::fflush(m_stream) == EOF)
 error.SetErrorToErrno();
 }

diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
index 2b85ebf18c6a..df8bac951fc4 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@ Expected PythonFile::FromFile(File &file, 
const char *mode) {
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // 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; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h 
b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
index 373d3212697d..302901664ec0 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@ inline llvm::Error keyError() {
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@ class PythonObject {
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API decla

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

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova accepted this revision.
stella.stamenova added a comment.

Thanks @labath


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] D69488: [LLDB][Python] fix another fflush issue on NetBSD

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a93a12a8dd9: [LLDB][Python] fix another fflush issue on 
NetBSD (authored by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69488

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


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // 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; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -310,7 +310,7 @@
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
-} else {
+} else if (m_options & eOpenOptionWrite) {
   if (::fflush(m_stream) == EOF)
 error.SetErrorToErrno();
 }
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
@@ -845,11 +845,16 @@
 def i(sbf):
 for i in range(10):
 f = sbf.GetFile()
+self.assertEqual(f.mode, "w")
 yield f
 sbf = lldb.SBFile.Create(f, borrow=True)
 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.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const c

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

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

This is broken on Windows. I moved the Buildbot to staging to resolve some of 
the issues with the move to the monorepo, so you can see the failures here:

http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/0/steps/test/logs/stdio


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68961



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

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

[Shafik Yaghmour via lldb-commits]

Stella,

Thank you for the heads up, I am looking at it now.

-Shafik

> On Oct 29, 2019, at 11:04 AM, Stella Stamenova via Phabricator 
>  wrote:
> 
> stella.stamenova added a comment.
> 
> This is broken on Windows. I moved the Buildbot to staging to resolve some of 
> the issues with the move to the monorepo, so you can see the failures here:
> 
> http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/0/steps/test/logs/stdio
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D68961/new/
> 
> https://reviews.llvm.org/D68961
> 
> 
> 

___
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

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

lawrence_danna added a comment.

In D69532#1724806 , @labath wrote:

> So, if I understand correctly. The problem here is the final call to 
> `fflush`, which can end up referencing a closed FILE*. Can we just not call 
> `fflush` then?


Hrm, maybe.  I'll try it and see if anything breaks.

> 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.

The other aspect here is that it is reasonable to ask a python programmer not 
to use a closed file.I don't  think it is ever safe to ask a python 
programmer to ensure that reference counts get decremented in a particular 
order.


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] [lldb] e658178 - [LLDB] Fix for windows bots broken by unsupported tests

[via lldb-commits]

Author: shafik
Date: 2019-10-29T11:33:11-07:00
New Revision: e6581783f767b7dcaf84223aeae05d2467106113

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

LOG: [LLDB] Fix for windows bots broken by unsupported tests

Added: 


Modified: 
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

Removed: 




diff  --git 
a/lldb/test/Shell/SymbolFile/DWARF/anon_class_w_and_wo_export_symbols.ll 
b/lldb/test/Shell/SymbolFile/DWARF/anon_class_w_and_wo_export_symbols.ll
index b0a9f2295057..aab0128264c1 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/anon_class_w_and_wo_export_symbols.ll
+++ b/lldb/test/Shell/SymbolFile/DWARF/anon_class_w_and_wo_export_symbols.ll
@@ -1,3 +1,5 @@
+; UNSUPPORTED: system-windows
+;
 ; This test verifies that we do the right thing with DIFlagExportSymbols which 
is the new
 ; behavioir and without the DIFlagExportSymbols which is the old behavior for 
the given
 ; definitions below.

diff  --git 
a/lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
 
b/lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
index f1254fe3b1b5..5e80c5c6d0b7 100644
--- 
a/lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
+++ 
b/lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
@@ -1,3 +1,5 @@
+// UNSUPPORTED: system-windows
+//
 // Test to verify we are corectly generating anonymous flags when parsing
 // anonymous class and unnamed structs from DWARF to the a clang AST node.
 



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

[Lldb-commits] [lldb] d46c655 - [ValueObjectDisplay] Generalize the description of an option.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2019-10-29T13:05:56-07:00
New Revision: d46c65592e3ac6a78c54514e4919d505c1f0c74a

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

LOG: [ValueObjectDisplay] Generalize the description of an option.

Added: 


Modified: 
lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp 
b/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp
index 81c10a6c762e..da0437ac299c 100644
--- a/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp
+++ b/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp
@@ -41,7 +41,7 @@ static const OptionDefinition g_option_table[] = {
  {}, 0, eArgTypeNone, "Show variable location information."},
 {LLDB_OPT_SET_1, false, "object-description", 'O',
  OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone,
- "Print as an Objective-C object."},
+ "Display using a language-specific description API, if possible."},
 {LLDB_OPT_SET_1, false, "ptr-depth", 'P', OptionParser::eRequiredArgument,
  nullptr, {}, 0, eArgTypeCount, "The number of pointers to be traversed "
 "when dumping values (default is zero)."},



___
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

[Guilherme Andrade via Phabricator via lldb-commits]

guiandrade updated this revision to Diff 226958.
guiandrade edited the summary of this revision.
guiandrade added a comment.

Moving to a single config, use-g-packet-for-reading, that forces the use of 'g' 
packets for reading, but does not force 'G' for writing. The latter only ends 
up being used if 'p' packets aren't supported (it assumes that 'P' also will 
not).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62931

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -301,13 +301,14 @@
 if (process_sp) {
   ProcessGDBRemote *gdb_process =
   static_cast(process_sp.get());
-  // read_all_registers_at_once will be true if 'p' packet is not
-  // supported.
+  bool pSupported =
+  gdb_process->GetGDBRemote().GetpPacketSupported(GetID());
   bool read_all_registers_at_once =
-  !gdb_process->GetGDBRemote().GetpPacketSupported(GetID());
+  !pSupported || gdb_process->m_use_g_packet_for_reading;
+  bool write_all_registers_at_once = !pSupported;
   reg_ctx_sp = std::make_shared(
   *this, concrete_frame_idx, gdb_process->m_register_info,
-  read_all_registers_at_once);
+  read_all_registers_at_once, write_all_registers_at_once);
 }
   } else {
 Unwind *unwinder = GetUnwinder();
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
@@ -13,4 +13,8 @@
 Global,
 DefaultFalse,
 Desc<"If true, the libraries-svr4 feature will be used to get a hold of the process's loaded modules.">;
+  def UseGPacketForReading: Property<"use-g-packet-for-reading", "Boolean">,
+Global,
+DefaultTrue,
+Desc<"Specify if the server should use 'g' packets to read registers.">;
 }
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -284,6 +284,7 @@
   lldb::CommandObjectSP m_command_sp;
   int64_t m_breakpoint_pc_offset;
   lldb::tid_t m_initial_tid; // The initial thread ID, given by stub on attach
+  bool m_use_g_packet_for_reading;
 
   bool m_replay_mode;
   bool m_allow_flash_writes;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -154,6 +154,11 @@
 nullptr, idx,
 g_processgdbremote_properties[idx].default_uint_value != 0);
   }
+
+  bool GetUseGPacketForReading() const {
+const uint32_t idx = ePropertyUseGPacketForReading;
+return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, true);
+  }
 };
 
 typedef std::shared_ptr ProcessKDPPropertiesSP;
@@ -309,6 +314,9 @@
   GetGlobalPluginProperties()->GetPacketTimeout();
   if (timeout_seconds > 0)
 m_gdb_comm.SetPacketTimeout(std::chrono::seconds(timeout_seconds));
+
+  m_use_g_packet_for_reading =
+  GetGlobalPluginProperties()->GetUseGPacketForReading();
 }
 
 // Destructor
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -41,7 +41,7 @@
 public:
   GDBRemoteRegisterContext(ThreadGDBRemote &thread, uint32_t concrete_frame_idx,
GDBRemoteDynamicRegisterInfo ®_info,
-   bool read_all_at_once);
+   bool read_all_at_once, bool write_all_at_once);
 
   ~GDBRemoteReg

[Lldb-commits] [PATCH] D69468: [LLDB][breakpoints] ArgInfo::count -> ArgInfo::max_positional_args

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

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



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:76-79
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+
+extern "C" llvm::Expected LLDBSwigPythonBreakpointCallbackFunction(

labath wrote:
> How sure are we that these functions need to be `extern "C"` ? AFAICT, the 
> only requirement is that they match the declarations in 
> ScriptInterpreterPython.cpp. That can be easily achieved by just removing 
> `extern "C"` from both declarations.
Looks like SWIG forces them to be extern "C". 



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2283
+[&](PythonException &E) {
+  debugger.GetErrorStream() << E.ReadBacktrace();
+},

labath wrote:
> Random idea: Should we make `PythonException::message()` include the 
> backtrace?
Huh, I don't know.  Maybe?   


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69468



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

[Lldb-commits] [lldb] a69bbe0 - [LLDB][breakpoints] ArgInfo::count -> ArgInfo::max_positional_args

[Lawrence D'Anna via lldb-commits]

Author: Lawrence D'Anna
Date: 2019-10-29T15:03:02-07:00
New Revision: a69bbe02a2352271e8b14542073f177e24c499c1

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

LOG: [LLDB][breakpoints] ArgInfo::count -> ArgInfo::max_positional_args

Summary:
Move breakpoints from the old, bad ArgInfo::count to the new, better
ArgInfo::max_positional_args.   Soon ArgInfo::count will be no more.

It looks like this functionality is already well tested by
`TestBreakpointCommandsFromPython.py`, so there's no need to write
additional tests for it.

Reviewers: labath, jingham, JDevlieghere

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


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

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommandsFromPython.py
lldb/scripts/Python/python-wrapper.swig
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h 
b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index 69af88091a40..b32962b80355 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -315,8 +315,7 @@ class ScriptInterpreter : public PluginInterface {
 
   Status SetBreakpointCommandCallbackFunction(
   std::vector &bp_options_vec,
-  const char *function_name, 
-  StructuredData::ObjectSP extra_args_sp);
+  const char *function_name, StructuredData::ObjectSP extra_args_sp);
 
   /// Set a script function as the callback for the breakpoint.
   virtual Status
@@ -472,9 +471,9 @@ class ScriptInterpreter : public PluginInterface {
   const char *GetScriptInterpreterPtyName();
 
   int GetMasterFileDescriptor();
-  
-  virtual llvm::Expected 
-  GetNumFixedArgumentsForCallable(const llvm::StringRef &callable_name) { 
+
+  virtual llvm::Expected
+  GetMaxPositionalArgumentsForCallable(const llvm::StringRef &callable_name) {
 return llvm::createStringError(
 llvm::inconvertibleErrorCode(), "Unimplemented function");
   }

diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommandsFromPython.py
 
b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommandsFromPython.py
index ccb61d79c403..15a31201c565 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommandsFromPython.py
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommandsFromPython.py
@@ -132,7 +132,7 @@ def do_set_python_command_from_python(self):
 # Now finish, and make sure the return value is correct.
 threads = lldbutil.get_threads_stopped_at_breakpoint(
 self.process, body_bkpt)
-self.assertTrue(len(threads) == 1, "Stopped at inner breakpoint.")
+self.assertEquals(len(threads), 1, "Stopped at inner breakpoint.")
 self.thread = threads[0]
 
 self.assertEquals("callback was here", side_effect.callback)

diff  --git a/lldb/scripts/Python/python-wrapper.swig 
b/lldb/scripts/Python/python-wrapper.swig
index 5e9a2ba1367c..71a958acb72c 100644
--- a/lldb/scripts/Python/python-wrapper.swig
+++ b/lldb/scripts/Python/python-wrapper.swig
@@ -39,7 +39,7 @@ private:
 // This function is called by 
lldb_private::ScriptInterpreterPython::BreakpointCallbackFunction(...)
 // and is used when a script command is attached to a breakpoint for execution.
 
-SWIGEXPORT bool
+SWIGEXPORT llvm::Expected
 LLDBSwigPythonBreakpointCallbackFunction
 (
 const char *python_function_name,
@@ -49,38 +49,40 @@ LLDBSwigPythonBreakpointCallbackFunction
 lldb_private::StructuredDataImpl *args_impl
 )
 {
+using namespace llvm;
+
 lldb::SBFrame sb_frame (frame_sp);
 lldb::SBBreakpointLocation sb_bp_loc(bp_loc_sp);
 
-bool stop_at_breakpoint = true;
-
 PyErr_Cleaner py_err_cleaner(true);
 auto dict = 
PythonModule::MainModule().ResolveName(session_dictionary_name);
 auto pfunc = 
PythonObject::ResolveNameWithDictionary(python_function_name, 
dict);
 
-if (!pfunc.IsAllocated())
-return stop_at_breakpoint;
+unsigned max_positional_args;
+if (auto arg_info = pfunc.GetArgInfo())
+max_positional_args = arg_info.get().max_positional_args;
+else
+return arg_info.takeError();
 
 PythonObject frame_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_frame));
 PythonOb

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

[Haibo Huang via Phabricator via lldb-commits]

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

This makes all dependencies correct.


Repository:
  rG LLVM Github Monorepo

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
@@ -99,77 +99,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_python_build_path})
+${PKG_DIR} ${ARG_FILES}
+  WORKING_DIRECTORY ${lldb_python_build_path}
+  DEPENDS "${Na

[Lldb-commits] [lldb] 8a82000 - [lldbsuite] Remove pre_kill_hook package

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-10-29T16:56:59-07:00
New Revision: 8a82000e486afe472519d288f2206399ada95aca

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

LOG: [lldbsuite] Remove pre_kill_hook package

This package was only used by dosep.py which has since been removed.

Added: 


Modified: 


Removed: 
lldb/packages/Python/lldbsuite/pre_kill_hook/README.md
lldb/packages/Python/lldbsuite/pre_kill_hook/__init__.py
lldb/packages/Python/lldbsuite/pre_kill_hook/darwin.py
lldb/packages/Python/lldbsuite/pre_kill_hook/linux.py
lldb/packages/Python/lldbsuite/pre_kill_hook/tests/__init__.py
lldb/packages/Python/lldbsuite/pre_kill_hook/tests/test_darwin.py
lldb/packages/Python/lldbsuite/pre_kill_hook/tests/test_linux.py



diff  --git a/lldb/packages/Python/lldbsuite/pre_kill_hook/README.md 
b/lldb/packages/Python/lldbsuite/pre_kill_hook/README.md
deleted file mode 100644
index 921eedb4a869..
--- a/lldb/packages/Python/lldbsuite/pre_kill_hook/README.md
+++ /dev/null
@@ -1,55 +0,0 @@
-# pre\_kill\_hook package
-
-## Overview
-
-The pre\_kill\_hook package provides a per-platform method for running code
-after a test process times out but before the concurrent test runner kills the
-timed-out process.
-
-## Detailed Description of Usage
-
-If a platform defines the hook, then the hook gets called right after a timeout
-is detected in a test run, but before the process is killed.
-
-The pre-kill-hook mechanism works as follows:
-
-* When a timeout is detected in the process_control.ProcessDriver class that
-  runs the per-test lldb process, a new overridable on\_timeout\_pre\_kill() 
method
-  is called on the ProcessDriver instance.
-
-* The concurrent test driver's derived ProcessDriver overrides this method. It
-  looks to see if a module called
-  "lldbsuite.pre\_kill\_hook.{platform-system-name}" module exists, where
-  platform-system-name is replaced with platform.system().lower().  (e.g.
-  "Darwin" becomes the darwin.py module).
-  
-  * If that module doesn't exist, the rest of the new behavior is skipped.
-
-  * If that module does exist, it is loaded, and the method
-"do\_pre\_kill(process\_id, context\_dict, output\_stream)" is called. If
-that method throws an exception, we log it and we ignore further processing
-of the pre-killed process.
-
-  * The process\_id argument of the do\_pre\_kill function is the process id as
-returned by the ProcessDriver.pid property.
-  
-  * The output\_stream argument of the do\_pre\_kill function takes a file-like
-object. Output to be collected from doing any processing on the
-process-to-be-killed should be written into the file-like object. The
-current impl uses a six.StringIO and then writes this output to
-{TestFilename}-{pid}.sample in the session directory.
-
-* Platforms where platform.system() is "Darwin" will get a pre-kill action that
-  runs the 'sample' program on the lldb that has timed out. That data will be
-  collected on CI and analyzed to determine what is happening during timeouts.
-  (This has an advantage over a core in that it is much smaller and that it
-  clearly demonstrates any liveness of the process, if there is any).
-
-## Running the tests
-
-To run the tests in the pre\_kill\_hook package, open a console, change into
-this directory and run the following:
-
-```
-python -m unittest discover
-```

diff  --git a/lldb/packages/Python/lldbsuite/pre_kill_hook/__init__.py 
b/lldb/packages/Python/lldbsuite/pre_kill_hook/__init__.py
deleted file mode 100644
index c3a852ea1bfe..
--- a/lldb/packages/Python/lldbsuite/pre_kill_hook/__init__.py
+++ /dev/null
@@ -1 +0,0 @@
-"""Initialize the package."""

diff  --git a/lldb/packages/Python/lldbsuite/pre_kill_hook/darwin.py 
b/lldb/packages/Python/lldbsuite/pre_kill_hook/darwin.py
deleted file mode 100644
index 2bee65a01e3f..
--- a/lldb/packages/Python/lldbsuite/pre_kill_hook/darwin.py
+++ /dev/null
@@ -1,46 +0,0 @@
-"""Provides a pre-kill method to run on macOS."""
-from __future__ import print_function
-
-# system imports
-import subprocess
-import sys
-
-# third-party module imports
-import six
-
-
-def do_pre_kill(process_id, runner_context, output_stream, sample_time=3):
-"""Samples the given process id, and puts the output to output_stream.
-
-@param process_id the local process to sample.
-
-@param runner_context a dictionary of details about the architectures
-and platform on which the given process is running.  Expected keys are
-archs (array of architectures), platform_name, platform_url, and
-platform_working_dir.
-
-@param output_stream file-like object that should be used to write the
-results of sampling.
-
-@param sample_t

[Lldb-commits] [lldb] 5cc2e06 - [lldbsuite] Remove unused support files

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-10-29T16:56:59-07:00
New Revision: 5cc2e0651fed7764f02421db1ba1719a10f17d25

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

LOG: [lldbsuite] Remove unused support files

To the best of my understanding these files or their content is nowhere
referenced.

Added: 


Modified: 


Removed: 
lldb/packages/Python/lldbsuite/support/fs.py
lldb/packages/Python/lldbsuite/support/sockutil.py



diff  --git a/lldb/packages/Python/lldbsuite/support/fs.py 
b/lldb/packages/Python/lldbsuite/support/fs.py
deleted file mode 100644
index 71394f03ac30..
--- a/lldb/packages/Python/lldbsuite/support/fs.py
+++ /dev/null
@@ -1,64 +0,0 @@
-"""
-Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-See https://llvm.org/LICENSE.txt for license information.
-SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-Prepares language bindings for LLDB build process.  Run with --help
-to see a description of the supported command line arguments.
-"""
-
-# Python modules:
-import os
-import platform
-import sys
-
-
-def _find_file_in_paths(paths, exe_basename):
-"""Returns the full exe path for the first path match.
-
-@params paths the list of directories to search for the exe_basename
-executable
-@params exe_basename the name of the file for which to search.
-e.g. "swig" or "swig.exe".
-
-@return the full path to the executable if found in one of the
-given paths; otherwise, returns None.
-"""
-for path in paths:
-trial_exe_path = os.path.join(path, exe_basename)
-if os.path.exists(trial_exe_path):
-return os.path.normcase(trial_exe_path)
-return None
-
-
-def find_executable(executable):
-"""Finds the specified executable in the PATH or known good locations."""
-
-# Figure out what we're looking for.
-if platform.system() == "Windows":
-executable = executable + ".exe"
-extra_dirs = []
-else:
-extra_dirs = ["/usr/local/bin"]
-
-# Figure out what paths to check.
-path_env = os.environ.get("PATH", None)
-if path_env is not None:
-paths_to_check = path_env.split(os.path.pathsep)
-else:
-paths_to_check = []
-
-# Add in the extra dirs
-paths_to_check.extend(extra_dirs)
-if len(paths_to_check) < 1:
-raise os.OSError(
-"executable was not specified, PATH has no "
-"contents, and there are no extra directories to search")
-
-result = _find_file_in_paths(paths_to_check, executable)
-
-if not result or len(result) < 1:
-raise os.OSError(
-"failed to find exe='%s' in paths='%s'" %
-(executable, paths_to_check))
-return result

diff  --git a/lldb/packages/Python/lldbsuite/support/sockutil.py 
b/lldb/packages/Python/lldbsuite/support/sockutil.py
deleted file mode 100644
index 56a9ed8bd79d..
--- a/lldb/packages/Python/lldbsuite/support/sockutil.py
+++ /dev/null
@@ -1,23 +0,0 @@
-"""
-Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-See https://llvm.org/LICENSE.txt for license information.
-SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-Helper functions for working with sockets.
-"""
-
-# Python modules:
-import io
-import socket
-
-# LLDB modules
-import use_lldb_suite
-
-
-def recvall(sock, size):
-bytes = io.BytesIO()
-while size > 0:
-this_result = sock.recv(size)
-bytes.write(this_result)
-size -= len(this_result)
-return bytes.getvalue()



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

[Lldb-commits] [PATCH] D69468: [LLDB][breakpoints] ArgInfo::count -> ArgInfo::max_positional_args

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa69bbe02a235: [LLDB][breakpoints] ArgInfo::count -> 
ArgInfo::max_positional_args (authored by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69468

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommandsFromPython.py
  lldb/scripts/Python/python-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -59,7 +59,7 @@
 #define LLDBSwigPyInit init_lldb
 #endif
 
-extern "C" bool LLDBSwigPythonBreakpointCallbackFunction(
+extern "C" llvm::Expected LLDBSwigPythonBreakpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
 const lldb::StackFrameSP &sb_frame,
 const lldb::BreakpointLocationSP &sb_bp_loc,
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -256,11 +256,10 @@
   BreakpointOptions *bp_options,
   std::unique_ptr &data_up) override;
 
-  Status SetBreakpointCommandCallback(
-  BreakpointOptions *bp_options, 
-   const char *command_body_text,
-   StructuredData::ObjectSP extra_args_sp,
-   bool uses_extra_args);
+  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  const char *command_body_text,
+  StructuredData::ObjectSP extra_args_sp,
+  bool uses_extra_args);
 
   /// Set a one-liner as the callback for the watchpoint.
   void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
@@ -378,10 +377,9 @@
   python::PythonDictionary &GetSessionDictionary();
 
   python::PythonDictionary &GetSysModuleDictionary();
-  
-  llvm::Expected 
-  GetNumFixedArgumentsForCallable(const llvm::StringRef &callable_name) 
-  override;
+
+  llvm::Expected GetMaxPositionalArgumentsForCallable(
+  const llvm::StringRef &callable_name) override;
 
   bool GetEmbeddedInterpreterModuleObjects();
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -72,11 +72,16 @@
 // These prototypes are the Pythonic implementations of the required callbacks.
 // Although these are scripting-language specific, their definition depends on
 // the public API.
-extern "C" bool LLDBSwigPythonBreakpointCallbackFunction(
+
+#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,
-const lldb::BreakpointLocationSP &sb_bp_loc,
-StructuredDataImpl *args_impl);
+const lldb::BreakpointLocationSP &sb_bp_loc, StructuredDataImpl *args_impl);
+
+#pragma clang diagnostic pop
 
 extern "C" bool LLDBSwigPythonWatchpointCallbackFunction(
 const char *python_function_name, const char *session_dictionary_name,
@@ -782,10 +787,9 @@
   return m_sys_module_dict;
 }
 
-llvm::Expected
-ScriptInterpreterPythonImpl::GetNumFixedArgumentsForCallable(
-const llvm::StringRef &callable_name)
-{
+llvm::Expected
+ScriptInterpreterPythonImpl::GetMaxPositionalArgumentsForCallable(
+const llvm::StringRef &callable_name) {
   if (callable_name.empty()) {
 return llvm::createStringError(
 llvm::inconvertibleErrorCode(),
@@ -796,16 +800,15 @@
  Locker::NoSTDIN);
   auto dict = PythonModule::MainModule()
   .ResolveName(m_dictionary_name);
-  auto pfunc 
- = PythonObject::ResolveNameWithDictionary(callable_name, 
-   dict);
+  auto pfunc = PythonObject::ResolveNameWithDictionary(
+  callable_name, dict);
   if (!pfunc.IsAllocated()) {
 return llvm::createStringError(
 llvm::inconvertibleErrorCode(),
 "can't find callable: %s", callable_name.str().c_str());
   }
   PythonCall

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

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

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

just don't even flush


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69532

Files:
  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/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)
___
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

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

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

fix the test too


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.
   file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
- py2

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

[Saleem Abdulrasool via Phabricator via lldb-commits]

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

Yeah, doing an incremental rollout makes sense.




Comment at: lldb/cmake/modules/LLDBConfig.cmake:225
+  if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.13 AND CMAKE_SYSTEM_NAME STREQUAL 
Windows)
+find_package(Python3 COMPONENTS Interpreter Development REQUIRED)
+if(Python3_VERSION VERSION_LESS 3.5)

labath wrote:
> What if I use a single-config generator and build a release configuration. Is 
> the "development" thingy still required?
Yes, it is - the component is the headers for building.


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] D69230: RFC: specialized Optional for T that can represent its own invalid state

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

lawrence_danna abandoned this revision.
lawrence_danna added a comment.

thanks for the feedback, I'll come back with a "Dense" version of this at some 
point


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] D69532: [LLDB][PythonFile] fix dangerous borrow semantics on python2

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

lawrence_danna added a comment.

@labath looks like you were right, just not flushing seems to work fine.


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