[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)

2024-08-03 Thread Dmitry Vasilyev via lldb-commits

https://github.com/slydiman edited 
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)

2024-08-03 Thread Dmitry Vasilyev via lldb-commits

https://github.com/slydiman edited 
https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [lldb] Fixed PipeWindows bugs; added Pipe::WriteWithTimeout() (PR #101383)

2024-08-03 Thread Dmitry Vasilyev via lldb-commits

slydiman wrote:

I have reverted the original `PipeWindows::CreateNew()` removed in this 
[commit](https://github.com/llvm/llvm-project/commit/e55850be23a09969a3a619c4767d86cd532b1006).
 Note the following comment was here at the beginning and it is correct. See 
also [here for more 
details](https://stackoverflow.com/questions/60645/overlapped-i-o-on-anonymous-pipe).

> Even for anonymous pipes, we open a named pipe. This is because you cannot 
> get overlapped i/o on Windows without using a named pipe.  So we synthesize a 
> unique name.

We cannot use ::CreatePipe() because this pipe does not support async I/O. 

Note the inheritance did not work for name pipes on Windows because of missing 
or incorrectly initialized SECURITY_ATTRIBUTES.

Note ReadWithTimeout() never worked as expected because of `timeout = 
duration.count() * 1000;` instead of `timeout = duration.count() / 1000;`. 

I have also fixed the logic of error checking.


https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] [WIP] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #101778)

2024-08-03 Thread Michael Buch via lldb-commits

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/101778

>From 3d7c2b87ec73a48de30e1c5387a7f0d8d817b0f4 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 2 Aug 2024 23:38:18 +0100
Subject: [PATCH 1/2] [lldb][ClangExpressionParser] Set
 BuiltinHeadersInSystemModules depending on SDK version

This patch addresses an issue that will arise with future
SDKs. The ClangExpressionParser currently unconditionally
sets `-fbuiltin-headers-in-system-modules` when evaluating
expressions with the `target.import-std-module` setting.
This flag should, however, be set depending on the SDK
version (which is what the Clang Darwin toolchain does).

Unfortunately, the compiler instance that we create with
`ClangExpressionParser` never consults the Clang driver,
and thus doesn't correctly infer `BuiltinHeadersInSystemModules`.
Note, this isn't an issue with the `CompilerInstance` that the
`ClangModulesDeclVendor` creates because it uses the `createInovcation`
API, which calls into `Darwin::addClangTargetOptions`.

This patch mimicks how `sdkSupportsBuiltinModules` is used in
`Darwin::addClangTargetOptions`.

This ensures that the `import-std-module` API tests run cleanly
regardless of SDK version.

The plan is to make the `CompilerInstance` construction in
`ClangExpressionParser` go through the driver, so we can avoid
duplicating the logic in LLDB.
---
 .../Clang/ClangExpressionParser.cpp   | 69 ++-
 1 file changed, 66 insertions(+), 3 deletions(-)

diff --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 2a8bdf29314e4..c6217e2f13394 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -11,6 +11,7 @@
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/Basic/DarwinSDKInfo.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TargetInfo.h"
@@ -561,7 +562,62 @@ static void SetupLangOpts(CompilerInstance &compiler,
   lang_opts.NoBuiltin = true;
 }
 
-static void SetupImportStdModuleLangOpts(CompilerInstance &compiler) {
+// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
+// Toolchains/Darwin.cpp
+static bool
+sdkSupportsBuiltinModulesImpl(const llvm::Triple &triple,
+  const std::optional &SDKInfo) {
+  if (!SDKInfo)
+return false;
+
+  VersionTuple SDKVersion = SDKInfo->getVersion();
+  switch (triple.getOS()) {
+  case Triple::OSType::MacOSX:
+return SDKVersion >= VersionTuple(15U);
+  case Triple::OSType::IOS:
+return SDKVersion >= VersionTuple(18U);
+  case Triple::OSType::TvOS:
+return SDKVersion >= VersionTuple(18U);
+  case Triple::OSType::WatchOS:
+return SDKVersion >= VersionTuple(11U);
+  case Triple::OSType::XROS:
+return SDKVersion >= VersionTuple(2U);
+  default:
+// New SDKs support builtin modules from the start.
+return true;
+  }
+}
+
+static bool
+sdkSupportsBuiltinModules(llvm::Triple const &triple,
+  std::vector const &include_dirs) {
+  static constexpr std::string_view s_sdk_suffix = ".sdk";
+  auto it = llvm::find_if(include_dirs, [](std::string const &path) {
+return path.find(s_sdk_suffix) != std::string::npos;
+  });
+  if (it == include_dirs.end())
+return false;
+
+  size_t suffix = it->find(s_sdk_suffix);
+  if (suffix == std::string::npos)
+return false;
+
+  auto VFS = FileSystem::Instance().GetVirtualFileSystem();
+  if (!VFS)
+return false;
+
+  std::string sdk_path = it->substr(0, suffix + s_sdk_suffix.size());
+  auto parsed = clang::parseDarwinSDKInfo(*VFS, sdk_path);
+  if (!parsed)
+return false;
+
+  return sdkSupportsBuiltinModulesImpl(triple, *parsed);
+}
+
+static void
+SetupImportStdModuleLangOpts(CompilerInstance &compiler,
+ llvm::Triple const &triple,
+ std::vector const &include_dirs) {
   LangOptions &lang_opts = compiler.getLangOpts();
   lang_opts.Modules = true;
   // We want to implicitly build modules.
@@ -578,7 +634,12 @@ static void SetupImportStdModuleLangOpts(CompilerInstance 
&compiler) {
   lang_opts.GNUMode = true;
   lang_opts.GNUKeywords = true;
   lang_opts.CPlusPlus11 = true;
-  lang_opts.BuiltinHeadersInSystemModules = true;
+
+  // FIXME: We should use the driver to derive this for use.
+  // ClangModulesDeclVendor already parses the SDKSettings for the purposes of
+  // this check.
+  lang_opts.BuiltinHeadersInSystemModules =
+  !sdkSupportsBuiltinModules(triple, include_dirs);
 
   // The Darwin libc expects this macro to be set.
   lang_opts.GNUCVersion = 40201;
@@ -663,7 +724,9 @@ ClangExpressionParser::ClangExpressionParser(
   if (auto *clang_expr = dyn_cast(&m_expr);
   clang_ex

[Lldb-commits] [lldb] [WIP] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #101778)

2024-08-03 Thread Michael Buch via lldb-commits

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/101778

>From 3d7c2b87ec73a48de30e1c5387a7f0d8d817b0f4 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 2 Aug 2024 23:38:18 +0100
Subject: [PATCH 1/5] [lldb][ClangExpressionParser] Set
 BuiltinHeadersInSystemModules depending on SDK version

This patch addresses an issue that will arise with future
SDKs. The ClangExpressionParser currently unconditionally
sets `-fbuiltin-headers-in-system-modules` when evaluating
expressions with the `target.import-std-module` setting.
This flag should, however, be set depending on the SDK
version (which is what the Clang Darwin toolchain does).

Unfortunately, the compiler instance that we create with
`ClangExpressionParser` never consults the Clang driver,
and thus doesn't correctly infer `BuiltinHeadersInSystemModules`.
Note, this isn't an issue with the `CompilerInstance` that the
`ClangModulesDeclVendor` creates because it uses the `createInovcation`
API, which calls into `Darwin::addClangTargetOptions`.

This patch mimicks how `sdkSupportsBuiltinModules` is used in
`Darwin::addClangTargetOptions`.

This ensures that the `import-std-module` API tests run cleanly
regardless of SDK version.

The plan is to make the `CompilerInstance` construction in
`ClangExpressionParser` go through the driver, so we can avoid
duplicating the logic in LLDB.
---
 .../Clang/ClangExpressionParser.cpp   | 69 ++-
 1 file changed, 66 insertions(+), 3 deletions(-)

diff --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 2a8bdf29314e4..c6217e2f13394 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -11,6 +11,7 @@
 #include "clang/AST/ExternalASTSource.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/Basic/DarwinSDKInfo.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TargetInfo.h"
@@ -561,7 +562,62 @@ static void SetupLangOpts(CompilerInstance &compiler,
   lang_opts.NoBuiltin = true;
 }
 
-static void SetupImportStdModuleLangOpts(CompilerInstance &compiler) {
+// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
+// Toolchains/Darwin.cpp
+static bool
+sdkSupportsBuiltinModulesImpl(const llvm::Triple &triple,
+  const std::optional &SDKInfo) {
+  if (!SDKInfo)
+return false;
+
+  VersionTuple SDKVersion = SDKInfo->getVersion();
+  switch (triple.getOS()) {
+  case Triple::OSType::MacOSX:
+return SDKVersion >= VersionTuple(15U);
+  case Triple::OSType::IOS:
+return SDKVersion >= VersionTuple(18U);
+  case Triple::OSType::TvOS:
+return SDKVersion >= VersionTuple(18U);
+  case Triple::OSType::WatchOS:
+return SDKVersion >= VersionTuple(11U);
+  case Triple::OSType::XROS:
+return SDKVersion >= VersionTuple(2U);
+  default:
+// New SDKs support builtin modules from the start.
+return true;
+  }
+}
+
+static bool
+sdkSupportsBuiltinModules(llvm::Triple const &triple,
+  std::vector const &include_dirs) {
+  static constexpr std::string_view s_sdk_suffix = ".sdk";
+  auto it = llvm::find_if(include_dirs, [](std::string const &path) {
+return path.find(s_sdk_suffix) != std::string::npos;
+  });
+  if (it == include_dirs.end())
+return false;
+
+  size_t suffix = it->find(s_sdk_suffix);
+  if (suffix == std::string::npos)
+return false;
+
+  auto VFS = FileSystem::Instance().GetVirtualFileSystem();
+  if (!VFS)
+return false;
+
+  std::string sdk_path = it->substr(0, suffix + s_sdk_suffix.size());
+  auto parsed = clang::parseDarwinSDKInfo(*VFS, sdk_path);
+  if (!parsed)
+return false;
+
+  return sdkSupportsBuiltinModulesImpl(triple, *parsed);
+}
+
+static void
+SetupImportStdModuleLangOpts(CompilerInstance &compiler,
+ llvm::Triple const &triple,
+ std::vector const &include_dirs) {
   LangOptions &lang_opts = compiler.getLangOpts();
   lang_opts.Modules = true;
   // We want to implicitly build modules.
@@ -578,7 +634,12 @@ static void SetupImportStdModuleLangOpts(CompilerInstance 
&compiler) {
   lang_opts.GNUMode = true;
   lang_opts.GNUKeywords = true;
   lang_opts.CPlusPlus11 = true;
-  lang_opts.BuiltinHeadersInSystemModules = true;
+
+  // FIXME: We should use the driver to derive this for use.
+  // ClangModulesDeclVendor already parses the SDKSettings for the purposes of
+  // this check.
+  lang_opts.BuiltinHeadersInSystemModules =
+  !sdkSupportsBuiltinModules(triple, include_dirs);
 
   // The Darwin libc expects this macro to be set.
   lang_opts.GNUCVersion = 40201;
@@ -663,7 +724,9 @@ ClangExpressionParser::ClangExpressionParser(
   if (auto *clang_expr = dyn_cast(&m_expr);
   clang_ex