lawrence_danna updated this revision to Diff 226618.
lawrence_danna added a comment.
protect python from being exposed to C++ reference borrowing semantics
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69488/new/
https://reviews.llvm.org/D69488
Files:
lldb/include/lldb/Host/File.h
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<char *>(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
enum class PyInitialValue { Invalid, Empty };
template <typename T, typename Enable = void> 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<char *>(s); }
-#else
- static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
public:
template <typename... T>
llvm::Expected<PythonObject> 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,28 @@
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<char *>(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<char *>(""), cmode, ::fflush);
+ // It's more or less safe to let a python program borrow a file descriptor.
+ // If they let it dangle and then use it, they'll probably get an exception.
+ // The worst that can happen is they'll wind up doing IO on the wrong
+ // descriptor. But it would be very unsafe to let a python program borrow
+ // a FILE*. They could actually crash the program just by keeping a
+ // reference to it around. Since in python 2 we must have a FILE* and
+ // not a descriptor, we dup the descriptor and fdopen a new FILE* to it
+ // so python can have something it can own safely.
+ auto opts = File::GetOptionsFromMode(mode);
+ if (!opts)
+ return opts.takeError();
+ int fd = file.GetDescriptor();
+ if (!File::DescriptorIsValid(fd))
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "File has no file descriptor");
+ NativeFile dupfile(fd, opts.get(), false);
+ FILE *stream = NativeFile::ReleaseFILE(std::move(dupfile));
+ if (!stream)
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "could not create FILE* stream");
+ file_obj = PyFile_FromFile(stream, py2_const_cast(""), py2_const_cast(mode),
+ ::fclose);
#endif
if (!file_obj)
Index: lldb/source/Host/common/File.cpp
===================================================================
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -304,13 +304,25 @@
return m_stream;
}
+FILE *NativeFile::ReleaseFILE(NativeFile &&file) {
+ FILE *stream = nullptr;
+ file.GetStream();
+ if (file.m_own_stream) {
+ stream = file.m_stream;
+ file.m_own_stream = false;
+ file.m_stream = nullptr;
+ }
+ file.Close();
+ return stream;
+}
+
Status NativeFile::Close() {
Status error;
if (StreamIsValid()) {
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,6 +845,7 @@
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
Index: lldb/include/lldb/Host/File.h
===================================================================
--- lldb/include/lldb/Host/File.h
+++ lldb/include/lldb/Host/File.h
@@ -411,6 +411,8 @@
size_t PrintfVarArg(const char *format, va_list args) override;
llvm::Expected<OpenOptions> GetOptions() const override;
+ static FILE *ReleaseFILE(NativeFile &&file);
+
static char ID;
virtual bool isA(const void *classID) const override {
return classID == &ID || File::isA(classID);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
