[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
labath added subscribers: lldb-commits, labath.
labath reopened this revision.
labath added a comment.
This revision is now accepted and ready to land.
(Please add lldb-commits to future reviews, so that people are aware of what's
going on.)
So, the reason why this failed to compile is that android does not have the
fopencookie function (neither does windows as far as I am aware). However,
looking at the change, it's not clear to me whether we actually need the
fopencookie functionality to implement this. Couldn't we just change the
File::Read/Write functions to call these directly. Right now they do `if
(am_i_backed_by_FILE) fwrite() else write()`. We could add a option for them to
be backed by a set of callbacks. An even better option would be to just use
virtual functions for this (that's the c++ way of doing cookie callbacks). I
think that's what the IOObject hierarchy (which File is a part of) was meant to
be used for, although I am not sure it will work out of the box for this case.
And it would be great to see some tests for this.
Comment at: lldb/trunk/include/lldb/Host/File.h:65
+ File(File &&rhs);
+
Why are you changing the File to be movable? I don't see the connection between
this and the fopencookie part.
Comment at: lldb/trunk/source/Host/common/File.cpp:124
+ } else {
+return (int)wrote;
+ }
cast seems wrong
Comment at: lldb/trunk/source/Host/common/File.cpp:140
+ } else {
+return (int)read;
+ }
same here
Repository:
rL LLVM
https://reviews.llvm.org/D38829
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39515: Remove TestMyFirstWatchpoint and TestStepOverWatchpoint from basic_process category
labath added a comment. I agree that we have too many ways of specifying categories. I think that the file-based and getCategories() way are redundant, particularly, as we tend to only have one test file per directory anyway. For very fine grained annotations, we can use the `@add_test_categories` decorator (which is already cumulative). I'll put up a patch to remove getCategories(). https://reviews.llvm.org/D39515 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39515: Remove TestMyFirstWatchpoint and TestStepOverWatchpoint from basic_process category
labath updated this revision to Diff 121287.
labath added a comment.
Herald added a subscriber: ki.stfu.
This removes the getCategories function altogether. I've moved the
filesystem-based category computation (which was implemented as a getCategories
base case) into the consumer, so that it is impossible to override. I've
converted the existing getCategories usages to either @add_test_categories (if
they affected a single test) or to .categories.
https://reviews.llvm.org/D39515
Files:
packages/Python/lldbsuite/test/example/TestSequenceFunctions.py
packages/Python/lldbsuite/test/functionalities/expr-doesnt-deadlock/TestExprDoesntBlock.py
packages/Python/lldbsuite/test/functionalities/load_unload/.categories
packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
packages/Python/lldbsuite/test/functionalities/thread/step_until/.categories
packages/Python/lldbsuite/test/functionalities/thread/step_until/TestStepUntil.py
packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchpoint/TestMyFirstWatchpoint.py
packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py
packages/Python/lldbsuite/test/lang/c/step-target/.categories
packages/Python/lldbsuite/test/lang/c/step-target/TestStepTarget.py
packages/Python/lldbsuite/test/lang/c/stepping/TestStepAndBreakpoints.py
packages/Python/lldbsuite/test/lang/objc/objc-stepping/TestObjCStepping.py
packages/Python/lldbsuite/test/lldbtest.py
packages/Python/lldbsuite/test/test_result.py
packages/Python/lldbsuite/test/tools/lldb-mi/.categories
packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
Index: packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
===
--- packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
+++ packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py
@@ -14,9 +14,6 @@
myexe = "a.out"
mylog = "child.log"
-def getCategories(self):
-return ['lldb-mi']
-
@classmethod
def classCleanup(cls):
TestBase.RemoveTempFile(cls.myexe)
Index: packages/Python/lldbsuite/test/tools/lldb-mi/.categories
===
--- /dev/null
+++ packages/Python/lldbsuite/test/tools/lldb-mi/.categories
@@ -0,0 +1 @@
+lldb-mi
Index: packages/Python/lldbsuite/test/test_result.py
===
--- packages/Python/lldbsuite/test/test_result.py
+++ packages/Python/lldbsuite/test/test_result.py
@@ -105,6 +105,32 @@
else:
return str(test)
+@staticmethod
+def _getFileBasedCategories(test):
+"""
+Returns the list of categories to which this test case belongs by
+looking for a ".categories" file. We start at the folder the test is in
+an traverse the hierarchy upwards - we guarantee a .categories to exist
+at the top level directory so we do not end up looping endlessly.
+"""
+import inspect
+import os.path
+folder = inspect.getfile(test.__class__)
+folder = os.path.dirname(folder)
+while folder != '/':
+categories_file_name = os.path.join(folder, ".categories")
+if os.path.exists(categories_file_name):
+categories_file = open(categories_file_name, 'r')
+categories = categories_file.readline()
+categories_file.close()
+categories = str.replace(categories, '\n', '')
+categories = str.replace(categories, '\r', '')
+return categories.split(',')
+else:
+folder = os.path.dirname(folder)
+continue
+
+
def getCategoriesForTest(self, test):
"""
Gets all the categories for the currently running test method in test case
@@ -114,7 +140,7 @@
if test_method is not None and hasattr(test_method, "categories"):
test_categories.extend(test_method.categories)
-test_categories.extend(test.getCategories())
+test_categories.extend(self._getFileBasedCategories(test))
return test_categories
Index: packages/Python/lldbsuite/test/lldbtest.py
===
--- packages/Python/lldbsuite/test/lldbtest.py
+++ packages/Python/lldbsuite/test/lldbtest.py
@@ -1834,30 +1834,6 @@
# Can be overridden by the LLDB_TIME_WAIT_NEXT_LAUNCH environment variable.
timeWaitNextLaunch = 1.0
-# Returns the list of categories to which this test case belongs
-# by default, look for a ".categories" file, and read its contents
-# if no such file exists, traverse the hierarchy - we guarantee
-# a .categories to exist at the top level directory so we do not end up
-# looping endlessly - subclasses are free to define their own categories
[Lldb-commits] [PATCH] D39487: Add float/vector registers for ppc64le
labath added inline comments.
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp:534
+Status NativeRegisterContextLinux_ppc64le::ReadVMX() {
+ void *buf = GetVMXBuffer();
+ if (!buf)
Isn't this somewhat over-engineered?
I mean, GetVMXBuffer returns a pointer to a member variable, so it's pretty
much impossible for it to be null. And all GetVMXSize does is return the size
of that member variable. If we remove all this boilerplate then this becomes
something like:
```
int regset = NT_PPC_VMX;
return NativeProcessLinux::PtraceWrapper(PTRACE_SETVRREGS, m_thread.GetID(),
®set, &m_vmx_ppc64le,
sizeof(m_vmx_ppc64le));
```
which I think is much easier to follow than jumping through dozens of these
tiny functions
https://reviews.llvm.org/D39487
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r317218 - Use LLVM version string
Author: sas
Date: Thu Nov 2 09:56:19 2017
New Revision: 317218
URL: http://llvm.org/viewvc/llvm-project?rev=317218&view=rev
Log:
Use LLVM version string
Summary:
macOS builds of LLDB use the bundle version from
`tools/driver/lldb-Info.plist`. That file hasn't been updated since the 4.0
release so the version we print provides no value. I also think that even if it
were up to date, that number has no meaning and displaying the version from the
LLVM tree is more valuable.
I know that Apple folks have some form of override for the clang version to
match the Xcode version, so it'd make sense for them to do the same for LLDB.
Reviewers: clayborg
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D39429
Modified:
lldb/trunk/source/lldb.cpp
Modified: lldb/trunk/source/lldb.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/lldb.cpp?rev=317218&r1=317217&r2=317218&view=diff
==
--- lldb/trunk/source/lldb.cpp (original)
+++ lldb/trunk/source/lldb.cpp Thu Nov 2 09:56:19 2017
@@ -47,13 +47,9 @@ const char *lldb_private::GetVersion() {
// as the clang tool.
static std::string g_version_str;
if (g_version_str.empty()) {
-
-#ifdef LLDB_VERSION_STRING
-g_version_str += EXPAND_AND_QUOTE(LLDB_VERSION_STRING);
-#else
g_version_str += "lldb version ";
g_version_str += CLANG_VERSION_STRING;
-#endif
+
const char *lldb_repo = GetLLDBRepository();
const char *lldb_rev = GetLLDBRevision();
if (lldb_repo || lldb_rev) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r317219 - Run clang-format on lldb.cpp
Author: sas
Date: Thu Nov 2 09:56:52 2017
New Revision: 317219
URL: http://llvm.org/viewvc/llvm-project?rev=317219&view=rev
Log:
Run clang-format on lldb.cpp
Modified:
lldb/trunk/source/lldb.cpp
Modified: lldb/trunk/source/lldb.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/lldb.cpp?rev=317219&r1=317218&r2=317219&view=diff
==
--- lldb/trunk/source/lldb.cpp (original)
+++ lldb/trunk/source/lldb.cpp Thu Nov 2 09:56:52 2017
@@ -15,11 +15,11 @@ using namespace lldb_private;
#include "clang/Basic/Version.h"
#ifdef HAVE_SVN_VERSION_INC
-# include "SVNVersion.inc"
+#include "SVNVersion.inc"
#endif
#ifdef HAVE_APPLE_VERSION_INC
-# include "AppleVersion.inc"
+#include "AppleVersion.inc"
#endif
static const char *GetLLDBRevision() {
@@ -38,7 +38,6 @@ static const char *GetLLDBRepository() {
#endif
}
-
#define QUOTE(str) #str
#define EXPAND_AND_QUOTE(str) QUOTE(str)
@@ -73,7 +72,6 @@ const char *lldb_private::GetVersion() {
g_version_str += "\n llvm revision ";
g_version_str += llvm_rev;
}
-
}
return g_version_str.c_str();
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39429: Use LLVM version string
This revision was automatically updated to reflect the committed changes.
Closed by commit rL317218: Use LLVM version string (authored by sas).
Repository:
rL LLVM
https://reviews.llvm.org/D39429
Files:
lldb/trunk/source/lldb.cpp
Index: lldb/trunk/source/lldb.cpp
===
--- lldb/trunk/source/lldb.cpp
+++ lldb/trunk/source/lldb.cpp
@@ -47,13 +47,9 @@
// as the clang tool.
static std::string g_version_str;
if (g_version_str.empty()) {
-
-#ifdef LLDB_VERSION_STRING
-g_version_str += EXPAND_AND_QUOTE(LLDB_VERSION_STRING);
-#else
g_version_str += "lldb version ";
g_version_str += CLANG_VERSION_STRING;
-#endif
+
const char *lldb_repo = GetLLDBRepository();
const char *lldb_rev = GetLLDBRevision();
if (lldb_repo || lldb_rev) {
Index: lldb/trunk/source/lldb.cpp
===
--- lldb/trunk/source/lldb.cpp
+++ lldb/trunk/source/lldb.cpp
@@ -47,13 +47,9 @@
// as the clang tool.
static std::string g_version_str;
if (g_version_str.empty()) {
-
-#ifdef LLDB_VERSION_STRING
-g_version_str += EXPAND_AND_QUOTE(LLDB_VERSION_STRING);
-#else
g_version_str += "lldb version ";
g_version_str += CLANG_VERSION_STRING;
-#endif
+
const char *lldb_repo = GetLLDBRepository();
const char *lldb_rev = GetLLDBRevision();
if (lldb_repo || lldb_rev) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
zturner added inline comments.
Comment at:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1075-1096
+static int readfn(void *ctx, char *buffer, int n)
+{
+ auto state = PyGILState_Ensure();
+ auto *file = (PyObject *) ctx;
+ int result = -1;
+ auto pybuffer = PyBuffer_FromMemory(buffer, n);
+ PyObject *pyresult = NULL;
I am still pretty unhappy about these functions, and passing function pointers
into the `File` class.
I think another approach would be this:
1) Make the `File` class contain a member `std::unique_ptr
LowLevelIo;`
2) In `File.cpp`, define something called `class DefaultLowLevelIo : public
IOObject` that implements the virtual methods against an fd.
3) In `PythonDataObjects`, define `PythonFileIo : public IOObject` and
implement the virtual methods against a `PyObject`.
4) Add an additional constructor to `File` which takes a
`std::unique_ptr LowLevelIo`, which we can use when creating one of
these from a python file.
One advantage of this method is that it allows the `PythonFileIo` class to be
easily tested.
(Also, sorry for not getting back to reviewing this several weeks ago)
Repository:
rL LLVM
https://reviews.llvm.org/D38829
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
lawrence_danna added inline comments.
Comment at:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1075-1096
+static int readfn(void *ctx, char *buffer, int n)
+{
+ auto state = PyGILState_Ensure();
+ auto *file = (PyObject *) ctx;
+ int result = -1;
+ auto pybuffer = PyBuffer_FromMemory(buffer, n);
+ PyObject *pyresult = NULL;
zturner wrote:
> I am still pretty unhappy about these functions, and passing function
> pointers into the `File` class.
>
> I think another approach would be this:
>
> 1) Make the `File` class contain a member `std::unique_ptr
> LowLevelIo;`
>
> 2) In `File.cpp`, define something called `class DefaultLowLevelIo : public
> IOObject` that implements the virtual methods against an fd.
>
> 3) In `PythonDataObjects`, define `PythonFileIo : public IOObject` and
> implement the virtual methods against a `PyObject`.
>
> 4) Add an additional constructor to `File` which takes a
> `std::unique_ptr LowLevelIo`, which we can use when creating one of
> these from a python file.
>
> One advantage of this method is that it allows the `PythonFileIo` class to be
> easily tested.
>
> (Also, sorry for not getting back to reviewing this several weeks ago)
I don't see how this approach gets around the problem that the interfaces in
SBDebugger use FILE *, not IOObject
The only way I can see to do it the way you are saying is to also add a
SBIOObject, with swig wrappers to that, and variants of the SBDebugger
interfaces that take IOObject instead of FILE *
Do you want to do it that way?
Repository:
rL LLVM
https://reviews.llvm.org/D38829
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39515: Remove TestMyFirstWatchpoint and TestStepOverWatchpoint from basic_process category
jingham accepted this revision. jingham added a comment. Very nice, thanks! https://reviews.llvm.org/D39515 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
labath added inline comments.
Comment at:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1075-1096
+static int readfn(void *ctx, char *buffer, int n)
+{
+ auto state = PyGILState_Ensure();
+ auto *file = (PyObject *) ctx;
+ int result = -1;
+ auto pybuffer = PyBuffer_FromMemory(buffer, n);
+ PyObject *pyresult = NULL;
lawrence_danna wrote:
> zturner wrote:
> > I am still pretty unhappy about these functions, and passing function
> > pointers into the `File` class.
> >
> > I think another approach would be this:
> >
> > 1) Make the `File` class contain a member `std::unique_ptr
> > LowLevelIo;`
> >
> > 2) In `File.cpp`, define something called `class DefaultLowLevelIo : public
> > IOObject` that implements the virtual methods against an fd.
> >
> > 3) In `PythonDataObjects`, define `PythonFileIo : public IOObject` and
> > implement the virtual methods against a `PyObject`.
> >
> > 4) Add an additional constructor to `File` which takes a
> > `std::unique_ptr LowLevelIo`, which we can use when creating one
> > of these from a python file.
> >
> > One advantage of this method is that it allows the `PythonFileIo` class to
> > be easily tested.
> >
> > (Also, sorry for not getting back to reviewing this several weeks ago)
> I don't see how this approach gets around the problem that the interfaces in
> SBDebugger use FILE *, not IOObject
>
> The only way I can see to do it the way you are saying is to also add a
> SBIOObject, with swig wrappers to that, and variants of the SBDebugger
> interfaces that take IOObject instead of FILE *
>
> Do you want to do it that way?
What's the final use case here. In the patch itself I don't see anything that
would necessitate a FILE * conversion, but I don't know what do you actually
intend to use this for. We can always return a null FILE * if the File object
is backed by a a python file (we do the same for file descriptors, as there is
no way to convert those into FILE*, not without going the fopencookie way).
Repository:
rL LLVM
https://reviews.llvm.org/D38829
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
zturner added inline comments.
Comment at:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1075-1096
+static int readfn(void *ctx, char *buffer, int n)
+{
+ auto state = PyGILState_Ensure();
+ auto *file = (PyObject *) ctx;
+ int result = -1;
+ auto pybuffer = PyBuffer_FromMemory(buffer, n);
+ PyObject *pyresult = NULL;
labath wrote:
> lawrence_danna wrote:
> > zturner wrote:
> > > I am still pretty unhappy about these functions, and passing function
> > > pointers into the `File` class.
> > >
> > > I think another approach would be this:
> > >
> > > 1) Make the `File` class contain a member `std::unique_ptr
> > > LowLevelIo;`
> > >
> > > 2) In `File.cpp`, define something called `class DefaultLowLevelIo :
> > > public IOObject` that implements the virtual methods against an fd.
> > >
> > > 3) In `PythonDataObjects`, define `PythonFileIo : public IOObject` and
> > > implement the virtual methods against a `PyObject`.
> > >
> > > 4) Add an additional constructor to `File` which takes a
> > > `std::unique_ptr LowLevelIo`, which we can use when creating
> > > one of these from a python file.
> > >
> > > One advantage of this method is that it allows the `PythonFileIo` class
> > > to be easily tested.
> > >
> > > (Also, sorry for not getting back to reviewing this several weeks ago)
> > I don't see how this approach gets around the problem that the interfaces
> > in SBDebugger use FILE *, not IOObject
> >
> > The only way I can see to do it the way you are saying is to also add a
> > SBIOObject, with swig wrappers to that, and variants of the SBDebugger
> > interfaces that take IOObject instead of FILE *
> >
> > Do you want to do it that way?
> What's the final use case here. In the patch itself I don't see anything that
> would necessitate a FILE * conversion, but I don't know what do you actually
> intend to use this for. We can always return a null FILE * if the File object
> is backed by a a python file (we do the same for file descriptors, as there
> is no way to convert those into FILE*, not without going the fopencookie way).
Alright, I re-read this more closely. This is actually something I wanted to
fix for a long time.Specifically, I don't believe LLDB should be using
`FILE*` anywhere. I would prefer to mark this method dangerous in big letters
in the SB API, and add new versions that take an fd. A `FILE*` doesn't even
mean anything in Python. It's specifically a native construct.
I see it being used in two places currently. 1) Setting to `None`, and 2)
setting to the result of `open("/dev/null")`. The open method documentation
says "Open a file, returning an object of the file type described in section
File Objects".
So when this method is being called from Python, it is not even a real `FILE*`,
it's a pointer to some python object. I think this is just a bug in the design
of the SB API, and we should fix it there.
I don't often propose adding new SB APIs, but I think we need an entirely
different API here. There should be methods:
```
SetOutputFileHandle(SBFile F);
SetInputFileHandle(SBFile F);
SetErrorFileHandle(SBFile F);
```
And we should be passing those. This will in turn necessitate a lot of trickle
down changes in the native side too. We can mark the older functions
deprecated to indicate that people should no longer be using them.
Thoughts?
Repository:
rL LLVM
https://reviews.llvm.org/D38829
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
zturner added inline comments.
Comment at:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1075-1096
+static int readfn(void *ctx, char *buffer, int n)
+{
+ auto state = PyGILState_Ensure();
+ auto *file = (PyObject *) ctx;
+ int result = -1;
+ auto pybuffer = PyBuffer_FromMemory(buffer, n);
+ PyObject *pyresult = NULL;
zturner wrote:
> labath wrote:
> > lawrence_danna wrote:
> > > zturner wrote:
> > > > I am still pretty unhappy about these functions, and passing function
> > > > pointers into the `File` class.
> > > >
> > > > I think another approach would be this:
> > > >
> > > > 1) Make the `File` class contain a member `std::unique_ptr
> > > > LowLevelIo;`
> > > >
> > > > 2) In `File.cpp`, define something called `class DefaultLowLevelIo :
> > > > public IOObject` that implements the virtual methods against an fd.
> > > >
> > > > 3) In `PythonDataObjects`, define `PythonFileIo : public IOObject` and
> > > > implement the virtual methods against a `PyObject`.
> > > >
> > > > 4) Add an additional constructor to `File` which takes a
> > > > `std::unique_ptr LowLevelIo`, which we can use when creating
> > > > one of these from a python file.
> > > >
> > > > One advantage of this method is that it allows the `PythonFileIo` class
> > > > to be easily tested.
> > > >
> > > > (Also, sorry for not getting back to reviewing this several weeks ago)
> > > I don't see how this approach gets around the problem that the interfaces
> > > in SBDebugger use FILE *, not IOObject
> > >
> > > The only way I can see to do it the way you are saying is to also add a
> > > SBIOObject, with swig wrappers to that, and variants of the SBDebugger
> > > interfaces that take IOObject instead of FILE *
> > >
> > > Do you want to do it that way?
> > What's the final use case here. In the patch itself I don't see anything
> > that would necessitate a FILE * conversion, but I don't know what do you
> > actually intend to use this for. We can always return a null FILE * if the
> > File object is backed by a a python file (we do the same for file
> > descriptors, as there is no way to convert those into FILE*, not without
> > going the fopencookie way).
> Alright, I re-read this more closely. This is actually something I wanted to
> fix for a long time.Specifically, I don't believe LLDB should be using
> `FILE*` anywhere. I would prefer to mark this method dangerous in big
> letters in the SB API, and add new versions that take an fd. A `FILE*`
> doesn't even mean anything in Python. It's specifically a native construct.
>
> I see it being used in two places currently. 1) Setting to `None`, and 2)
> setting to the result of `open("/dev/null")`. The open method documentation
> says "Open a file, returning an object of the file type described in section
> File Objects".
>
> So when this method is being called from Python, it is not even a real
> `FILE*`, it's a pointer to some python object. I think this is just a bug in
> the design of the SB API, and we should fix it there.
>
> I don't often propose adding new SB APIs, but I think we need an entirely
> different API here. There should be methods:
>
> ```
> SetOutputFileHandle(SBFile F);
> SetInputFileHandle(SBFile F);
> SetErrorFileHandle(SBFile F);
> ```
>
> And we should be passing those. This will in turn necessitate a lot of
> trickle down changes in the native side too. We can mark the older functions
> deprecated to indicate that people should no longer be using them.
>
> Thoughts?
Sorry, s/add a new version that takes an fd/add a new version that takes an
SBFile/
Repository:
rL LLVM
https://reviews.llvm.org/D38829
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
labath added inline comments.
Comment at:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1075-1096
+static int readfn(void *ctx, char *buffer, int n)
+{
+ auto state = PyGILState_Ensure();
+ auto *file = (PyObject *) ctx;
+ int result = -1;
+ auto pybuffer = PyBuffer_FromMemory(buffer, n);
+ PyObject *pyresult = NULL;
zturner wrote:
> zturner wrote:
> > labath wrote:
> > > lawrence_danna wrote:
> > > > zturner wrote:
> > > > > I am still pretty unhappy about these functions, and passing function
> > > > > pointers into the `File` class.
> > > > >
> > > > > I think another approach would be this:
> > > > >
> > > > > 1) Make the `File` class contain a member `std::unique_ptr
> > > > > LowLevelIo;`
> > > > >
> > > > > 2) In `File.cpp`, define something called `class DefaultLowLevelIo :
> > > > > public IOObject` that implements the virtual methods against an fd.
> > > > >
> > > > > 3) In `PythonDataObjects`, define `PythonFileIo : public IOObject`
> > > > > and implement the virtual methods against a `PyObject`.
> > > > >
> > > > > 4) Add an additional constructor to `File` which takes a
> > > > > `std::unique_ptr LowLevelIo`, which we can use when
> > > > > creating one of these from a python file.
> > > > >
> > > > > One advantage of this method is that it allows the `PythonFileIo`
> > > > > class to be easily tested.
> > > > >
> > > > > (Also, sorry for not getting back to reviewing this several weeks ago)
> > > > I don't see how this approach gets around the problem that the
> > > > interfaces in SBDebugger use FILE *, not IOObject
> > > >
> > > > The only way I can see to do it the way you are saying is to also add a
> > > > SBIOObject, with swig wrappers to that, and variants of the SBDebugger
> > > > interfaces that take IOObject instead of FILE *
> > > >
> > > > Do you want to do it that way?
> > > What's the final use case here. In the patch itself I don't see anything
> > > that would necessitate a FILE * conversion, but I don't know what do you
> > > actually intend to use this for. We can always return a null FILE * if
> > > the File object is backed by a a python file (we do the same for file
> > > descriptors, as there is no way to convert those into FILE*, not without
> > > going the fopencookie way).
> > Alright, I re-read this more closely. This is actually something I wanted
> > to fix for a long time.Specifically, I don't believe LLDB should be
> > using `FILE*` anywhere. I would prefer to mark this method dangerous in
> > big letters in the SB API, and add new versions that take an fd. A `FILE*`
> > doesn't even mean anything in Python. It's specifically a native
> > construct.
> >
> > I see it being used in two places currently. 1) Setting to `None`, and 2)
> > setting to the result of `open("/dev/null")`. The open method
> > documentation says "Open a file, returning an object of the file type
> > described in section File Objects".
> >
> > So when this method is being called from Python, it is not even a real
> > `FILE*`, it's a pointer to some python object. I think this is just a bug
> > in the design of the SB API, and we should fix it there.
> >
> > I don't often propose adding new SB APIs, but I think we need an entirely
> > different API here. There should be methods:
> >
> > ```
> > SetOutputFileHandle(SBFile F);
> > SetInputFileHandle(SBFile F);
> > SetErrorFileHandle(SBFile F);
> > ```
> >
> > And we should be passing those. This will in turn necessitate a lot of
> > trickle down changes in the native side too. We can mark the older
> > functions deprecated to indicate that people should no longer be using them.
> >
> > Thoughts?
> Sorry, s/add a new version that takes an fd/add a new version that takes an
> SBFile/
>I don't often propose adding new SB APIs, but I think we need an entirely
>different API here. There should be methods:
>SetOutputFileHandle(SBFile F);
>SetInputFileHandle(SBFile F);
>SetErrorFileHandle(SBFile F);
>And we should be passing those. This will in turn necessitate a lot of trickle
>down changes in the native side too. We can mark the older functions
>deprecated to indicate that people should no longer be using them.
Note that these file handles eventually trickle down into libedit, which
expects to see a FILE *. These could probably be synthesized(*) for libedit's
usage alone, and leave the rest of the world oblivious to FILE*, but it will
need to be done very carefully.
(*) Windows does not have a fopencookie/funopen equivalent afaik, but then
again, it also does not have libedit...
Repository:
rL LLVM
https://reviews.llvm.org/D38829
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
clayborg added a comment.
Another option would be to add a URL version of these functions:
class SBDebugger {
void SetInputURL(const char *url);
void SetOutputURL(const char *url);
void SetErrorURL(const char *url);
};
Then we allow this to trickle down to the IOBase in that way. This allows us to
innovate by adding new support for new URLs.
"FILE*" is unfortunately what we will need for libedit for now, but we can
probably make sure the IOBase call can provide this somehow, or convert a file
descriptor into one, and fall back on a simple non-libedit based command
interpreter if not. The current IOHandler stack will check the FILE's abilities
and do the right thing already.
Repository:
rL LLVM
https://reviews.llvm.org/D38829
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
zturner added a comment. Ok, I wasn't aware of the libedit problem. I still don't like this approach, because it causes the design of the `File` class to be influenced by a limitation of a 3rd party library. What about adding a method to `IOObject` called `FILE *GetFileStream()`. Then the method I proposed earlier would just involve `PythonFileIo` implementing this in the proper way. Then we can pass `SBFile` or `lldb_private::File` through all layers of the codebase, and once we're in libedit we just call `file.GetFileStream()` and pass it to libedit? Repository: rL LLVM https://reviews.llvm.org/D38829 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
lawrence_danna added a comment. @labath > (Please add lldb-commits to future reviews, so that people are aware of > what's going on.) ok > Couldn't we just change the File::Read/Write functions to call these directly Like I said to @zturner , this is possible, but it can't work with the existing APIs in SBDebugger.h. We'd need to add new APIs that take SBIOBase instead of FILE *. Do you agree with that plan? > And it would be great to see some tests for this yup, there's tests in TestFileHandle.py Comment at: lldb/trunk/include/lldb/Host/File.h:65 + File(File &&rhs); + labath wrote: > Why are you changing the File to be movable? I don't see the connection > between this and the fopencookie part. I use the move constructor here: file = File(m_py_obj, readable ? readfn : NULL, writable ? writefn : NULL, closefn); Repository: rL LLVM https://reviews.llvm.org/D38829 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
clayborg added a comment. In https://reviews.llvm.org/D38829#914300, @zturner wrote: > Ok, I wasn't aware of the libedit problem. > > I still don't like this approach, because it causes the design of the `File` > class to be influenced by a limitation of a 3rd party library. > > What about adding a method to `IOObject` called `FILE *GetFileStream()`. > Then the method I proposed earlier would just involve `PythonFileIo` > implementing this in the proper way. Then we can pass `SBFile` or > `lldb_private::File` through all layers of the codebase, and once we're in > libedit we just call `file.GetFileStream()` and pass it to libedit? That could work. If we don't get a "FILE *" back from IOObject::GetFileStream() we need to fall back onto a simple implementation. The current IOHandler will fall back to fgets() on a FILE*, but we can easily make it just call a method on IOObject. Probably would be best to add a fgets() type call to IOObject so the underlying implementation can do this as efficiently as possible. I mainly care that the current API continues to work with "FILE *" at least on all Unix based platforms. We can add new APIs, but we must leave the existing ones there. Repository: rL LLVM https://reviews.llvm.org/D38829 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
labath added a comment. In https://reviews.llvm.org/D38829#914300, @zturner wrote: > Ok, I wasn't aware of the libedit problem. > > I still don't like this approach, because it causes the design of the `File` > class to be influenced by a limitation of a 3rd party library. > > What about adding a method to `IOObject` called `FILE *GetFileStream()`. > Then the method I proposed earlier would just involve `PythonFileIo` > implementing this in the proper way. Then we can pass `SBFile` or > `lldb_private::File` through all layers of the codebase, and once we're in > libedit we just call `file.GetFileStream()` and pass it to libedit? That's how I'd do it. (*) libedit needs both a FILE* and a raw file descriptor in this case (I think it's calling select, and some terminal functions). Repository: rL LLVM https://reviews.llvm.org/D38829 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
lawrence_danna added inline comments.
Comment at:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1075-1096
+static int readfn(void *ctx, char *buffer, int n)
+{
+ auto state = PyGILState_Ensure();
+ auto *file = (PyObject *) ctx;
+ int result = -1;
+ auto pybuffer = PyBuffer_FromMemory(buffer, n);
+ PyObject *pyresult = NULL;
labath wrote:
> zturner wrote:
> > zturner wrote:
> > > labath wrote:
> > > > lawrence_danna wrote:
> > > > > zturner wrote:
> > > > > > I am still pretty unhappy about these functions, and passing
> > > > > > function pointers into the `File` class.
> > > > > >
> > > > > > I think another approach would be this:
> > > > > >
> > > > > > 1) Make the `File` class contain a member
> > > > > > `std::unique_ptr LowLevelIo;`
> > > > > >
> > > > > > 2) In `File.cpp`, define something called `class DefaultLowLevelIo
> > > > > > : public IOObject` that implements the virtual methods against an
> > > > > > fd.
> > > > > >
> > > > > > 3) In `PythonDataObjects`, define `PythonFileIo : public IOObject`
> > > > > > and implement the virtual methods against a `PyObject`.
> > > > > >
> > > > > > 4) Add an additional constructor to `File` which takes a
> > > > > > `std::unique_ptr LowLevelIo`, which we can use when
> > > > > > creating one of these from a python file.
> > > > > >
> > > > > > One advantage of this method is that it allows the `PythonFileIo`
> > > > > > class to be easily tested.
> > > > > >
> > > > > > (Also, sorry for not getting back to reviewing this several weeks
> > > > > > ago)
> > > > > I don't see how this approach gets around the problem that the
> > > > > interfaces in SBDebugger use FILE *, not IOObject
> > > > >
> > > > > The only way I can see to do it the way you are saying is to also add
> > > > > a SBIOObject, with swig wrappers to that, and variants of the
> > > > > SBDebugger
> > > > > interfaces that take IOObject instead of FILE *
> > > > >
> > > > > Do you want to do it that way?
> > > > What's the final use case here. In the patch itself I don't see
> > > > anything that would necessitate a FILE * conversion, but I don't know
> > > > what do you actually intend to use this for. We can always return a
> > > > null FILE * if the File object is backed by a a python file (we do the
> > > > same for file descriptors, as there is no way to convert those into
> > > > FILE*, not without going the fopencookie way).
> > > Alright, I re-read this more closely. This is actually something I
> > > wanted to fix for a long time.Specifically, I don't believe LLDB
> > > should be using `FILE*` anywhere. I would prefer to mark this method
> > > dangerous in big letters in the SB API, and add new versions that take an
> > > fd. A `FILE*` doesn't even mean anything in Python. It's specifically a
> > > native construct.
> > >
> > > I see it being used in two places currently. 1) Setting to `None`, and
> > > 2) setting to the result of `open("/dev/null")`. The open method
> > > documentation says "Open a file, returning an object of the file type
> > > described in section File Objects".
> > >
> > > So when this method is being called from Python, it is not even a real
> > > `FILE*`, it's a pointer to some python object. I think this is just a
> > > bug in the design of the SB API, and we should fix it there.
> > >
> > > I don't often propose adding new SB APIs, but I think we need an entirely
> > > different API here. There should be methods:
> > >
> > > ```
> > > SetOutputFileHandle(SBFile F);
> > > SetInputFileHandle(SBFile F);
> > > SetErrorFileHandle(SBFile F);
> > > ```
> > >
> > > And we should be passing those. This will in turn necessitate a lot of
> > > trickle down changes in the native side too. We can mark the older
> > > functions deprecated to indicate that people should no longer be using
> > > them.
> > >
> > > Thoughts?
> > Sorry, s/add a new version that takes an fd/add a new version that takes an
> > SBFile/
> >I don't often propose adding new SB APIs, but I think we need an entirely
> >different API here. There should be methods:
>
> >SetOutputFileHandle(SBFile F);
> >SetInputFileHandle(SBFile F);
> >SetErrorFileHandle(SBFile F);
> >And we should be passing those. This will in turn necessitate a lot of
> >trickle down changes in the native side too. We can mark the older functions
> >deprecated to indicate that people should no longer be using them.
>
> Note that these file handles eventually trickle down into libedit, which
> expects to see a FILE *. These could probably be synthesized(*) for libedit's
> usage alone, and leave the rest of the world oblivious to FILE*, but it will
> need to be done very carefully.
>
> (*) Windows does not have a fopencookie/funopen equivalent afaik, but then
> again, it also does not have libedit...
> What's the final use case here
LLDB integration with iPython. I want to be able to red
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
labath added a comment. In https://reviews.llvm.org/D38829#914305, @lawrence_danna wrote: > @labath > > > Couldn't we just change the File::Read/Write functions to call these > > directly > > Like I said to @zturner , this is possible, but it can't work with the > existing APIs in SBDebugger.h. Ok, I am starting to understand what you want to do here. > > >> And it would be great to see some tests for this > > yup, there's tests in TestFileHandle.py These may prove that you don't break existing functionality, but here you are introducing new one, which should be tested as well. (But don't go writing these yet until we figure out how exactly to implement this) Repository: rL LLVM https://reviews.llvm.org/D38829 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.
hintonda updated this revision to Diff 121350.
hintonda added a comment.
I realize this probably isn't an acceptable change, but it was a good
learing experience for me, as I'd never looked at the lldb code before
last Sunday, so I figured I'd go ahead and complete it.
While the basic technique remains the same, this version just adds a
single enum value, ePathSyntaxRegex, and modifies PathSyntaxIsPosix()
to treat it as a posix path so it doesn't get modified.
The logic for testing the regex was moved to
FileSpecList::FindFileIndex() which does the filtering and already
deals with equality issues, e.g., whether or not to include the
directory in the match.
Finally, the SB API was modified to use accept a new regex flag, and a
test was added.
Thanks again for all your feedback and suggestions...
https://reviews.llvm.org/D39436
Files:
include/lldb/API/SBFileSpec.h
include/lldb/Utility/FileSpec.h
packages/Python/lldbsuite/test/python_api/breakpoint/TestBreakpointAPI.py
scripts/interface/SBFileSpec.i
source/API/SBFileSpec.cpp
source/Commands/CommandObjectBreakpoint.cpp
source/Core/FileSpecList.cpp
source/Utility/FileSpec.cpp
Index: source/Utility/FileSpec.cpp
===
--- source/Utility/FileSpec.cpp
+++ source/Utility/FileSpec.cpp
@@ -48,7 +48,8 @@
bool PathSyntaxIsPosix(FileSpec::PathSyntax syntax) {
return (syntax == FileSpec::ePathSyntaxPosix ||
(syntax == FileSpec::ePathSyntaxHostNative &&
- GetNativeSyntax() == FileSpec::ePathSyntaxPosix));
+ GetNativeSyntax() == FileSpec::ePathSyntaxPosix) ||
+ syntax == FileSpec::ePathSyntaxRegex);
}
const char *GetPathSeparators(FileSpec::PathSyntax syntax) {
Index: source/Core/FileSpecList.cpp
===
--- source/Core/FileSpecList.cpp
+++ source/Core/FileSpecList.cpp
@@ -10,6 +10,7 @@
#include "lldb/Core/FileSpecList.h"
#include "lldb/Utility/ConstString.h" // for ConstString
+#include "lldb/Utility/RegularExpression.h"
#include "lldb/Utility/Stream.h"
#include // for find
@@ -90,6 +91,12 @@
bool compare_filename_only = file_spec.GetDirectory().IsEmpty();
for (size_t idx = start_idx; idx < num_files; ++idx) {
+if (m_files[idx].GetPathSyntax() == FileSpec::ePathSyntaxRegex) {
+ RegularExpression regex(m_files[idx].GetPath(false));
+ if (regex.Execute(file_spec.GetPath()))
+return idx;
+ continue;
+}
if (compare_filename_only) {
if (ConstString::Equals(
m_files[idx].GetFilename(), file_spec.GetFilename(),
Index: source/Commands/CommandObjectBreakpoint.cpp
===
--- source/Commands/CommandObjectBreakpoint.cpp
+++ source/Commands/CommandObjectBreakpoint.cpp
@@ -256,6 +256,8 @@
{ LLDB_OPT_NOT_10, false, "shlib", 's', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eModuleCompletion, eArgTypeShlibName, "Set the breakpoint only in this shared library. Can repeat this option "
"multiple times to specify multiple shared libraries." },
{ LLDB_OPT_SET_ALL, false, "hardware", 'H', OptionParser::eNoArgument, nullptr, nullptr, 0, eArgTypeNone,"Require the breakpoint to use hardware breakpoints." },
+ { LLDB_OPT_FILE, false, "source-file-regex", 'z', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeRegularExpression, "Only files matching pattern." },
+ { LLDB_OPT_FILE, false, "module-regex", 'Z', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeRegularExpression, "Only modules matching pattern." },
{ LLDB_OPT_FILE, false, "file", 'f', OptionParser::eRequiredArgument, nullptr, nullptr, CommandCompletions::eSourceFileCompletion, eArgTypeFilename,"Specifies the source file in which to set this breakpoint. Note, by default "
"lldb only looks for files that are #included if they use the standard include "
"file extensions. To set breakpoints on .c/.cpp/.m/.mm files that are "
@@ -560,6 +562,16 @@
m_source_regex_func_names.insert(option_arg);
break;
+ case 'z':
+m_filenames.AppendIfUnique(
+FileSpec(option_arg, false, FileSpec::ePathSyntaxRegex));
+break;
+
+ case 'Z':
+m_modules.AppendIfUnique(
+FileSpec(option_arg, false, FileSpec::ePathSyntaxRegex));
+break;
+
default:
error.SetErrorStringWithFormat("unrecognized option '%c'",
short_option);
Index: source/API/SBFileSpec.cpp
===
--- sour
[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase
zturner added inline comments.
Comment at:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1075-1096
+static int readfn(void *ctx, char *buffer, int n)
+{
+ auto state = PyGILState_Ensure();
+ auto *file = (PyObject *) ctx;
+ int result = -1;
+ auto pybuffer = PyBuffer_FromMemory(buffer, n);
+ PyObject *pyresult = NULL;
lawrence_danna wrote:
> labath wrote:
> > zturner wrote:
> > > zturner wrote:
> > > > labath wrote:
> > > > > lawrence_danna wrote:
> > > > > > zturner wrote:
> > > > > > > I am still pretty unhappy about these functions, and passing
> > > > > > > function pointers into the `File` class.
> > > > > > >
> > > > > > > I think another approach would be this:
> > > > > > >
> > > > > > > 1) Make the `File` class contain a member
> > > > > > > `std::unique_ptr LowLevelIo;`
> > > > > > >
> > > > > > > 2) In `File.cpp`, define something called `class
> > > > > > > DefaultLowLevelIo : public IOObject` that implements the virtual
> > > > > > > methods against an fd.
> > > > > > >
> > > > > > > 3) In `PythonDataObjects`, define `PythonFileIo : public
> > > > > > > IOObject` and implement the virtual methods against a `PyObject`.
> > > > > > >
> > > > > > > 4) Add an additional constructor to `File` which takes a
> > > > > > > `std::unique_ptr LowLevelIo`, which we can use when
> > > > > > > creating one of these from a python file.
> > > > > > >
> > > > > > > One advantage of this method is that it allows the `PythonFileIo`
> > > > > > > class to be easily tested.
> > > > > > >
> > > > > > > (Also, sorry for not getting back to reviewing this several weeks
> > > > > > > ago)
> > > > > > I don't see how this approach gets around the problem that the
> > > > > > interfaces in SBDebugger use FILE *, not IOObject
> > > > > >
> > > > > > The only way I can see to do it the way you are saying is to also
> > > > > > add a SBIOObject, with swig wrappers to that, and variants of the
> > > > > > SBDebugger
> > > > > > interfaces that take IOObject instead of FILE *
> > > > > >
> > > > > > Do you want to do it that way?
> > > > > What's the final use case here. In the patch itself I don't see
> > > > > anything that would necessitate a FILE * conversion, but I don't know
> > > > > what do you actually intend to use this for. We can always return a
> > > > > null FILE * if the File object is backed by a a python file (we do
> > > > > the same for file descriptors, as there is no way to convert those
> > > > > into FILE*, not without going the fopencookie way).
> > > > Alright, I re-read this more closely. This is actually something I
> > > > wanted to fix for a long time.Specifically, I don't believe LLDB
> > > > should be using `FILE*` anywhere. I would prefer to mark this method
> > > > dangerous in big letters in the SB API, and add new versions that take
> > > > an fd. A `FILE*` doesn't even mean anything in Python. It's
> > > > specifically a native construct.
> > > >
> > > > I see it being used in two places currently. 1) Setting to `None`,
> > > > and 2) setting to the result of `open("/dev/null")`. The open method
> > > > documentation says "Open a file, returning an object of the file type
> > > > described in section File Objects".
> > > >
> > > > So when this method is being called from Python, it is not even a real
> > > > `FILE*`, it's a pointer to some python object. I think this is just a
> > > > bug in the design of the SB API, and we should fix it there.
> > > >
> > > > I don't often propose adding new SB APIs, but I think we need an
> > > > entirely different API here. There should be methods:
> > > >
> > > > ```
> > > > SetOutputFileHandle(SBFile F);
> > > > SetInputFileHandle(SBFile F);
> > > > SetErrorFileHandle(SBFile F);
> > > > ```
> > > >
> > > > And we should be passing those. This will in turn necessitate a lot of
> > > > trickle down changes in the native side too. We can mark the older
> > > > functions deprecated to indicate that people should no longer be using
> > > > them.
> > > >
> > > > Thoughts?
> > > Sorry, s/add a new version that takes an fd/add a new version that takes
> > > an SBFile/
> > >I don't often propose adding new SB APIs, but I think we need an entirely
> > >different API here. There should be methods:
> >
> > >SetOutputFileHandle(SBFile F);
> > >SetInputFileHandle(SBFile F);
> > >SetErrorFileHandle(SBFile F);
> > >And we should be passing those. This will in turn necessitate a lot of
> > >trickle down changes in the native side too. We can mark the older
> > >functions deprecated to indicate that people should no longer be using
> > >them.
> >
> > Note that these file handles eventually trickle down into libedit, which
> > expects to see a FILE *. These could probably be synthesized(*) for
> > libedit's usage alone, and leave the rest of the world oblivious to FILE*,
> > but it will need to be done very carefully.
> >
> >
[Lldb-commits] [lldb] r317270 - Fix some warnings found by ToT clang
Author: labath
Date: Thu Nov 2 14:35:26 2017
New Revision: 317270
URL: http://llvm.org/viewvc/llvm-project?rev=317270&view=rev
Log:
Fix some warnings found by ToT clang
These fall into two categories:
- unused variables
- (uint8_t *)NULL + X -- changed to reinterpret_cast(X)
Modified:
lldb/trunk/include/lldb/Core/RangeMap.h
lldb/trunk/source/Breakpoint/BreakpointIDList.cpp
lldb/trunk/source/Core/FileSpecList.cpp
lldb/trunk/source/Core/Value.cpp
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
lldb/trunk/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
lldb/trunk/source/Symbol/CompilerType.cpp
lldb/trunk/source/Symbol/Type.cpp
lldb/trunk/source/Target/PathMappingList.cpp
lldb/trunk/source/Utility/UriParser.cpp
Modified: lldb/trunk/include/lldb/Core/RangeMap.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/RangeMap.h?rev=317270&r1=317269&r2=317270&view=diff
==
--- lldb/trunk/include/lldb/Core/RangeMap.h (original)
+++ lldb/trunk/include/lldb/Core/RangeMap.h Thu Nov 2 14:35:26 2017
@@ -975,7 +975,6 @@ public:
#endif
if (!m_entries.empty()) {
- typename Collection::const_iterator pos;
for (const auto &entry : m_entries) {
if (entry.Contains(addr))
indexes.push_back(entry.data);
Modified: lldb/trunk/source/Breakpoint/BreakpointIDList.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointIDList.cpp?rev=317270&r1=317269&r2=317270&view=diff
==
--- lldb/trunk/source/Breakpoint/BreakpointIDList.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointIDList.cpp Thu Nov 2 14:35:26 2017
@@ -139,7 +139,6 @@ void BreakpointIDList::FindAndReplaceIDR
return;
}
-llvm::StringRef range_expr;
Status error;
std::tie(range_from, range_to) =
Modified: lldb/trunk/source/Core/FileSpecList.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/FileSpecList.cpp?rev=317270&r1=317269&r2=317270&view=diff
==
--- lldb/trunk/source/Core/FileSpecList.cpp (original)
+++ lldb/trunk/source/Core/FileSpecList.cpp Thu Nov 2 14:35:26 2017
@@ -49,7 +49,7 @@ void FileSpecList::Append(const FileSpec
// contained a copy of "file_spec".
//--
bool FileSpecList::AppendIfUnique(const FileSpec &file_spec) {
- collection::iterator pos, end = m_files.end();
+ collection::iterator end = m_files.end();
if (find(m_files.begin(), end, file_spec) == end) {
m_files.push_back(file_spec);
return true;
Modified: lldb/trunk/source/Core/Value.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Value.cpp?rev=317270&r1=317269&r2=317270&view=diff
==
--- lldb/trunk/source/Core/Value.cpp (original)
+++ lldb/trunk/source/Core/Value.cpp Thu Nov 2 14:35:26 2017
@@ -535,7 +535,7 @@ Status Value::GetValueAsData(ExecutionCo
"trying to read from host address of 0.");
return error;
}
- memcpy(dst, (uint8_t *)NULL + address, byte_size);
+ memcpy(dst, reinterpret_cast(address), byte_size);
} else if ((address_type == eAddressTypeLoad) ||
(address_type == eAddressTypeFile)) {
if (file_so_addr.IsValid()) {
Modified:
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp?rev=317270&r1=317269&r2=317270&view=diff
==
---
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
(original)
+++
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
Thu Nov 2 14:35:26 2017
@@ -324,8 +324,6 @@ public:
clang::ASTContext &ast_ctx(interface_decl->getASTContext());
-clang::QualType return_qual_type;
-
const bool isInstance = instance;
const bool isVariadic = false;
const bool isSynthesized = false;
Modified:
lldb/trunk/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp?rev=317270&r1=317269&r2=317270&view=diff
==
--- lldb/trunk/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
(original)
+++ lldb/trunk/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
Thu Nov 2 14:35:26 2017
@@ -85,8 +85,6 @@ size_t UnwindMacOSXFrameBackchain::GetSt
if
[Lldb-commits] [lldb] r317276 - Remove android watchpoint xfails
Author: labath Date: Thu Nov 2 15:12:55 2017 New Revision: 317276 URL: http://llvm.org/viewvc/llvm-project?rev=317276&view=rev Log: Remove android watchpoint xfails Now that the wathpoint tests have their own category, we can easily skip them on devices which don't have watchpoint support. Therefore, we don't need an android xfail on each of these tests. Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/n_watch_n_break/TestConcurrentNWatchNBreak.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoints_one_breakpoint/TestConcurrentTwoWatchpointsOneBreakpoint.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchlocation/TestWatchLocation.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchpoint/TestMyFirstWatchpoint.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_threads/TestWatchpointMultipleThreads.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/TestWatchpointCommands.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/condition/TestWatchpointConditionCmd.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/TestWatchpointDisable.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_events/TestWatchpointEvents.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_on_vectors/TestValueOfVectorVariable.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_set_command/TestWatchLocationWithWatchSet.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_size/TestWatchpointSizes.py lldb/trunk/packages/Python/lldbsuite/test/python_api/watchpoint/TestSetWatchpoint.py lldb/trunk/packages/Python/lldbsuite/test/python_api/watchpoint/TestWatchpointIgnoreCount.py lldb/trunk/packages/Python/lldbsuite/test/python_api/watchpoint/TestWatchpointIter.py lldb/trunk/packages/Python/lldbsuite/test/python_api/watchpoint/condition/TestWatchpointConditionAPI.py lldb/trunk/packages/Python/lldbsuite/test/python_api/watchpoint/watchlocation/TestSetWatchlocation.py lldb/trunk/packages/Python/lldbsuite/test/python_api/watchpoint/watchlocation/TestTargetWatchAddress.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/n_watch_n_break/TestConcurrentNWatchNBreak.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/n_watch_n_break/TestConcurrentNWatchNBreak.py?rev=317276&r1=317275&r2=317276&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/n_watch_n_break/TestConcurrentNWatchNBreak.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/n_watch_n_break/TestConcurrentNWatchNBreak.py Thu Nov 2 15:12:55 2017 @@ -16,7 +16,6 @@ class ConcurrentNWatchNBreak(ConcurrentE @skipIfRemoteDueToDeadlock # Atomic sequences are not supported yet for MIPS in LLDB. @skipIf(triple='^mips') -@expectedFailureAll(oslist=["linux"]) # Very flakey @add_test_categories(["watchpoint"]) def test(self): """Test with 5 watchpoint and breakpoint threads.""" Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoints_one_breakpoint/TestConcurrentTwoWatchpointsOneBreakpoint.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoints_one_breakpoint/TestConcurrentTwoWatchpointsOneBreakpoint.py?rev=317276&r1=317275&r2=317276&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoints_one_breakpoint/TestConcurrentTwoWatchpointsOneBreakpoint.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/two_watchpoints_one_breakpoint/TestConcurrentTwoWatchpointsOneBreakpoint.py Thu Nov 2 15:12:55 2017 @@ -16,7 +16,6 @@ class ConcurrentTwoWat
[Lldb-commits] [lldb] r317277 - Remove getCategories mechanism of specifying test categories
Author: labath Date: Thu Nov 2 15:13:09 2017 New Revision: 317277 URL: http://llvm.org/viewvc/llvm-project?rev=317277&view=rev Log: Remove getCategories mechanism of specifying test categories Summary: This mechanism was mostly redundant with the file-based .categories mechanism, and it was interfering with it, as any test which implemented a getCategories method would not inherit the filesystem categories. This patch removes it. The existing categories are preserved either by adding a .categories file, or using the @add_test_categories decorator. Reviewers: jingham, clayborg, zturner Subscribers: lldb-commits Differential Revision: https://reviews.llvm.org/D39515 Added: lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/.categories lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/step_until/.categories lldb/trunk/packages/Python/lldbsuite/test/lang/c/step-target/.categories lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/.categories Modified: lldb/trunk/packages/Python/lldbsuite/test/example/TestSequenceFunctions.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/expr-doesnt-deadlock/TestExprDoesntBlock.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/step_until/TestStepUntil.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchpoint/TestMyFirstWatchpoint.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py lldb/trunk/packages/Python/lldbsuite/test/lang/c/step-target/TestStepTarget.py lldb/trunk/packages/Python/lldbsuite/test/lang/c/stepping/TestStepAndBreakpoints.py lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-stepping/TestObjCStepping.py lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py lldb/trunk/packages/Python/lldbsuite/test/test_result.py lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py Modified: lldb/trunk/packages/Python/lldbsuite/test/example/TestSequenceFunctions.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/example/TestSequenceFunctions.py?rev=317277&r1=317276&r2=317277&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/example/TestSequenceFunctions.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/example/TestSequenceFunctions.py Thu Nov 2 15:13:09 2017 @@ -30,8 +30,5 @@ class SequenceFunctionsTestCase(unittest for element in random.sample(self.seq, 5): self.assertTrue(element in self.seq) -def getCategories(self): -return [] - if __name__ == '__main__': unittest.main() Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/expr-doesnt-deadlock/TestExprDoesntBlock.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/expr-doesnt-deadlock/TestExprDoesntBlock.py?rev=317277&r1=317276&r2=317277&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/expr-doesnt-deadlock/TestExprDoesntBlock.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/expr-doesnt-deadlock/TestExprDoesntBlock.py Thu Nov 2 15:13:09 2017 @@ -16,15 +16,13 @@ from lldbsuite.test import lldbutil class ExprDoesntDeadlockTestCase(TestBase): -def getCategories(self): -return ['basic_process'] - mydir = TestBase.compute_mydir(__file__) @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr17946') @expectedFailureAll( oslist=["windows"], bugnumber="Windows doesn't have pthreads, test needs to be ported") +@add_test_categories(["basic_process"]) def test_with_run_command(self): """Test that expr will time out and allow other threads to run if it blocks.""" self.build() Added: lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/.categories URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/.categories?rev=317277&view=auto == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/.categories (added) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/.categories Thu Nov 2 15:13:09 2017 @@ -0,0 +1 @@ +basic_process Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py?rev=317277&r1=317276&r2=317277&view=diff ==
[Lldb-commits] [PATCH] D39515: Remove TestMyFirstWatchpoint and TestStepOverWatchpoint from basic_process category
This revision was automatically updated to reflect the committed changes. Closed by commit rL317277: Remove getCategories mechanism of specifying test categories (authored by labath). Repository: rL LLVM https://reviews.llvm.org/D39515 Files: lldb/trunk/packages/Python/lldbsuite/test/example/TestSequenceFunctions.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/expr-doesnt-deadlock/TestExprDoesntBlock.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/.categories lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/step_until/.categories lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/step_until/TestStepUntil.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/hello_watchpoint/TestMyFirstWatchpoint.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py lldb/trunk/packages/Python/lldbsuite/test/lang/c/step-target/.categories lldb/trunk/packages/Python/lldbsuite/test/lang/c/step-target/TestStepTarget.py lldb/trunk/packages/Python/lldbsuite/test/lang/c/stepping/TestStepAndBreakpoints.py lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-stepping/TestObjCStepping.py lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py lldb/trunk/packages/Python/lldbsuite/test/test_result.py lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/.categories lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py Index: lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-stepping/TestObjCStepping.py === --- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-stepping/TestObjCStepping.py +++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-stepping/TestObjCStepping.py @@ -13,9 +13,6 @@ class TestObjCStepping(TestBase): -def getCategories(self): -return ['basic_process'] - mydir = TestBase.compute_mydir(__file__) def setUp(self): @@ -35,7 +32,7 @@ self.main_source, '// Step over nil should stop here.') @skipUnlessDarwin -@add_test_categories(['pyapi']) +@add_test_categories(['pyapi', 'basic_process']) def test_with_python_api(self): """Test stepping through ObjC method dispatch in various forms.""" self.build() Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/step-target/TestStepTarget.py === --- lldb/trunk/packages/Python/lldbsuite/test/lang/c/step-target/TestStepTarget.py +++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/step-target/TestStepTarget.py @@ -14,9 +14,6 @@ mydir = TestBase.compute_mydir(__file__) -def getCategories(self): -return ['basic_process'] - def setUp(self): # Call super's setUp(). TestBase.setUp(self) Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/step-target/.categories === --- lldb/trunk/packages/Python/lldbsuite/test/lang/c/step-target/.categories +++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/step-target/.categories @@ -0,0 +1 @@ +basic_process Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/stepping/TestStepAndBreakpoints.py === --- lldb/trunk/packages/Python/lldbsuite/test/lang/c/stepping/TestStepAndBreakpoints.py +++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/stepping/TestStepAndBreakpoints.py @@ -15,16 +15,13 @@ mydir = TestBase.compute_mydir(__file__) -def getCategories(self): -return ['basic_process'] - def setUp(self): # Call super's setUp(). TestBase.setUp(self) # Find the line numbers that we will step to in main: self.main_source = "main.c" -@add_test_categories(['pyapi']) +@add_test_categories(['pyapi', 'basic_process']) @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr17932') @expectedFailureAll(oslist=["linux"], bugnumber="llvm.org/pr14437") @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24777") Index: lldb/trunk/packages/Python/lldbsuite/test/test_result.py === --- lldb/trunk/packages/Python/lldbsuite/test/test_result.py +++ lldb/trunk/packages/Python/lldbsuite/test/test_result.py @@ -105,6 +105,32 @@ else: return str(test) +@staticmethod +def _getFileBasedCategories(test): +""" +Returns the list of categories to which this test case belongs by +looking for a ".categories" file. We start at the folder the test is in +an traverse the hierarchy upwards - we guarantee a .categories to exist +at the top level direct
[Lldb-commits] [PATCH] D39574: Add type to FileSpec::PathSyntax enum.
hintonda created this revision.
Add type to FileSpec::PathSyntax enum to decrease size for
FileSpec on systems with 32 bit pointers.
Thanks to @zturner for pointing this out.
https://reviews.llvm.org/D39574
Files:
include/lldb/Utility/FileSpec.h
Index: include/lldb/Utility/FileSpec.h
===
--- include/lldb/Utility/FileSpec.h
+++ include/lldb/Utility/FileSpec.h
@@ -61,7 +61,7 @@
//--
class FileSpec {
public:
- enum PathSyntax {
+ enum PathSyntax : unsigned char {
ePathSyntaxPosix,
ePathSyntaxWindows,
ePathSyntaxHostNative
Index: include/lldb/Utility/FileSpec.h
===
--- include/lldb/Utility/FileSpec.h
+++ include/lldb/Utility/FileSpec.h
@@ -61,7 +61,7 @@
//--
class FileSpec {
public:
- enum PathSyntax {
+ enum PathSyntax : unsigned char {
ePathSyntaxPosix,
ePathSyntaxWindows,
ePathSyntaxHostNative
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39578: Fix a couple of self-assignments using memcpy.
hintonda created this revision.
These two methods are essentially assignents, but don't check
for self-assignment and use memcpy for member variables.
Since they aren't actually operator=(), it's unclear what should be done:
- check and shortcut
- assert
- allow but s/memcpy/memmove/
https://reviews.llvm.org/D39578
Files:
source/Core/RegisterValue.cpp
source/Core/Value.cpp
Index: source/Core/Value.cpp
===
--- source/Core/Value.cpp
+++ source/Core/Value.cpp
@@ -143,6 +143,8 @@
}
size_t Value::AppendDataToHostBuffer(const Value &rhs) {
+ // FIXME: What should we do if this == &rhs?
+ // If we allow, change s/memcpy/memmove/ below.
size_t curr_size = m_data_buffer.GetByteSize();
Status error;
switch (rhs.GetValueType()) {
Index: source/Core/RegisterValue.cpp
===
--- source/Core/RegisterValue.cpp
+++ source/Core/RegisterValue.cpp
@@ -539,6 +539,11 @@
}
bool RegisterValue::CopyValue(const RegisterValue &rhs) {
+ // Maintain current behavior, but guard against self-assignment (see memcpy
+ // below).?
+ if (this == &rhs)
+return rhs.m_type == eTypeInvalid ? false : true;
+
m_type = rhs.m_type;
switch (m_type) {
case eTypeInvalid:
Index: source/Core/Value.cpp
===
--- source/Core/Value.cpp
+++ source/Core/Value.cpp
@@ -143,6 +143,8 @@
}
size_t Value::AppendDataToHostBuffer(const Value &rhs) {
+ // FIXME: What should we do if this == &rhs?
+ // If we allow, change s/memcpy/memmove/ below.
size_t curr_size = m_data_buffer.GetByteSize();
Status error;
switch (rhs.GetValueType()) {
Index: source/Core/RegisterValue.cpp
===
--- source/Core/RegisterValue.cpp
+++ source/Core/RegisterValue.cpp
@@ -539,6 +539,11 @@
}
bool RegisterValue::CopyValue(const RegisterValue &rhs) {
+ // Maintain current behavior, but guard against self-assignment (see memcpy
+ // below).?
+ if (this == &rhs)
+return rhs.m_type == eTypeInvalid ? false : true;
+
m_type = rhs.m_type;
switch (m_type) {
case eTypeInvalid:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39580: Quash a few 'warning: comparison of function 'compression_decode_buffer' not equal to a null pointer is always true [-Wtautologica l-pointer-compare]'
hintonda created this revision.
Quash several warnings of this type using clang's suggested fix:
[2741/3631] Building CXX object
tools/lldb/source/Plugins/Process/gdb-remote/CMakeFiles/lldbPluginProcessGDBRemote.dir/GDBRemoteCommunication.cpp.o
/Users/dhinton/projects/llvm_project/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:603:7:
warning: comparison of function 'compression_decode_buffer' not equal to a
null pointer is always true [-Wtautological-pointer-compare]
if (compression_decode_buffer != NULL &&
^
/Users/dhinton/projects/llvm_project/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:603:7:
note: prefix with the address-of operator to silence this warning
if (compression_decode_buffer != NULL &&
^
&
https://reviews.llvm.org/D39580
Files:
source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1024,7 +1024,7 @@
#if defined(HAVE_LIBCOMPRESSION)
// libcompression is weak linked so test if compression_decode_buffer() is
// available
- if (compression_decode_buffer != NULL &&
+ if (&compression_decode_buffer != NULL &&
avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "lzfse") {
@@ -1039,7 +1039,7 @@
#if defined(HAVE_LIBCOMPRESSION)
// libcompression is weak linked so test if compression_decode_buffer() is
// available
- if (compression_decode_buffer != NULL &&
+ if (&compression_decode_buffer != NULL &&
avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "zlib-deflate") {
@@ -1066,7 +1066,7 @@
#if defined(HAVE_LIBCOMPRESSION)
// libcompression is weak linked so test if compression_decode_buffer() is
// available
- if (compression_decode_buffer != NULL &&
+ if (&compression_decode_buffer != NULL &&
avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "lz4") {
@@ -1081,7 +1081,7 @@
#if defined(HAVE_LIBCOMPRESSION)
// libcompression is weak linked so test if compression_decode_buffer() is
// available
- if (compression_decode_buffer != NULL &&
+ if (&compression_decode_buffer != NULL &&
avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "lzma") {
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -600,7 +600,7 @@
#if defined(HAVE_LIBCOMPRESSION)
// libcompression is weak linked so check that compression_decode_buffer() is
// available
- if (compression_decode_buffer != NULL &&
+ if (&compression_decode_buffer != NULL &&
(m_compression_type == CompressionType::ZlibDeflate ||
m_compression_type == CompressionType::LZFSE ||
m_compression_type == CompressionType::LZ4)) {
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1024,7 +1024,7 @@
#if defined(HAVE_LIBCOMPRESSION)
// libcompression is weak linked so test if compression_decode_buffer() is
// available
- if (compression_decode_buffer != NULL &&
+ if (&compression_decode_buffer != NULL &&
avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "lzfse") {
@@ -1039,7 +1039,7 @@
#if defined(HAVE_LIBCOMPRESSION)
// libcompression is weak linked so test if compression_decode_buffer() is
// available
- if (compression_decode_buffer != NULL &&
+ if (&compression_decode_buffer != NULL &&
avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "zlib-deflate") {
@@ -1066,7 +1066,7 @@
#if defined(HAVE_LIBCOMPRESSION)
// libcompression is weak linked so test if compression_decode_buffer() is
// available
- if (compression_decode_buffer != NULL &&
+ if (&compression_decode_buffer != NULL &&
avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "lz4") {
@@ -1081,7 +1081,7 @@
#if defined(HAVE_LIBCOMPRESSION)
// libcompression is weak l
