[Lldb-commits] [PATCH] D157589: [lldb] Fix building with latest libc++

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: JDevlieghere, mib, jingham.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

Since https://reviews.llvm.org/D157058 in libc++,
the base template for char_traits has been removed - it is only
provided for char, wchar_t, char8_t, char16_t and char32_t.
(Thus, to use basic_string with a type other than those, we'd need
to supply suitable traits ourselves.)

For this particular use, a vector works just as well as basic_string.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157589

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp


Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -631,7 +631,7 @@
   } else {
 // Zero-out any unreadable values.
 if (reg_info.byte_size > 0) {
-  std::basic_string zeros(reg_info.byte_size, '\0');
+  std::vector zeros(reg_info.byte_size, '\0');
   AppendHexValue(response, zeros.data(), zeros.size(), false);
 }
   }


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -631,7 +631,7 @@
   } else {
 // Zero-out any unreadable values.
 if (reg_info.byte_size > 0) {
-  std::basic_string zeros(reg_info.byte_size, '\0');
+  std::vector zeros(reg_info.byte_size, '\0');
   AppendHexValue(response, zeros.data(), zeros.size(), false);
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157589: [lldb] Fix building with latest libc++

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG68744ffbdd7d: [lldb] Fix building with latest libc++ 
(authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157589

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp


Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -631,7 +631,7 @@
   } else {
 // Zero-out any unreadable values.
 if (reg_info.byte_size > 0) {
-  std::basic_string zeros(reg_info.byte_size, '\0');
+  std::vector zeros(reg_info.byte_size, '\0');
   AppendHexValue(response, zeros.data(), zeros.size(), false);
 }
   }


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -631,7 +631,7 @@
   } else {
 // Zero-out any unreadable values.
 if (reg_info.byte_size > 0) {
-  std::basic_string zeros(reg_info.byte_size, '\0');
+  std::vector zeros(reg_info.byte_size, '\0');
   AppendHexValue(response, zeros.data(), zeros.size(), false);
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158391: [lldb][debugserver] Fix build after libcxx removed generic char_traits implementation

[Martin Storsjö via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158391

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

[Lldb-commits] [PATCH] D156817: [lldb][windows] _wsopen_s does not accept bits other than `_S_IREAD | _S_IWRITE`

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

I noticed this review is accepted but not committed. If you don't have commit 
access, what's your preferred git identity, i.e. `Real Name `?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156817

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

[Lldb-commits] [PATCH] D156817: [lldb][windows] _wsopen_s does not accept bits other than `_S_IREAD | _S_IWRITE`

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a4b3fdb8232: [lldb][windows] _wsopen_s does not accept bits 
other than `_S_IREAD | _S_IWRITE` (authored by yshui, committed by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D156817?vs=546133&id=554679#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156817

Files:
  lldb/source/Host/windows/FileSystem.cpp


Index: lldb/source/Host/windows/FileSystem.cpp
===
--- lldb/source/Host/windows/FileSystem.cpp
+++ lldb/source/Host/windows/FileSystem.cpp
@@ -101,6 +101,8 @@
   std::wstring wpath;
   if (!llvm::ConvertUTF8toWide(path, wpath))
 return -1;
+  // All other bits are rejected by _wsopen_s
+  mode = mode & (_S_IREAD | _S_IWRITE);
   int result;
   ::_wsopen_s(&result, wpath.c_str(), flags, _SH_DENYNO, mode);
   return result;


Index: lldb/source/Host/windows/FileSystem.cpp
===
--- lldb/source/Host/windows/FileSystem.cpp
+++ lldb/source/Host/windows/FileSystem.cpp
@@ -101,6 +101,8 @@
   std::wstring wpath;
   if (!llvm::ConvertUTF8toWide(path, wpath))
 return -1;
+  // All other bits are rejected by _wsopen_s
+  mode = mode & (_S_IREAD | _S_IWRITE);
   int result;
   ::_wsopen_s(&result, wpath.c_str(), flags, _SH_DENYNO, mode);
   return result;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116467: Deprecate `LLVM_LIBRARY_DIRS`

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added subscribers: beanz, mstorsjo.
mstorsjo added a reviewer: beanz.
mstorsjo added a comment.

This looks reasonable to me, but I'm not sure if I know all the aspects that 
might be at play here, so I don't think I'm comfortable with accepting it on my 
own. Adding @beanz who should know lots about our cmake setup too.




Comment at: llvm/cmake/modules/CMakeLists.txt:58
 set(LLVM_CONFIG_TOOLS_BINARY_DIR "${LLVM_TOOLS_BINARY_DIR}")
+set(LLVM_CONFIG_LIBRARY_DIR "${LLVM_LIBRARY_DIR}")
 

Curious: Why the reordering? I presume you're moving `LLVM_CONFIG_CMAKE_DIR` 
into alphabetical order, but then `LLVM_CONFIG_LIBRARY_DIR` seems out of place?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116467

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

[Lldb-commits] [PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

This broke building ANGLE as part of Qt 5.15 for a mingw target, with the 
following error:

  
../../../3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.cpp:532:38:
 error: explicit instantiation of 'allocate' does not refer to a function 
template, variable template, member function, member class, or static data 
member
  ANGLE_RESOURCE_TYPE_OP(Instantitate, ANGLE_INSTANTIATE_OP)
   ^
  
../../../3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.h:301:15:
 note: candidate template ignored: could not match 'GetInitDataFromD3D11' 
(aka 'typename 
InitDataType::Value>::Value') against 
'const D3D11_SUBRESOURCE_DATA' 
  gl::Error allocate(Renderer11 *renderer,
^
  
../../../3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.cpp:358:30:
 note: candidate template ignored: could not match 'GetInitDataFromD3D11' 
(aka 'typename 
InitDataType::Value>::Value') against 
'const D3D11_SUBRESOURCE_DATA'
  gl::Error ResourceManager11::allocate(Renderer11 *renderer,
   ^

Do you happen to know what's going on here? The files mentioned are 
https://code.qt.io/cgit/qt/qtbase.git/tree/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.cpp?h=5.15.1
 and 
https://code.qt.io/cgit/qt/qtbase.git/tree/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.h?h=5.15.1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[Lldb-commits] [PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D110216#3130193 , @mizvekov wrote:

> In D110216#3130184 , @mstorsjo 
> wrote:
>
>> Do you happen to know what's going on here? The files mentioned are 
>> https://code.qt.io/cgit/qt/qtbase.git/tree/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.cpp?h=5.15.1
>>  and 
>> https://code.qt.io/cgit/qt/qtbase.git/tree/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/ResourceManager11.h?h=5.15.1.
>
> Thanks for the report!
>
> Not immediately obvious to me, would need some time to isolate / create a 
> minimal test case.
> Would you be in a position to extract a preprocessed source file + command 
> line which repros this error?

Sure: https://martin.st/temp/angle-preproc.cpp, built with `clang -target 
x86_64-w64-mingw32 -std=c++17 -w -c angle-preproc.cpp`.

> Also, feel free to revert if it helps you.

No big hurry for me wrt that yet, at least until we know which part is at fault 
here.

(I haven't tested with the very latest Qt and/or ANGLE - the full build setup 
for this is rather complex.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[Lldb-commits] [PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: llvm/CMakeLists.txt:75
 set(LLVM_ENABLE_PROJECTS "" CACHE STRING
-   "Semicolon-separated list of projects to build 
(${LLVM_KNOWN_PROJECTS}), or \"all\".")
+   "Semicolon-separated list of projects to build (${LLVM_KNOWN_PROJECTS}), or 
\"all\".")
 foreach(proj ${LLVM_ENABLE_PROJECTS})

Nit: This looks like a spurious unrelated change to whitespace?



Comment at: llvm/CMakeLists.txt:164
   set(LLVM_CCACHE_MAXSIZE "" CACHE STRING "Size of ccache")
-  set(LLVM_CCACHE_DIR "" CACHE STRING "Directory to keep ccached data")
+  set(LLVM_CCACHE_DIR "" CACHE PATH "Directory to keep ccached data")
   set(LLVM_CCACHE_PARAMS "CCACHE_CPP2=yes CCACHE_HASHDIR=yes"

Could this bit be split out to separate change, to keep this as small as 
possible - unless it's strictly needed and tied to this one?



Comment at: llvm/CMakeLists.txt:294
 
-set(LLVM_UTILS_INSTALL_DIR "${LLVM_TOOLS_INSTALL_DIR}" CACHE STRING
+set(LLVM_UTILS_INSTALL_DIR "${LLVM_TOOLS_INSTALL_DIR}" CACHE PATH
 "Path to install LLVM utilities (enabled by LLVM_INSTALL_UTILS=ON) 
(defaults to LLVM_TOOLS_INSTALL_DIR)")

Nit: Unrelated and can be split out?



Comment at: llvm/CMakeLists.txt:408
 
-set(LLVM_Z3_INSTALL_DIR "" CACHE STRING "Install directory of the Z3 solver.")
+set(LLVM_Z3_INSTALL_DIR "" CACHE PATH "Install directory of the Z3 solver.")
 

Unrelated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100810

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

[Lldb-commits] [PATCH] D116625: [lldb] [debugserver] Simplify handling of arch specific files

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, JDevlieghere, jasonmolenda, clayborg.
Herald added subscribers: kristof.beyls, mgorny.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

There are no duplicates among the include files, and all the
source files are wrapped in architecture ifdefs, so there's no harm
in including all of them, always.

This fixes builds if TARGET_TRIPLE is set to something else than the
build architecture.

This also allows building for multiple architectures at once by
setting CMAKE_OSX_ARCHITECTURES.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116625

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


Index: lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
===
--- lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
+++ lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
@@ -1,30 +1,8 @@
-# The debugserver build needs to conditionally include files depending on the
-# target architecture.
-#
-# Switch on the architecture specified by TARGET_TRIPLE, as
-# the llvm and swift build systems use this variable to identify the
-# target (through LLVM_HOST_TRIPLE).
-#
-# It would be possible to switch on CMAKE_OSX_ARCHITECTURES, but the swift
-# build does not provide it, preferring instead to pass arch-specific
-# CFLAGS etc explicitly. Switching on LLVM_HOST_TRIPLE is also an option,
-# but it breaks down when cross-compiling.
+list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
 
-if(TARGET_TRIPLE)
-  string(REGEX MATCH "^[^-]*" LLDB_DEBUGSERVER_ARCH ${TARGET_TRIPLE})
-else()
-  set(LLDB_DEBUGSERVER_ARCH ${CMAKE_OSX_ARCHITECTURES})
-endif()
-
-if("${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*arm.*")
-  list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
-endif()
-
-if(NOT LLDB_DEBUGSERVER_ARCH OR "${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*86.*")
-  list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
-endif()
+list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
 
 include_directories(..)
 


Index: lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
===
--- lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
+++ lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
@@ -1,30 +1,8 @@
-# The debugserver build needs to conditionally include files depending on the
-# target architecture.
-#
-# Switch on the architecture specified by TARGET_TRIPLE, as
-# the llvm and swift build systems use this variable to identify the
-# target (through LLVM_HOST_TRIPLE).
-#
-# It would be possible to switch on CMAKE_OSX_ARCHITECTURES, but the swift
-# build does not provide it, preferring instead to pass arch-specific
-# CFLAGS etc explicitly. Switching on LLVM_HOST_TRIPLE is also an option,
-# but it breaks down when cross-compiling.
+list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
 
-if(TARGET_TRIPLE)
-  string(REGEX MATCH "^[^-]*" LLDB_DEBUGSERVER_ARCH ${TARGET_TRIPLE})
-else()
-  set(LLDB_DEBUGSERVER_ARCH ${CMAKE_OSX_ARCHITECTURES})
-endif()
-
-if("${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*arm.*")
-  list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
-endif()
-
-if(NOT LLDB_DEBUGSERVER_ARCH OR "${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*86.*")
-  list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
-endif()
+list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
 
 include_directories(..)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116625: [lldb] [debugserver] Simplify handling of arch specific files

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1919720fdd34: [lldb] [debugserver] Simplify handling of arch 
specific files (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116625

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


Index: lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
===
--- lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
+++ lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
@@ -1,30 +1,8 @@
-# The debugserver build needs to conditionally include files depending on the
-# target architecture.
-#
-# Switch on the architecture specified by TARGET_TRIPLE, as
-# the llvm and swift build systems use this variable to identify the
-# target (through LLVM_HOST_TRIPLE).
-#
-# It would be possible to switch on CMAKE_OSX_ARCHITECTURES, but the swift
-# build does not provide it, preferring instead to pass arch-specific
-# CFLAGS etc explicitly. Switching on LLVM_HOST_TRIPLE is also an option,
-# but it breaks down when cross-compiling.
+list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
 
-if(TARGET_TRIPLE)
-  string(REGEX MATCH "^[^-]*" LLDB_DEBUGSERVER_ARCH ${TARGET_TRIPLE})
-else()
-  set(LLDB_DEBUGSERVER_ARCH ${CMAKE_OSX_ARCHITECTURES})
-endif()
-
-if("${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*arm.*")
-  list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
-endif()
-
-if(NOT LLDB_DEBUGSERVER_ARCH OR "${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*86.*")
-  list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
-endif()
+list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
 
 include_directories(..)
 


Index: lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
===
--- lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
+++ lldb/tools/debugserver/source/MacOSX/CMakeLists.txt
@@ -1,30 +1,8 @@
-# The debugserver build needs to conditionally include files depending on the
-# target architecture.
-#
-# Switch on the architecture specified by TARGET_TRIPLE, as
-# the llvm and swift build systems use this variable to identify the
-# target (through LLVM_HOST_TRIPLE).
-#
-# It would be possible to switch on CMAKE_OSX_ARCHITECTURES, but the swift
-# build does not provide it, preferring instead to pass arch-specific
-# CFLAGS etc explicitly. Switching on LLVM_HOST_TRIPLE is also an option,
-# but it breaks down when cross-compiling.
+list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
 
-if(TARGET_TRIPLE)
-  string(REGEX MATCH "^[^-]*" LLDB_DEBUGSERVER_ARCH ${TARGET_TRIPLE})
-else()
-  set(LLDB_DEBUGSERVER_ARCH ${CMAKE_OSX_ARCHITECTURES})
-endif()
-
-if("${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*arm.*")
-  list(APPEND SOURCES arm/DNBArchImpl.cpp arm64/DNBArchImplARM64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/arm ${CURRENT_SOURCE_DIR}/arm64)
-endif()
-
-if(NOT LLDB_DEBUGSERVER_ARCH OR "${LLDB_DEBUGSERVER_ARCH}" MATCHES ".*86.*")
-  list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
-  include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
-endif()
+list(APPEND SOURCES i386/DNBArchImplI386.cpp x86_64/DNBArchImplX86_64.cpp)
+include_directories(${CURRENT_SOURCE_DIR}/i386 ${CURRENT_SOURCE_DIR}/x86_64)
 
 include_directories(..)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D118405: [LLD][MinGW] Add --heap argument support

[Martin Storsjö via Phabricator via lldb-commits]

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

Thanks, I tested it and it seems to work fine. (FWIW the comma-separated form 
of the option is pretty hard to pass via `-Wl` through the compiler driver, but 
with `-Xlinker --heap -Xlinker 1234,5678` it worked.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118405

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

[Lldb-commits] [PATCH] D118405: [LLD][MinGW] Add --heap argument support

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG460830a9c664: [LLD][MinGW] Add --heap argument support 
(authored by mati865, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118405

Files:
  lld/MinGW/Driver.cpp
  lld/MinGW/Options.td
  lld/test/MinGW/driver.test


Index: lld/test/MinGW/driver.test
===
--- lld/test/MinGW/driver.test
+++ lld/test/MinGW/driver.test
@@ -340,3 +340,8 @@
 
 RUN: ld.lld -### -m i386pep foo.o --disable-stdcall-fixup 
--enable-stdcall-fixup 2>&1 | FileCheck -check-prefix ENABLE-FIXUP %s
 ENABLE-FIXUP: -stdcall-fixup{{ }}
+
+RUN: ld.lld -### foo.o -m i386pep -heap 8388608,16384 2>&1 | FileCheck 
-check-prefix=HEAP %s
+RUN: ld.lld -### foo.o -m i386pep --heap 8388608,16384 2>&1 | FileCheck 
-check-prefix=HEAP %s
+RUN: ld.lld -### foo.o -m i386pep --heap=8388608,16384 2>&1 | FileCheck 
-check-prefix=HEAP %s
+HEAP: -heap:8388608,16384
Index: lld/MinGW/Options.td
===
--- lld/MinGW/Options.td
+++ lld/MinGW/Options.td
@@ -66,6 +66,7 @@
 defm gc_sections: B<"gc-sections",
 "Remove unused sections",
 "Don't remove unused sections">;
+defm heap: Eq<"heap", "Set size of the initial heap">;
 def help: F<"help">, HelpText<"Print option help">;
 defm high_entropy_va: B_disable<"high-entropy-va",
 "Set the 'high entropy VA' flag", "Don't set the 'high entropy VA' flag">;
Index: lld/MinGW/Driver.cpp
===
--- lld/MinGW/Driver.cpp
+++ lld/MinGW/Driver.cpp
@@ -264,6 +264,8 @@
 add("-filealign:" + StringRef(a->getValue()));
   if (auto *a = args.getLastArg(OPT_section_alignment))
 add("-align:" + StringRef(a->getValue()));
+  if (auto *a = args.getLastArg(OPT_heap))
+add("-heap:" + StringRef(a->getValue()));
 
   if (auto *a = args.getLastArg(OPT_o))
 add("-out:" + StringRef(a->getValue()));


Index: lld/test/MinGW/driver.test
===
--- lld/test/MinGW/driver.test
+++ lld/test/MinGW/driver.test
@@ -340,3 +340,8 @@
 
 RUN: ld.lld -### -m i386pep foo.o --disable-stdcall-fixup --enable-stdcall-fixup 2>&1 | FileCheck -check-prefix ENABLE-FIXUP %s
 ENABLE-FIXUP: -stdcall-fixup{{ }}
+
+RUN: ld.lld -### foo.o -m i386pep -heap 8388608,16384 2>&1 | FileCheck -check-prefix=HEAP %s
+RUN: ld.lld -### foo.o -m i386pep --heap 8388608,16384 2>&1 | FileCheck -check-prefix=HEAP %s
+RUN: ld.lld -### foo.o -m i386pep --heap=8388608,16384 2>&1 | FileCheck -check-prefix=HEAP %s
+HEAP: -heap:8388608,16384
Index: lld/MinGW/Options.td
===
--- lld/MinGW/Options.td
+++ lld/MinGW/Options.td
@@ -66,6 +66,7 @@
 defm gc_sections: B<"gc-sections",
 "Remove unused sections",
 "Don't remove unused sections">;
+defm heap: Eq<"heap", "Set size of the initial heap">;
 def help: F<"help">, HelpText<"Print option help">;
 defm high_entropy_va: B_disable<"high-entropy-va",
 "Set the 'high entropy VA' flag", "Don't set the 'high entropy VA' flag">;
Index: lld/MinGW/Driver.cpp
===
--- lld/MinGW/Driver.cpp
+++ lld/MinGW/Driver.cpp
@@ -264,6 +264,8 @@
 add("-filealign:" + StringRef(a->getValue()));
   if (auto *a = args.getLastArg(OPT_section_alignment))
 add("-align:" + StringRef(a->getValue()));
+  if (auto *a = args.getLastArg(OPT_heap))
+add("-heap:" + StringRef(a->getValue()));
 
   if (auto *a = args.getLastArg(OPT_o))
 add("-out:" + StringRef(a->getValue()));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128410: [lldb] Add a testcase for nested exceptions on Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D128410#3608190 , @labath wrote:

> In D128410#3604927 , @alvinhochun 
> wrote:
>
>> It may be possible to stuff a pointer to an `EXCEPTION_RECORD` into another 
>> `EXCEPTION_RECORD` and use `RtlRaiseException` to generate the exception, 
>> but you'll have to test how it actually works.
>
> That would be pretty cool.

Yeah - I guess it's two separate kinds of testcases; this one would be more of 
a macro-testcase, "does this real-world case work - whichever way lldb happens 
to handle it" (nested exception or not?) while that would be more of a clinical 
unit test for specifically testing nested exceptions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128410

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D128268#3608197 , @labath wrote:

> In D128268#3604555 , @mstorsjo 
> wrote:
>
>> I found that this duality was introduced in 
>> 5e6f45201f0b62c1e7a24fc396f3ea6e10dc880d / D7120 
>>  and 
>> ad587ae4ca143d388c0ec4ef2faa1b5eddedbf67 / D4658 
>>  (CC @zturner), what do you make out of the 
>> reasonings in those commits?
>
> The first patch seems like it's just working around some mismatches in the 
> windows dynamic loader plugin. I think a better approach would be to have the 
> dynamic loader claim both architectures, though I don't think that is 
> necessary if we're just consistent about what we use. I don't see anything 
> wrong with the second patch (the darwin platform does something similar for 
> arm architectures, even though I'm not convinced it's necessary (the reason 
> it's necessary for darwin is because there they actually make a distinction 
> between armv6XX and armv7YY and treat those as different architectures).

The odd thing about the second one is the added hardcoded 
`AddArch(ArchSpec("i686-pc-windows"));` and 
`AddArch(ArchSpec("i386-pc-windows"));` at the top of `PlatformWindows.cpp`.

Anyway, it does seem to work fine in my quick test to get rid of this duality, 
so I'll post a patch that does that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128617: [lldb] Stop passing both i386 and i686 in parallel as architectures on Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, zturner, DavidSpickett.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

When an object file returns multiple architectures, it is treated
as a fat binary - which really isn't the case of i386 vs i686 where
the object file actually has one architecture.

This allows getting rid of hardcoded architecture triples in
PlatformWindows.

The parallel i386 and i686 architecture strings stem from
5e6f45201f0b62c1e7a24fc396f3ea6e10dc880d / D7120 
 and
ad587ae4ca143d388c0ec4ef2faa1b5eddedbf67 / D4658 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128617

Files:
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml


Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -18,7 +18,7 @@
 # RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
 
 # CHECK-LABEL: image list --triple --basename
-# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+# CHECK-NEXT: i386-pc-windows-[[ABI]] [[FILENAME]]
 
 --- !COFF
 OptionalHeader:
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -124,11 +124,9 @@
 if (spec.IsValid())
   m_supported_architectures.push_back(spec);
   };
-  AddArch(ArchSpec("i686-pc-windows"));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKindDefault));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind32));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind64));
-  AddArch(ArchSpec("i386-pc-windows"));
 }
 
 Status PlatformWindows::ConnectRemote(Args &args) {
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -337,9 +337,6 @@
 spec.SetTriple("i386-pc-windows");
 spec.GetTriple().setEnvironment(env);
 specs.Append(module_spec);
-spec.SetTriple("i686-pc-windows");
-spec.GetTriple().setEnvironment(env);
-specs.Append(module_spec);
 break;
   case MachineArmNt:
 spec.SetTriple("armv7-pc-windows");
Index: lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
===
--- lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
+++ lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
@@ -137,8 +137,6 @@
   case PDB_Machine::x86:
 module_arch.SetTriple("i386-pc-windows");
 specs.Append(module_spec);
-module_arch.SetTriple("i686-pc-windows");
-specs.Append(module_spec);
 break;
   case PDB_Machine::ArmNT:
 module_arch.SetTriple("armv7-pc-windows");


Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -18,7 +18,7 @@
 # RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
 
 # CHECK-LABEL: image list --triple --basename
-# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+# CHECK-NEXT: i386-pc-windows-[[ABI]] [[FILENAME]]
 
 --- !COFF
 OptionalHeader:
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -124,11 +124,9 @@
 if (spec.IsValid())
   m_supported_architectures.push_back(spec);
   };
-  AddArch(ArchSpec("i686-pc-windows"));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKindDefault));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind32));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind64));
-  AddArch(ArchSpec("i386-pc-windows"));
 }
 
 Status PlatformWindows::ConnectRemote(Args &args) {
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -337,9 +337,6 @@
 spec.SetTriple("i386-pc-windows");
 spec.GetTriple().setEnvironment(env);
 specs.Append(module_spec);
-spec.SetTriple("i686-pc-windows");
-spec.GetTriple().setEnvironment(

[Lldb-commits] [PATCH] D128668: [LLDB] Fix PDB/pointers.test for 32bit Arm/Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

I presume this is ok - if the main intent of this test is to test handling of 
pointers and the size thereof in PDBs.




Comment at: lldb/test/Shell/SymbolFile/PDB/pointers.test:2
 REQUIRES: system-windows, msvc
 RUN: %build --compiler=clang-cl --mode=compile --arch=32 --nodefaultlib 
--output=%T/PointerTypeTest.cpp.obj %S/Inputs/PointerTypeTest.cpp
 RUN: %build --compiler=msvc --mode=link --arch=32 --nodefaultlib 
--output=%T/PointerTypeTest.cpp.exe %T/PointerTypeTest.cpp.obj

I wonder if it'd be better to hardcode this to build for `i386` (assuming the 
`%build` script allows that?) to keep it testing what it was originally made to 
test?


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

https://reviews.llvm.org/D128668

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

[Lldb-commits] [PATCH] D128668: [LLDB] Fix PDB/pointers.test for 32bit Arm/Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo accepted this revision.
mstorsjo added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/test/Shell/SymbolFile/PDB/pointers.test:2
 REQUIRES: system-windows, msvc
 RUN: %build --compiler=clang-cl --mode=compile --arch=32 --nodefaultlib 
--output=%T/PointerTypeTest.cpp.obj %S/Inputs/PointerTypeTest.cpp
 RUN: %build --compiler=msvc --mode=link --arch=32 --nodefaultlib 
--output=%T/PointerTypeTest.cpp.exe %T/PointerTypeTest.cpp.obj

omjavaid wrote:
> mstorsjo wrote:
> > I wonder if it'd be better to hardcode this to build for `i386` (assuming 
> > the `%build` script allows that?) to keep it testing what it was originally 
> > made to test?
> Given we expect this to pass for Windows/Arm it will reduce coverage if made 
> to run only for x86.
Right, as we probably need to link, that would require more of the host 
environment than we normally require (for other tests where we don't link 
things, we can compile things for x86 even if we're running on arm, as long as 
the x86 support is available in the compiler).

Given the structure of the lldb tests I guess this is more in line with how 
things normally are done here, so I guess this is fine.


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

https://reviews.llvm.org/D128668

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

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/Calling-conentions.test for Arm/Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

There's consistent typos about the test name in the commit subject and 
description.




Comment at: lldb/test/Shell/SymbolFile/PDB/calling-conventions-arm.test:1
+REQUIRES: system-windows, lld, (target-arm || target-aarch64) 
+RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib --output=%t.exe 
%S/Inputs/CallingConventionsTest.cpp

Hmm, I wasn't aware of the fact that you can do such `||` expressions in the 
`REQUIRES` line - I presume you've verified that this test actually does get 
picked up and executed?



Comment at: lldb/test/Shell/SymbolFile/PDB/calling-conventions-arm.test:2
+REQUIRES: system-windows, lld, (target-arm || target-aarch64) 
+RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib --output=%t.exe 
%S/Inputs/CallingConventionsTest.cpp
+RUN: lldb-test symbols -dump-ast %t.exe | FileCheck %s

Here, there's probably no need to force it to 32 bit mode - unless we expect to 
have a similar test for the undecorated mangling in aarch64 and x86_64 mode too?


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

https://reviews.llvm.org/D128678

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D128268#3614661 , @labath wrote:

> In D128268#3611053 , @mstorsjo 
> wrote:
>
>> The odd thing about the second one is the added hardcoded 
>> `AddArch(ArchSpec("i686-pc-windows"));` and 
>> `AddArch(ArchSpec("i386-pc-windows"));` at the top of `PlatformWindows.cpp`.
>>
>> Anyway, it does seem to work fine in my quick test to get rid of this 
>> duality, so I'll post a patch that does that.
>
> I think those hardcoded lines are there precisely because of the duality 
> (i.e. wanting to support both i386 and i686 regardless of what the host layer 
> reports).

Both that, and because the i386+i686 fat binary arches from ObjectFile 
triggered the more elaborate arch validation, these had to be present to make 
it work. Anyway, after getting rid of the duality, those hardcoded ones don't 
seem to be needed either.

> If that is removed, maybe all of that can be changed to  the pattern used in 
> other platform plugins 
> .

That looks reasonable!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/calling-conventions.test for Arm/Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

Thanks, this looks more complete and consistent to me now!

In D128678#3615531 , @labath wrote:

> It seems like this is not actually running the code. Can we make it such that 
> these tests are conditional on the appropriate llvm targets being enabled 
> (instead of the host being of a specific type)?

Well it does need being able to link executables for these targets, which you 
generally can't rely on I think? (I don't think the `%build` macro in general 
works as a cross compiler?)


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

https://reviews.llvm.org/D128678

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

[Lldb-commits] [PATCH] D128617: [lldb] Stop passing both i386 and i686 in parallel as architectures on Windows

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4270c9cd44f2: [lldb] Stop passing both i386 and i686 in 
parallel as architectures on Windows (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128617

Files:
  lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml


Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -18,7 +18,7 @@
 # RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
 
 # CHECK-LABEL: image list --triple --basename
-# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+# CHECK-NEXT: i386-pc-windows-[[ABI]] [[FILENAME]]
 
 --- !COFF
 OptionalHeader:
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -124,11 +124,9 @@
 if (spec.IsValid())
   m_supported_architectures.push_back(spec);
   };
-  AddArch(ArchSpec("i686-pc-windows"));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKindDefault));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind32));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind64));
-  AddArch(ArchSpec("i386-pc-windows"));
 }
 
 Status PlatformWindows::ConnectRemote(Args &args) {
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -337,9 +337,6 @@
 spec.SetTriple("i386-pc-windows");
 spec.GetTriple().setEnvironment(env);
 specs.Append(module_spec);
-spec.SetTriple("i686-pc-windows");
-spec.GetTriple().setEnvironment(env);
-specs.Append(module_spec);
 break;
   case MachineArmNt:
 spec.SetTriple("armv7-pc-windows");
Index: lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
===
--- lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
+++ lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
@@ -137,8 +137,6 @@
   case PDB_Machine::x86:
 module_arch.SetTriple("i386-pc-windows");
 specs.Append(module_spec);
-module_arch.SetTriple("i686-pc-windows");
-specs.Append(module_spec);
 break;
   case PDB_Machine::ArmNT:
 module_arch.SetTriple("armv7-pc-windows");


Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -18,7 +18,7 @@
 # RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
 
 # CHECK-LABEL: image list --triple --basename
-# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+# CHECK-NEXT: i386-pc-windows-[[ABI]] [[FILENAME]]
 
 --- !COFF
 OptionalHeader:
Index: lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
===
--- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
+++ lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
@@ -124,11 +124,9 @@
 if (spec.IsValid())
   m_supported_architectures.push_back(spec);
   };
-  AddArch(ArchSpec("i686-pc-windows"));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKindDefault));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind32));
   AddArch(HostInfo::GetArchitecture(HostInfo::eArchKind64));
-  AddArch(ArchSpec("i386-pc-windows"));
 }
 
 Status PlatformWindows::ConnectRemote(Args &args) {
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -337,9 +337,6 @@
 spec.SetTriple("i386-pc-windows");
 spec.GetTriple().setEnvironment(env);
 specs.Append(module_spec);
-spec.SetTriple("i686-pc-windows");
-spec.GetTriple().setEnvironment(env);
-specs.Append(module_spec);
 break;
   case MachineArmNt:
 spec.SetTriple("armv7-pc-windows");
Index: lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
===
--- lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
+++ lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp
@@ -137,8 +137,6 @@
   case PDB_Machine::x86:
 m

[Lldb-commits] [PATCH] D129455: [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, jasonmolenda.
Herald added a subscriber: jsji.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

This fixes https://github.com/llvm/llvm-project/issues/56095.

@labath, FWIW while making the testcase, I realized why most tests on
Windows need the %build script instead of plain %clang_host.

For this testcase, I do need to have at least symbols available
(full debug info isn't needed). On unix-like systems, a command like
"clang source.c -o exec" will produce an executable with symbols.
With MSVC and clang in MSVC mode, the default linking command won't
produce any debug info at all, no symbols, nothing. We can fix it by
passing "-fuse-ld=lld -Wl,-debug:symtab", but that breaks mingw builds
(where symbols are kept by default).

The %build script handles all the options that are needed for creating
PDB debug info with clang-cl/lld-link, and produces symbols when building
in mingw mode too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129455

Files:
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
  lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
  lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
  lldb/test/Shell/Unwind/windows-unaligned-x86_64.test


Index: lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
===
--- /dev/null
+++ lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
@@ -0,0 +1,26 @@
+# Test unwinding through stack frames that aren't aligned to 16 bytes.
+# (In real world code, this happens when unwinding through
+# KiUserExceptionDispatcher and KiUserCallbackDispatcher in ntdll.dll.)
+
+# REQUIRES: target-x86_64, native, system-windows
+
+# RUN: %build %p/Inputs/windows-unaligned-x86_64.cpp 
%p/Inputs/windows-unaligned-x86_64-asm.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+# Future TODO: If %build could compile the source file in C mode, the symbol
+# name handling would be easier across msvc and mingw build configurations.
+# (In mingw mode, the extern C symbol "func" is printed as "::func" while
+# it's plain "func" in msvc mode. Without the extern C, it's "func(..." in
+# mingw mode, but "void __cdecl func(..." in msvc mode.)
+
+breakpoint set -n func
+# CHECK: Breakpoint 1: where = {{.*}}`{{(::)?}}func
+
+process launch
+# CHECK: stop reason = breakpoint 1.1
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`{{(::)?}}func
+# CHECK: frame #1: {{.*}}`realign_stack
+# CHECK: frame #2: {{.*}}`call_func
+# CHECK: frame #3: {{.*}}`main
Index: lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
@@ -0,0 +1,8 @@
+extern "C" void call_func(void (*ptr)(int a), int a);
+
+extern "C" void func(int arg) { }
+
+int main(int argc, char **argv) {
+  call_func(func, 42);
+  return 0;
+}
Index: lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
@@ -0,0 +1,25 @@
+.globlcall_func
+.def  call_func;   .scl2;  .type   32; .endef
+.seh_proc call_func
+call_func:
+subq$32, %rsp
+.seh_stackalloc 32
+.seh_endprologue
+callrealign_stack
+addq$32, %rsp
+ret
+.seh_endproc
+
+.globlrealign_stack
+.def  realign_stack;   .scl2;  .type   32; .endef
+.seh_proc realign_stack
+realign_stack:
+subq$32, %rsp
+.seh_stackalloc 32
+.seh_endprologue
+movq%rcx, %rax
+movl%edx, %ecx
+call*%rax
+addq$32, %rsp
+ret
+.seh_endproc
Index: lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
===
--- lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
+++ lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
@@ -40,10 +40,15 @@
 
   bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
 
-  // In Windows_x86_64 ABI, stack will always be maintained 16-byte aligned
+  // In Windows_x86_64 ABI requires that the stack will be maintained 16-byte
+  // aligned.
+  // When ntdll invokes callbacks such as KiUserExceptionDispatcher or
+  // KiUserCallbackDispatcher, those functions won't have a properly 16-byte
+  // aligned stack - but tolerate unwinding through them by relaxing the
+  // requirement to 8 bytes.
   bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
- if (cfa & (16ull - 1ull))
-  return false; // Not 16 byte aligned
+if (cfa & (8ull - 1ull))
+  return false; // Not 8 byte aligned
 if (cfa == 0)
   return false; // Zero is not a valid stack addr

[Lldb-commits] [PATCH] D129455: [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D129455#3641967 , @labath wrote:

> You say that the issue is the lack of symtab in the "msvc" mode. What makes 
> this test work then?

When invoked via the `%build` python script (lldb/test/Shell/helper/build.py), 
clang is invoked with extra options (`/Z7`) that generate codeview debug info, 
and then later the linker is invoked with extra options (`/DEBUG:FULL` and 
`'/PDB:' + self._pdb_file_name())`) to write that codeview debug info into a 
separate standalone PDB file.

In the MSVC ecosystem, executables/DLLs never have embedded debug info - it's 
always in a separate PDB file. Contrary to the GCC-compatible ecosystem, debug 
info is opt-in here, not opt-out (e.g. if linking with a unix linker, you get 
the symbol table and debug info included automatically unless you pass `-Wl,-s` 
or strip things separately afterwards). In the MSVC ecosystem, the only way to 
have a symbol table (without actual dwarf debug info) is with a PDB file.

In the mingw ecosystem, things behave as on unix - you get any embedded DWARF 
debug info included in the executable by default, and a symbol table - both 
which can be stripped out afterwards.

> Do some of the assembler directives (`.def`?) produce some sort of debug info 
> entries?

Those just set the symbol type to "function" - it could be omitted from this 
test for clarity.

> What determines the modes that clang is operating in? Is it the default 
> target triple?

Exactly.

Additionally, there's a couple more tooling differences that makes things a bit 
more complicated:

In the MSVC ecosystem, you normally would execute cl.exe (the MSVC compiler) or 
clang-cl (the cl.exe compatible clang frontend) - which has got an option 
syntax that is distinct and mostly incompatible from the gcc-compatible normal 
`clang` interface.

Despite this, you can also build code in MSVC mode with the regular 
gcc-compatible clang interface (either by passing e.g. 
`--target=x86_64-windows-msvc` or if such a triple is the deafult target 
triple). You can do most things in this setup too, but some MSVC-isms are 
tricky to achieve. This is what the `%clang_host` lit test macro does. Regular 
compiling, e.g. `%clang_host input.c -o executable -luser32` works fine for 
both MSVC and mingw mode.

In the MSVC ecosystem, you very rarely use the compiler driver to run linking - 
you'd normally execute `link.exe` or `lld-link` directly. If linking via the 
gcc-compatible clang interface, options passed to the linked with `-Wl,` need 
to be link.exe/lld-link options, which a unix-style linker obviously doesn't 
recognize.

> What does `-Wl,-debug:symtab` actually produce?

It produces an executable embedded symbol table, like in mingw mode. This is a 
lld specific option, MS's upstream link.exe doesn't support this option (it 
supports other parameters to `-debug:` but `symtab` is lld's invention). LLDB 
happily picks up and uses that (for mingw built executables it's the default 
case anyway).

> Would it make sense to make it a part of the `%clang_host` substitution (only 
> for the msvc mode) to make the output of %clang_host more uniform? Or maybe 
> achieve equivalence in some other way (add `-g1`, or similar, to the cmd 
> line)?

I think that might be a sensible path forward in general! But as noted, I'd 
rather not hold up this trivial patch with that. (Also I'm a bit more short on 
time at the moment than usual, but I'm trying to get low hanging fruit merged 
before the llvm 15 branch deadline.)

> as I think it would be very useful if we could figure out a way to make this 
> test (and others) cross-platform, so we don't have to have two versions of 
> all of them.

A bunch of tests might be doable crossplatform that way, but any tests that 
involve assembly might be tricky. On i386, arm and aarch64, Windows has got 
mostly the same calling conventions as other OSes, but on x86_64, it's 
radically different - arguments are passed in different registers than on 
x86_64 unix. Additionally, the caller is required to allocate shadow space on 
the stack for all arguments passed in registers (the extra `subq $32, %rsp`) so 
that the callee can dump them there if wanted. And finally, this particular 
testcase generates SEH unwind instructions with the `.seh_*` directives. (I 
didn't test whether the testcase would work without the SEH unwind instructions 
or not - I made it to look mostly like what a regular Windows function would 
look like.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129455

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

[Lldb-commits] [PATCH] D129455: [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D129455#3642015 , @mstorsjo wrote:

> In D129455#3641967 , @labath wrote:
>
>> What does `-Wl,-debug:symtab` actually produce? Would it make sense to make 
>> it a part of the `%clang_host` substitution (only for the msvc mode) to make 
>> the output of %clang_host more uniform? Or maybe achieve equivalence in some 
>> other way (add `-g1`, or similar, to the cmd line)?
>
> I think that might be a sensible path forward in general!

If we'd want to take that even further, we could consider to have `%clang_host` 
add options for generating DWARF debug info even in MSVC mode, and pass 
`-Wl,-debug:dwarf` to include it embedded in the exe like in mingw mode. (I've 
understood that some people even do this in proper production setups.) It's not 
what you'd normally have in MSVC mode, but could be useful for making testcases 
more portable, for cases where such details don't matter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129455

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

[Lldb-commits] [PATCH] D129455: [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG66cdd6548ac5: [lldb] Reduce the stack alignment requirements 
for the Windows x86_64 ABI (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129455

Files:
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
  lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
  lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
  lldb/test/Shell/Unwind/windows-unaligned-x86_64.test


Index: lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
===
--- /dev/null
+++ lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
@@ -0,0 +1,26 @@
+# Test unwinding through stack frames that aren't aligned to 16 bytes.
+# (In real world code, this happens when unwinding through
+# KiUserExceptionDispatcher and KiUserCallbackDispatcher in ntdll.dll.)
+
+# REQUIRES: target-x86_64, native, system-windows
+
+# RUN: %build %p/Inputs/windows-unaligned-x86_64.cpp 
%p/Inputs/windows-unaligned-x86_64-asm.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+# Future TODO: If %build could compile the source file in C mode, the symbol
+# name handling would be easier across msvc and mingw build configurations.
+# (In mingw mode, the extern C symbol "func" is printed as "::func" while
+# it's plain "func" in msvc mode. Without the extern C, it's "func(..." in
+# mingw mode, but "void __cdecl func(..." in msvc mode.)
+
+breakpoint set -n func
+# CHECK: Breakpoint 1: where = {{.*}}`{{(::)?}}func
+
+process launch
+# CHECK: stop reason = breakpoint 1.1
+
+thread backtrace
+# CHECK: frame #0: {{.*}}`{{(::)?}}func
+# CHECK: frame #1: {{.*}}`realign_stack
+# CHECK: frame #2: {{.*}}`call_func
+# CHECK: frame #3: {{.*}}`main
Index: lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64.cpp
@@ -0,0 +1,8 @@
+extern "C" void call_func(void (*ptr)(int a), int a);
+
+extern "C" void func(int arg) { }
+
+int main(int argc, char **argv) {
+  call_func(func, 42);
+  return 0;
+}
Index: lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
===
--- /dev/null
+++ lldb/test/Shell/Unwind/Inputs/windows-unaligned-x86_64-asm.s
@@ -0,0 +1,25 @@
+.globlcall_func
+.def  call_func;   .scl2;  .type   32; .endef
+.seh_proc call_func
+call_func:
+subq$32, %rsp
+.seh_stackalloc 32
+.seh_endprologue
+callrealign_stack
+addq$32, %rsp
+ret
+.seh_endproc
+
+.globlrealign_stack
+.def  realign_stack;   .scl2;  .type   32; .endef
+.seh_proc realign_stack
+realign_stack:
+subq$32, %rsp
+.seh_stackalloc 32
+.seh_endprologue
+movq%rcx, %rax
+movl%edx, %ecx
+call*%rax
+addq$32, %rsp
+ret
+.seh_endproc
Index: lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
===
--- lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
+++ lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
@@ -40,10 +40,15 @@
 
   bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
 
-  // In Windows_x86_64 ABI, stack will always be maintained 16-byte aligned
+  // In Windows_x86_64 ABI requires that the stack will be maintained 16-byte
+  // aligned.
+  // When ntdll invokes callbacks such as KiUserExceptionDispatcher or
+  // KiUserCallbackDispatcher, those functions won't have a properly 16-byte
+  // aligned stack - but tolerate unwinding through them by relaxing the
+  // requirement to 8 bytes.
   bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
- if (cfa & (16ull - 1ull))
-  return false; // Not 16 byte aligned
+if (cfa & (8ull - 1ull))
+  return false; // Not 8 byte aligned
 if (cfa == 0)
   return false; // Zero is not a valid stack address
 return true;


Index: lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
===
--- /dev/null
+++ lldb/test/Shell/Unwind/windows-unaligned-x86_64.test
@@ -0,0 +1,26 @@
+# Test unwinding through stack frames that aren't aligned to 16 bytes.
+# (In real world code, this happens when unwinding through
+# KiUserExceptionDispatcher and KiUserCallbackDispatcher in ntdll.dll.)
+
+# REQUIRES: target-x86_64, native, system-windows
+
+# RUN: %build %p/Inputs/windows-unaligned-x86_64.cpp %p/Inputs/windows-unaligned-x86_64-asm.s -o %t
+# RUN: %lldb %t -s %s -o exit | FileCheck %s
+
+# Future TODO: If %build could compile the source file in C mode, the symbol
+# name handling would

[Lldb-commits] [PATCH] D130309: [NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

I ran into this build break too - please try to fix or revert!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130309

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

[Lldb-commits] [PATCH] D130309: [NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D130309#3672934 , @clayborg wrote:

> Submitted a fix for the buildbots:
>
> commit f959d815f4637890ebbacca379f1c38ab47e4e14 
>  (HEAD 
> -> main)
> Author: Greg Clayton 
> Date:   Fri Jul 22 13:24:04 2022 -0700
>
>   Fix buildbot breakage after https://reviews.llvm.org/D130309.

Still broken for me:

  ../tools/lldb/source/Host/linux/HostInfoLinux.cpp: In static member function 
‘static bool 
lldb_private::HostInfoLinux::ComputeSupportExeDirectory(lldb_private::FileSpec&)’:
  ../tools/lldb/source/Host/linux/HostInfoLinux.cpp:174:64: error: passing 
‘const lldb_private::ConstString’ as ‘this’ argument discards qualifiers 
[-fpermissive]
174 |   file_spec.GetDirectory() = GetProgramFileSpec().GetDirectory();
|^
  In file included from ../tools/lldb/include/lldb/Utility/ArchSpec.h:13,
   from ../tools/lldb/include/lldb/Host/HostInfoBase.h:12,
   from 
../tools/lldb/include/lldb/Host/posix/HostInfoPosix.h:12,
   from 
../tools/lldb/include/lldb/Host/linux/HostInfoLinux.h:12,
   from ../tools/lldb/source/Host/linux/HostInfoLinux.cpp:9:
  ../tools/lldb/include/lldb/Utility/ConstString.h:40:7: note:   in call to 
‘constexpr lldb_private::ConstString& 
lldb_private::ConstString::operator=(const lldb_private::ConstString&)’
 40 | class ConstString {
|   ^~~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130309

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

[Lldb-commits] [PATCH] D130309: [NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D130309#3673045 , @clayborg wrote:

> Another fix for linux:
>
> commit 0bbce7a4c2d2bff622bdadd4323f93f5d90e6d24 
>  (HEAD 
> -> main, origin/main, origin/HEAD)
> Author: Greg Clayton 
> Date:   Fri Jul 22 13:59:06 2022 -0700
>
>   Fix buildbot breakage after https://reviews.llvm.org/D130309.

Thanks, now it finally built correctly for me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130309

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

[Lldb-commits] [PATCH] D129377: [lldb/Fuzzer] Add fuzzer for expression evaluator

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

This broke building of Clang, when it's set up by symlinking 
`llvm-project/clang` into `llvm-project/llvm/tools`. In that case, cmake 
configure errors out like this:

  -- Configuring done 
  CMake Error in tools/clang/tools/clang-fuzzer/handle-cxx/CMakeLists.txt:
Target "clangHandleCXX" INTERFACE_INCLUDE_DIRECTORIES property contains
path:
   
  
"/home/martin/code/llvm-project/llvm/tools/clang/tools/clang-fuzzer/handle-cxx/."
 
  
which is prefixed in the source directory.

See e.g. 
https://stackoverflow.com/questions/25676277/cmake-target-include-directories-prints-an-error-when-i-try-to-add-the-source
 for some discussion on the issue.

Can we revert this commit until this has been sorted out?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129377

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

[Lldb-commits] [PATCH] D129377: [lldb/Fuzzer] Add fuzzer for expression evaluator

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D129377#3673237 , @mib wrote:

> In D129377#3673204 , @mstorsjo 
> wrote:
>
>> This broke building of Clang, when it's set up by symlinking 
>> `llvm-project/clang` into `llvm-project/llvm/tools`. In that case, cmake 
>> configure errors out like this:
>>
>>   -- Configuring done 
>>   CMake Error in tools/clang/tools/clang-fuzzer/handle-cxx/CMakeLists.txt:
>> Target "clangHandleCXX" INTERFACE_INCLUDE_DIRECTORIES property contains
>> path:
>>
>>   
>> "/home/martin/code/llvm-project/llvm/tools/clang/tools/clang-fuzzer/handle-cxx/."
>>  
>>   
>> which is prefixed in the source directory.
>>
>> See e.g. 
>> https://stackoverflow.com/questions/25676277/cmake-target-include-directories-prints-an-error-when-i-try-to-add-the-source
>>  for some discussion on the issue.
>>
>> Can we revert this commit until this has been sorted out?
>
> @mstorsjo I reverted Chelsea's patch in b797834748f1 
> , since 
> she's offline now. Do you have a link to a bot failure that would help us 
> investigate the issue ?
>
> Thanks!

Thanks!

It’s not on a public buildbot but only in my local continuous builds 
unfortunately - but I’ll have look at the issue myself today - I was running 
out of time yesterday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129377

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

[Lldb-commits] [PATCH] D129377: [lldb/Fuzzer] Add fuzzer for expression evaluator

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D129377#3673237 , @mib wrote:

> In D129377#3673204 , @mstorsjo 
> wrote:
>
>> This broke building of Clang, when it's set up by symlinking 
>> `llvm-project/clang` into `llvm-project/llvm/tools`. In that case, cmake 
>> configure errors out like this:
>>
>>   -- Configuring done 
>>   CMake Error in tools/clang/tools/clang-fuzzer/handle-cxx/CMakeLists.txt:
>> Target "clangHandleCXX" INTERFACE_INCLUDE_DIRECTORIES property contains
>> path:
>>
>>   
>> "/home/martin/code/llvm-project/llvm/tools/clang/tools/clang-fuzzer/handle-cxx/."
>>  
>>   
>> which is prefixed in the source directory.
>>
>> See e.g. 
>> https://stackoverflow.com/questions/25676277/cmake-target-include-directories-prints-an-error-when-i-try-to-add-the-source
>>  for some discussion on the issue.
>>
>> Can we revert this commit until this has been sorted out?
>
> @mstorsjo I reverted Chelsea's patch in b797834748f1 
> , since 
> she's offline now. Do you have a link to a bot failure that would help us 
> investigate the issue ?

So in short, I think this would work if you'd simply remove the newly added 
`target_include_directories(clangHandleCXX PUBLIC .)` in 
`clang/tools/clang-fuzzer/handle-cxx/CMakeLists.txt` and 
`target_include_directories(clangProtoToCXX PUBLIC .)` and 
`target_include_directories(clangLoopProtoToCXX PUBLIC .)` in 
`clang/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt`.

To reproduce the issue - instead of configuring llvm+clang+lldb by passing 
`-DLLVM_ENABLE_PROJECTS="clang;lldb"` make symlinks in 
`llvm-project/llvm/tools` pointing to `llvm-project/clang` and 
`llvm-project/lldb`. (I.e., `cd llvm-project/llvm/tools; ln -s ../../clang .; 
ln -s ../../lldb .`) This slightly affects the effective path at where CMake 
sees the clang files, which causes CMake to report the unexpected path overlap 
issue here.

I've tried to build all these fuzzers - I haven't really managed to build it 
all, but I think I'm fairly close. Anyway, I tried removing those 
`target_include_directories()` in that setup, and it didn't change anything, 
but maybe it only makes a difference if one gets further than what I got. 
(Changing `PUBLIC` into `PRIVATE` in those lines avoided the issue too.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129377

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

[Lldb-commits] [PATCH] D130942: [LLDB][DWARF] Set MSInheritanceAttr for record decl when it's using Microsoft compatible ABI.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

This looks sensible to me, although it might be good if someone else more 
familiar with this code has a look too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130942

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

[Lldb-commits] [PATCH] D130942: [LLDB][DWARF] Set MSInheritanceAttr for record decl when it's using Microsoft compatible ABI.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1754
+  m_ast.getASTContext(),
+  
clang::MSInheritanceAttr::Spelling::Keyword_unspecified_inheritance));
+}

rnk wrote:
> I'm concerned that this isn't really the right fix. Changing the inheritance 
> model changes the size of the member pointer representation, so the 
> consequences of getting the wrong one could affect expression evaluation 
> results. Zequan suggested guessing the inheritance model from the class 
> definition, but we really ought to write down the inheritance model in the 
> DWARF when using the MS ABI. This probably needs its own DWARF attribute.
FWIW, there’s two use cases at play here:

1. The executable actually is using the itanium ABI, but is being 
(mis)interpreted with the MSVC ABI. When this happens, we currently crash, 
which is a terrible user experience. In this case, there’s probably no good 
solution (we can’t really get it right at this point) - but pretty much 
anything is better than crashing. Before D127048, we always interpreted 
everything with the MSVC C++ ABI; now we probably are getting it right in a 
majority of cases at least.

2. People intentionally using dwarf debug into with MSVC ABI. Not very common, 
but something that some people do use, as far as I know. Here we at least have 
a chance of getting right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130942

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

[Lldb-commits] [PATCH] D130942: [LLDB][DWARF] Set MSInheritanceAttr for record decl when it's using Microsoft compatible ABI.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1754
+  m_ast.getASTContext(),
+  
clang::MSInheritanceAttr::Spelling::Keyword_unspecified_inheritance));
+}

rnk wrote:
> mstorsjo wrote:
> > rnk wrote:
> > > I'm concerned that this isn't really the right fix. Changing the 
> > > inheritance model changes the size of the member pointer representation, 
> > > so the consequences of getting the wrong one could affect expression 
> > > evaluation results. Zequan suggested guessing the inheritance model from 
> > > the class definition, but we really ought to write down the inheritance 
> > > model in the DWARF when using the MS ABI. This probably needs its own 
> > > DWARF attribute.
> > FWIW, there’s two use cases at play here:
> > 
> > 1. The executable actually is using the itanium ABI, but is being 
> > (mis)interpreted with the MSVC ABI. When this happens, we currently crash, 
> > which is a terrible user experience. In this case, there’s probably no good 
> > solution (we can’t really get it right at this point) - but pretty much 
> > anything is better than crashing. Before D127048, we always interpreted 
> > everything with the MSVC C++ ABI; now we probably are getting it right in a 
> > majority of cases at least.
> > 
> > 2. People intentionally using dwarf debug into with MSVC ABI. Not very 
> > common, but something that some people do use, as far as I know. Here we at 
> > least have a chance of getting right.
> I see, I thought we already knew we were in use case 2 not 1. Having this as 
> a workaround to not crash seems fine, but we really ought to warn when LLDB 
> encounters these conditions:
> 1. MS C++ ABI is in use
> 2. DWARF is in use
> 3. A C++ source file is in use
> 
> Using DWARF for C code in the MS ABI model is fine, no need to warn in that 
> case.
Some kind of warning would be great, yes. Is MS ABI C++ in dwarf essentially 
broken by definition (unless we’d extend our dwarf generation)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130942

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

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a subscriber: alvinhochun.
mstorsjo added a comment.

I don't really have experience with this API - is this pattern (creating and 
closing event objects for each poll run) the common way of using this API? Is 
there any potential performance risk compared with keeping event objects around 
(if that's possible?).

Can @alvinhochun take the patch for a spin? I think you've got more realistic 
usecases of lldb to try it out in. (Do you want me to make a build with the 
patch for you to try out?) @labath What's the best way to exercise this code in 
actual use of lldb?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131159

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

[Lldb-commits] [PATCH] D130689: [LLVM] Update C++ standard to 17

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D130689#3700094 , @thakis wrote:

> In D130689#3696186 , @thieta wrote:
>
>> @nikic @thakis I fixed this issue in https://reviews.llvm.org/D131063 and it 
>> can be built with CXX_STANDARD=17 and OSX_DEPLOYMENT_TARGET=10.11.
>
> Thanks! We took this opportunity to bump our clang deployment target from 
> 10.7 to 10.12 (which makes a few other minor things easier too), so we don't 
> need this change any more. But it's a good change anyways :)

FWIW I ran into an issue when doing Universal macOS builds (by building for 
arm64 and x86_64 at the same time) where I now had to raise my deployment 
target to 10.14 - see https://github.com/llvm/llvm-project/issues/57017 for 
more details about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D131159#3709965 , @labath wrote:

>> Can @alvinhochun take the patch for a spin? I think you've got more 
>> realistic usecases of lldb to try it out in. (Do you want me to make a build 
>> with the patch for you to try out?) @labath What's the best way to exercise 
>> this code in actual use of lldb?
>
> I would appreciate that. I have tried to build it and run the test suite, but 
> I didn't have a clean baseline, so I can't really say whether it works for 
> everything. The main user of this code is the gdb-remote communication code, 
> so debugging something via lldb-server (or running the lldb-server test 
> suite) should be a good indicator of whether it works.

I presume this is covered by running `ninja check-lldb`, or should I flip the 
switch to prefer lldb-server on Windows before doing that? (I guess I can do 
both.) I've got an environment where `check-lldb` mostly passes, I can check 
that it doesn't regress it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131159

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

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D131159#3711024 , @mstorsjo wrote:

> In D131159#3709965 , @labath wrote:
>
>>> Can @alvinhochun take the patch for a spin? I think you've got more 
>>> realistic usecases of lldb to try it out in. (Do you want me to make a 
>>> build with the patch for you to try out?) @labath What's the best way to 
>>> exercise this code in actual use of lldb?
>>
>> I would appreciate that. I have tried to build it and run the test suite, 
>> but I didn't have a clean baseline, so I can't really say whether it works 
>> for everything. The main user of this code is the gdb-remote communication 
>> code, so debugging something via lldb-server (or running the lldb-server 
>> test suite) should be a good indicator of whether it works.
>
> I presume this is covered by running `ninja check-lldb`, or should I flip the 
> switch to prefer lldb-server on Windows before doing that? (I guess I can do 
> both.) I've got an environment where `check-lldb` mostly passes, I can check 
> that it doesn't regress it.

I tested out this patch with running `ninja check-lldb`. Out of the box, this 
patch doesn't regress anything - the tests still pass. (In my environment, 
there's a bunch of occasional stray failures already before, but by rerunning 
the tests a couple times, I can get a clean run.)

If I edit `ShouldUseLLDBServer()` in 
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp to always return 
true (before applying this patch), most tests still pass, but I have 11 tests 
that hang so that I end up having to kill lldb-server. With this patch on top, 
I still get the same set of 11 hanging tests, so I think that means that this 
patch doesn't regress anything with respect to that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131159

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

[Lldb-commits] [PATCH] D130942: [LLDB][DWARF] Set MSInheritanceAttr for record decl when it's using Microsoft compatible ABI.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1754
+  m_ast.getASTContext(),
+  
clang::MSInheritanceAttr::Spelling::Keyword_unspecified_inheritance));
+}

mstorsjo wrote:
> rnk wrote:
> > mstorsjo wrote:
> > > rnk wrote:
> > > > I'm concerned that this isn't really the right fix. Changing the 
> > > > inheritance model changes the size of the member pointer 
> > > > representation, so the consequences of getting the wrong one could 
> > > > affect expression evaluation results. Zequan suggested guessing the 
> > > > inheritance model from the class definition, but we really ought to 
> > > > write down the inheritance model in the DWARF when using the MS ABI. 
> > > > This probably needs its own DWARF attribute.
> > > FWIW, there’s two use cases at play here:
> > > 
> > > 1. The executable actually is using the itanium ABI, but is being 
> > > (mis)interpreted with the MSVC ABI. When this happens, we currently 
> > > crash, which is a terrible user experience. In this case, there’s 
> > > probably no good solution (we can’t really get it right at this point) - 
> > > but pretty much anything is better than crashing. Before D127048, we 
> > > always interpreted everything with the MSVC C++ ABI; now we probably are 
> > > getting it right in a majority of cases at least.
> > > 
> > > 2. People intentionally using dwarf debug into with MSVC ABI. Not very 
> > > common, but something that some people do use, as far as I know. Here we 
> > > at least have a chance of getting right.
> > I see, I thought we already knew we were in use case 2 not 1. Having this 
> > as a workaround to not crash seems fine, but we really ought to warn when 
> > LLDB encounters these conditions:
> > 1. MS C++ ABI is in use
> > 2. DWARF is in use
> > 3. A C++ source file is in use
> > 
> > Using DWARF for C code in the MS ABI model is fine, no need to warn in that 
> > case.
> Some kind of warning would be great, yes. Is MS ABI C++ in dwarf essentially 
> broken by definition (unless we’d extend our dwarf generation)?
Ping @zequanwu - do you want to follow up on this one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130942

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

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

FYI, this patch added some new compilation warnings:

  [4055/7050] Building CXX object 
tools/lldb/source/Host/CMakeFiles/lldbHost.dir/windows/MainLoopWindows.cpp.obj
  llvm-project/lldb/source/Host/windows/MainLoopWindows.cpp:28:9: warning: 
unused variable 'result' [-Wunused-variable]
  int result = WSAEventSelect(fd, info.event, FD_READ | FD_ACCEPT | 
FD_CLOSE);
  ^
  llvm-project/lldb/source/Host/windows/MainLoopWindows.cpp:38:9: warning: 
unused variable 'result' [-Wunused-variable]
  int result = WSAEventSelect(fd.first, WSA_INVALID_EVENT, 0);
  ^
  llvm-project/lldb/source/Host/windows/MainLoopWindows.cpp:86:8: warning: 
unused variable 'result' [-Wunused-variable]
BOOL result = WSACloseEvent(it->second.event);
 ^
  3 warnings generated.

(They're only used in asserts.) I guess we should add void casts to mark the 
variables as used outside of asserts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131159

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

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D131159#3742370 , @labath wrote:

> In D131159#3742364 , @mgorny wrote:
>
>> In D131159#3742235 , @mstorsjo 
>> wrote:
>>
>>> (They're only used in asserts.) I guess we should add void casts to mark 
>>> the variables as used outside of asserts.
>>
>> Actually, there's a dedicated `UNUSED_IF_ASSERT_DISABLED` macro for that. 
>> Please use it instead.
>
> One of these days, I'm going to have to delete it. If void casts are good 
> enough for the rest of llvm, I don't see why should we be any different.

Ok, so is this discussion settled and I can go ahead and use void casts for 
these trivial fixups? (I usually just push such patches directly without 
passing via review.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131159

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

[Lldb-commits] [PATCH] D122389: [lldb] Fix interpreting absolute Windows paths with forward slashes

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

In practice, Windows paths can use either backslashes or forward slashes.

This fixes an issue reported downstream at
https://github.com/mstorsjo/llvm-mingw/issues/266.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122389

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Utility/FileSpec.cpp
  lldb/unittests/Utility/FileSpecTest.cpp


Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -196,9 +196,12 @@
   EXPECT_EQ(FileSpec::Style::posix, FileSpec::GuessPathStyle("//net/bar.txt"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(C:\foo.txt)"));
+  EXPECT_EQ(FileSpec::Style::windows,
+FileSpec::GuessPathStyle(R"(C:/foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(\\net\foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:\)"));
+  EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:/)"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo/bar.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("Z:"));
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -311,7 +311,8 @@
   if (absolute_path.startswith(R"(\\)"))
 return Style::windows;
   if (absolute_path.size() >= 3 && llvm::isAlpha(absolute_path[0]) &&
-  absolute_path.substr(1, 2) == R"(:\)")
+  (absolute_path.substr(1, 2) == R"(:\)" ||
+   absolute_path.substr(1, 2) == R"(:/)"))
 return Style::windows;
   return llvm::None;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -772,7 +772,8 @@
 
   // check whether we have a windows path, and so the first character is a
   // drive-letter not a hostname.
-  if (host.size() == 1 && llvm::isAlpha(host[0]) && path.startswith("\\"))
+  if (host.size() == 1 && llvm::isAlpha(host[0]) &&
+  (path.startswith("\\") || path.startswith("/")))
 return path_from_dwarf;
 
   return path;


Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -196,9 +196,12 @@
   EXPECT_EQ(FileSpec::Style::posix, FileSpec::GuessPathStyle("//net/bar.txt"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(C:\foo.txt)"));
+  EXPECT_EQ(FileSpec::Style::windows,
+FileSpec::GuessPathStyle(R"(C:/foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(\\net\foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:\)"));
+  EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:/)"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo/bar.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("Z:"));
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -311,7 +311,8 @@
   if (absolute_path.startswith(R"(\\)"))
 return Style::windows;
   if (absolute_path.size() >= 3 && llvm::isAlpha(absolute_path[0]) &&
-  absolute_path.substr(1, 2) == R"(:\)")
+  (absolute_path.substr(1, 2) == R"(:\)" ||
+   absolute_path.substr(1, 2) == R"(:/)"))
 return Style::windows;
   return llvm::None;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -772,7 +772,8 @@
 
   // check whether we have a windows path, and so the first character is a
   // drive-letter not a hostname.
-  if (host.size() == 1 && llvm::isAlpha(host[0]) && path.startswith("\\"))
+  if (host.size() == 1 && llvm::isAlpha(host[0]) &&
+  (path.startswith("\\") || path.startswith("/")))
 return path_from_dwarf;
 
   return path;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122486: [lldb] Fix building for mingw after changes to sigtstp_handler

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

Some signal handlers are set up within an !_MSC_VER condition,
i.e. omitted in MSVC builds but included in mingw builds. Previously
sigtstp_handler was defined in all builds, but since
4bcadd66864bf4af929ac8753a51d7ad408cdef0 
 / D120320 
 it's only
defined non platforms other than Windows.

This applies a matching ifdef around the usage of sigtstp_handler.

Alternatively the ifdef condition could be changed from !_MSC_VER
to !_WIN32 - I'm unsure whether the other ones that are hooked up
in mingw builds but not in MSVC builds actually ever make a difference
in mingw builds.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122486

Files:
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -832,7 +832,9 @@
 #if !defined(_MSC_VER)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGWINCH, sigwinch_handler);
+#if !defined(_WIN32)
   signal(SIGTSTP, sigtstp_handler);
+#endif
 #endif
 
   int exit_code = 0;


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -832,7 +832,9 @@
 #if !defined(_MSC_VER)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGWINCH, sigwinch_handler);
+#if !defined(_WIN32)
   signal(SIGTSTP, sigtstp_handler);
+#endif
 #endif
 
   int exit_code = 0;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122486: [lldb] Fix building for mingw after changes to sigtstp_handler

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

> Alternatively the ifdef condition could be changed from !_MSC_VER to !_WIN32 
> - I'm unsure whether the other ones that are hooked up in mingw builds but 
> not in MSVC builds actually ever make a difference in mingw builds.

On second thought, maybe it'd be best to just go down this route, to reduce 
unnecessary differences between the MSVC and mingw builds. If the (more 
officially supported) MSVC build can do without these signals, the mingw build 
should too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122486

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

[Lldb-commits] [PATCH] D122486: [lldb] Fix building for mingw after changes to sigtstp_handler

[Martin Storsjö via Phabricator via lldb-commits]

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

Changed the existing ifdef around the signals that were omitted in the MSVC 
build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122486

Files:
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -829,7 +829,7 @@
   SBHostOS::ThreadCreated("");
 
   signal(SIGINT, sigint_handler);
-#if !defined(_MSC_VER)
+#if !defined(_WIN32)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGWINCH, sigwinch_handler);
   signal(SIGTSTP, sigtstp_handler);


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -829,7 +829,7 @@
   SBHostOS::ThreadCreated("");
 
   signal(SIGINT, sigint_handler);
-#if !defined(_MSC_VER)
+#if !defined(_WIN32)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGWINCH, sigwinch_handler);
   signal(SIGTSTP, sigtstp_handler);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122486: [lldb] Fix building for mingw after changes to sigtstp_handler

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbc13101cf945: [lldb] Fix building for mingw after changes to 
sigtstp_handler (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122486

Files:
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -829,7 +829,7 @@
   SBHostOS::ThreadCreated("");
 
   signal(SIGINT, sigint_handler);
-#if !defined(_MSC_VER)
+#if !defined(_WIN32)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGWINCH, sigwinch_handler);
   signal(SIGTSTP, sigtstp_handler);


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -829,7 +829,7 @@
   SBHostOS::ThreadCreated("");
 
   signal(SIGINT, sigint_handler);
-#if !defined(_MSC_VER)
+#if !defined(_WIN32)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGWINCH, sigwinch_handler);
   signal(SIGTSTP, sigtstp_handler);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122389: [lldb] Fix interpreting absolute Windows paths with forward slashes

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb548f5847235: [lldb] Fix interpreting absolute Windows paths 
with forward slashes (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122389

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Utility/FileSpec.cpp
  lldb/unittests/Utility/FileSpecTest.cpp


Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -196,9 +196,12 @@
   EXPECT_EQ(FileSpec::Style::posix, FileSpec::GuessPathStyle("//net/bar.txt"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(C:\foo.txt)"));
+  EXPECT_EQ(FileSpec::Style::windows,
+FileSpec::GuessPathStyle(R"(C:/foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(\\net\foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:\)"));
+  EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:/)"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo/bar.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("Z:"));
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -311,7 +311,8 @@
   if (absolute_path.startswith(R"(\\)"))
 return Style::windows;
   if (absolute_path.size() >= 3 && llvm::isAlpha(absolute_path[0]) &&
-  absolute_path.substr(1, 2) == R"(:\)")
+  (absolute_path.substr(1, 2) == R"(:\)" ||
+   absolute_path.substr(1, 2) == R"(:/)"))
 return Style::windows;
   return llvm::None;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -772,7 +772,8 @@
 
   // check whether we have a windows path, and so the first character is a
   // drive-letter not a hostname.
-  if (host.size() == 1 && llvm::isAlpha(host[0]) && path.startswith("\\"))
+  if (host.size() == 1 && llvm::isAlpha(host[0]) &&
+  (path.startswith("\\") || path.startswith("/")))
 return path_from_dwarf;
 
   return path;


Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -196,9 +196,12 @@
   EXPECT_EQ(FileSpec::Style::posix, FileSpec::GuessPathStyle("//net/bar.txt"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(C:\foo.txt)"));
+  EXPECT_EQ(FileSpec::Style::windows,
+FileSpec::GuessPathStyle(R"(C:/foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows,
 FileSpec::GuessPathStyle(R"(\\net\foo.txt)"));
   EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:\)"));
+  EXPECT_EQ(FileSpec::Style::windows, FileSpec::GuessPathStyle(R"(Z:/)"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("foo/bar.txt"));
   EXPECT_EQ(llvm::None, FileSpec::GuessPathStyle("Z:"));
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -311,7 +311,8 @@
   if (absolute_path.startswith(R"(\\)"))
 return Style::windows;
   if (absolute_path.size() >= 3 && llvm::isAlpha(absolute_path[0]) &&
-  absolute_path.substr(1, 2) == R"(:\)")
+  (absolute_path.substr(1, 2) == R"(:\)" ||
+   absolute_path.substr(1, 2) == R"(:/)"))
 return Style::windows;
   return llvm::None;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -772,7 +772,8 @@
 
   // check whether we have a windows path, and so the first character is a
   // drive-letter not a hostname.
-  if (host.size() == 1 && llvm::isAlpha(host[0]) && path.startswith("\\"))
+  if (host.size() == 1 && llvm::isAlpha(host[0]) &&
+  (path.startswith("\\") || path.startswith("/")))
 return path_from_dwarf;
 
   return path;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122710: [lldb-vscode] Avoid a -Wunused-but-set-variable warning. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, clayborg.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122710

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


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -3246,7 +3246,6 @@
 g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, 
false);
   }
 
-  uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
 llvm::json::Object object;
 lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
@@ -3257,7 +3256,6 @@
 
 if (!g_vsc.HandleObject(object))
   return 1;
-++packet_idx;
   }
 
   return EXIT_SUCCESS;


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -3246,7 +3246,6 @@
 g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
   }
 
-  uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
 llvm::json::Object object;
 lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
@@ -3257,7 +3256,6 @@
 
 if (!g_vsc.HandleObject(object))
   return 1;
-++packet_idx;
   }
 
   return EXIT_SUCCESS;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122710: [lldb-vscode] Avoid a -Wunused-but-set-variable warning. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa37cb5ece513: [lldb-vscode] Avoid a 
-Wunused-but-set-variable warning. NFC. (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122710

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


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -3246,7 +3246,6 @@
 g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, 
false);
   }
 
-  uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
 llvm::json::Object object;
 lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
@@ -3257,7 +3256,6 @@
 
 if (!g_vsc.HandleObject(object))
   return 1;
-++packet_idx;
   }
 
   return EXIT_SUCCESS;


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -3246,7 +3246,6 @@
 g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
   }
 
-  uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
 llvm::json::Object object;
 lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
@@ -3257,7 +3256,6 @@
 
 if (!g_vsc.HandleObject(object))
   return 1;
-++packet_idx;
   }
 
   return EXIT_SUCCESS;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123025: [lldb-vscode] Implement stderr/stdout on win32 and redirect lldb log to VSCode

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a reviewer: labath.
mstorsjo added inline comments.



Comment at: lldb/tools/lldb-vscode/OutputRedirector.cpp:12
+#else
+#include 
+#include 

Minor style issue - I guess it'd be less of double negation, if we'd change the 
ifdef to `#if defined(_WIN32) .. #else`



Comment at: lldb/tools/lldb-vscode/OutputRedirector.cpp:55
   }
-  callback(StringRef(buffer, bytes_count).str());
+  callback(StringRef(buffer, bytes_count));
 }

This change looks unrelated (although I'm not familiar with this piece of 
code), although it's probably correct


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123025

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

[Lldb-commits] [PATCH] D123202: [lldb] Fix detecting warning options for GCC

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a subscriber: mgorny.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

If testing for a warning option like -Wno- with GCC, GCC won't
print any diagnostic at all, leading to the options being accepted
incorrectly. However later, if compiling a file that actually prints
another warning, GCC will also print warnings about these -Wno-
options being unrecognized.

This avoids warning spam like this, for every lldb source file that
produces build warnings with GCC:

  At global scope:
  cc1plus: warning: unrecognized command line option ‘-Wno-vla-extension’
  cc1plus: warning: unrecognized command line option ‘-Wno-deprecated-register’

This matches how such warning options are detected and added in
llvm/cmake/modules/HandleLLVMOptions.cmake, e.g. like this:

  check_cxx_compiler_flag("-Wclass-memaccess" CXX_SUPPORTS_CLASS_MEMACCESS_FLAG)
  append_if(CXX_SUPPORTS_CLASS_MEMACCESS_FLAG "-Wno-class-memaccess" 
CMAKE_CXX_FLAGS)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123202

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -168,22 +168,27 @@
 endif ()
 include_directories("${CMAKE_CURRENT_BINARY_DIR}/../clang/include")
 
+# GCC silently accepts any -Wno- option, but warns about those options
+# being unrecognized only if the compilation triggers other warnings to be
+# printed. Therefore, check for whether the compiler supports options in the
+# form -W, and if supported, add the corresponding -Wno- option.
+
 # Disable GCC warnings
-check_cxx_compiler_flag("-Wno-deprecated-declarations" 
CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS 
"-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-declarations" 
CXX_SUPPORTS_DEPRECATED_DECLARATIONS)
+append_if(CXX_SUPPORTS_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" 
CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-unknown-pragmas" CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS)
-append_if(CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wunknown-pragmas" CXX_SUPPORTS_UNKNOWN_PRAGMAS)
+append_if(CXX_SUPPORTS_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-strict-aliasing" CXX_SUPPORTS_NO_STRICT_ALIASING)
-append_if(CXX_SUPPORTS_NO_STRICT_ALIASING "-Wno-strict-aliasing" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
+append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
 # Disable Clang warnings
-check_cxx_compiler_flag("-Wno-deprecated-register" 
CXX_SUPPORTS_NO_DEPRECATED_REGISTER)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-register" 
CXX_SUPPORTS_DEPRECATED_REGISTER)
+append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-vla-extension" CXX_SUPPORTS_NO_VLA_EXTENSION)
-append_if(CXX_SUPPORTS_NO_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wvla-extension" CXX_SUPPORTS_VLA_EXTENSION)
+append_if(CXX_SUPPORTS_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
 
 # Disable MSVC warnings
 if( MSVC )


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -168,22 +168,27 @@
 endif ()
 include_directories("${CMAKE_CURRENT_BINARY_DIR}/../clang/include")
 
+# GCC silently accepts any -Wno- option, but warns about those options
+# being unrecognized only if the compilation triggers other warnings to be
+# printed. Therefore, check for whether the compiler supports options in the
+# form -W, and if supported, add the corresponding -Wno- option.
+
 # Disable GCC warnings
-check_cxx_compiler_flag("-Wno-deprecated-declarations" CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-declarations" CXX_SUPPORTS_DEPRECATED_DECLARATIONS)
+append_if(CXX_SUPPORTS_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-unknown-pragmas" CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS)
-append_if(CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wunknown-pragmas" CXX_SUPPORTS_UNKNOWN_PRAGMAS)
+append_if(CXX_SUPPORTS_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-strict-aliasing" CXX_SUPPORTS_NO_STRICT_ALIASING)
-append_if(CXX_SUPPORTS_NO_STRICT_ALIA

[Lldb-commits] [PATCH] D123203: [lldb] Silence GCC warnings about missing returns after fully covered switches. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

This silences warnings like this:

lldb/source/Core/DebuggerEvents.cpp: In member function ‘llvm::StringRef 
lldb_private::DiagnosticEventData::GetPrefix() const’:
lldb/source/Core/DebuggerEvents.cpp:55:1: warning: control reaches end of 
non-void function [-Wreturn-type]

  55 | }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123203

Files:
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Core/DebuggerEvents.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp


Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -102,6 +102,7 @@
 case BitOffset:
   return 10;
 }
+llvm_unreachable("Fully covered switch above!");
   };
 
   auto createError = [&](const char *expected_value_message) {
Index: lldb/source/Core/DebuggerEvents.cpp
===
--- lldb/source/Core/DebuggerEvents.cpp
+++ lldb/source/Core/DebuggerEvents.cpp
@@ -52,6 +52,7 @@
   case Type::Error:
 return "error";
   }
+  llvm_unreachable("Fully covered switch above!");
 }
 
 void DiagnosticEventData::Dump(Stream *s) const {
Index: lldb/source/Breakpoint/Breakpoint.cpp
===
--- lldb/source/Breakpoint/Breakpoint.cpp
+++ lldb/source/Breakpoint/Breakpoint.cpp
@@ -1026,6 +1026,7 @@
 case eBreakpointEventTypeThreadChanged: return "thread changed";
 case eBreakpointEventTypeAutoContinueChanged: return "autocontinue 
changed";
   };
+  llvm_unreachable("Fully covered switch above!");
 }
 
 Log *Breakpoint::BreakpointEventData::GetLogChannel() {


Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -102,6 +102,7 @@
 case BitOffset:
   return 10;
 }
+llvm_unreachable("Fully covered switch above!");
   };
 
   auto createError = [&](const char *expected_value_message) {
Index: lldb/source/Core/DebuggerEvents.cpp
===
--- lldb/source/Core/DebuggerEvents.cpp
+++ lldb/source/Core/DebuggerEvents.cpp
@@ -52,6 +52,7 @@
   case Type::Error:
 return "error";
   }
+  llvm_unreachable("Fully covered switch above!");
 }
 
 void DiagnosticEventData::Dump(Stream *s) const {
Index: lldb/source/Breakpoint/Breakpoint.cpp
===
--- lldb/source/Breakpoint/Breakpoint.cpp
+++ lldb/source/Breakpoint/Breakpoint.cpp
@@ -1026,6 +1026,7 @@
 case eBreakpointEventTypeThreadChanged: return "thread changed";
 case eBreakpointEventTypeAutoContinueChanged: return "autocontinue changed";
   };
+  llvm_unreachable("Fully covered switch above!");
 }
 
 Log *Breakpoint::BreakpointEventData::GetLogChannel() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123204: [lldb] Fix initialization of LazyBool/bool variables m_overwrite/m_overwrite_lazy. NFCI.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: jingham, labath.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

This silences a GCC warning after
1f7b58f2a50461493f083b2ed807b25e036286f6 
 / D122680 
:

lldb/source/Commands/CommandObjectCommands.cpp:1650:22: warning: enum constant 
in boolean context [-Wint-in-bool-context]
 1650 |   bool m_overwrite = eLazyBoolCalculate;

  |  ^~


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123204

Files:
  lldb/source/Commands/CommandObjectCommands.cpp


Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -1480,7 +1480,7 @@
 std::string m_class_name;
 std::string m_funct_name;
 std::string m_short_help;
-LazyBool m_overwrite_lazy;
+LazyBool m_overwrite_lazy = eLazyBoolCalculate;
 ScriptedCommandSynchronicity m_synchronicity =
 eScriptedCommandSynchronicitySynchronous;
   };
@@ -1647,7 +1647,7 @@
   std::string m_cmd_name;
   CommandObjectMultiword *m_container = nullptr;
   std::string m_short_help;
-  bool m_overwrite = eLazyBoolCalculate;
+  bool m_overwrite = false;
   ScriptedCommandSynchronicity m_synchronicity =
   eScriptedCommandSynchronicitySynchronous;
 };


Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -1480,7 +1480,7 @@
 std::string m_class_name;
 std::string m_funct_name;
 std::string m_short_help;
-LazyBool m_overwrite_lazy;
+LazyBool m_overwrite_lazy = eLazyBoolCalculate;
 ScriptedCommandSynchronicity m_synchronicity =
 eScriptedCommandSynchronicitySynchronous;
   };
@@ -1647,7 +1647,7 @@
   std::string m_cmd_name;
   CommandObjectMultiword *m_container = nullptr;
   std::string m_short_help;
-  bool m_overwrite = eLazyBoolCalculate;
+  bool m_overwrite = false;
   ScriptedCommandSynchronicity m_synchronicity =
   eScriptedCommandSynchronicitySynchronous;
 };
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123205: [lldb] Silence GCC/glibc warnings about ignoring the return value of write(). NFC.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

This matches how another similar warning is silenced in
Host/posix/ProcessLauncherPosixFork.cpp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123205

Files:
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -893,7 +893,8 @@
 return result;
   }
 
-  write(fds[WRITE], data, size);
+  int r = write(fds[WRITE], data, size);
+  (void)r;
   // Close the write end of the pipe, so that the command interpreter will exit
   // when it consumes all the data.
   llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -893,7 +893,8 @@
 return result;
   }
 
-  write(fds[WRITE], data, size);
+  int r = write(fds[WRITE], data, size);
+  (void)r;
   // Close the write end of the pipe, so that the command interpreter will exit
   // when it consumes all the data.
   llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123206: [lldb] Silence warnings about unused static variables in RegisterInfos_arm64.h

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

Move them to the only source file that included RegisterInfos_arm64.h
that actually used these variables.

This silences warnings like these:

  In file included from 
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp:42:
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h:790:35: warning: 
‘g_register_infos_mte’ defined but not used [-Wunused-variable]
790 | static lldb_private::RegisterInfo g_register_infos_mte[] = {
|   ^~~~
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h:787:35: warning: 
‘g_register_infos_pauth’ defined but not used [-Wunused-variable]
787 | static lldb_private::RegisterInfo g_register_infos_pauth[] = {
|   ^~


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123206

Files:
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h


Index: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -784,10 +784,5 @@
 {DEFINE_DBG(wcr, 15)}
 };
 // clang-format on
-static lldb_private::RegisterInfo g_register_infos_pauth[] = {
-DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
-
-static lldb_private::RegisterInfo g_register_infos_mte[] = {
-DEFINE_EXTENSION_REG(mte_ctrl)};
 
 #endif // DECLARE_REGISTER_INFOS_ARM64_STRUCT
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -72,6 +72,12 @@
 #include "RegisterInfos_arm64_sve.h"
 #undef DECLARE_REGISTER_INFOS_ARM64_STRUCT
 
+static lldb_private::RegisterInfo g_register_infos_pauth[] = {
+DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
+
+static lldb_private::RegisterInfo g_register_infos_mte[] = {
+DEFINE_EXTENSION_REG(mte_ctrl)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,


Index: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -784,10 +784,5 @@
 {DEFINE_DBG(wcr, 15)}
 };
 // clang-format on
-static lldb_private::RegisterInfo g_register_infos_pauth[] = {
-DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
-
-static lldb_private::RegisterInfo g_register_infos_mte[] = {
-DEFINE_EXTENSION_REG(mte_ctrl)};
 
 #endif // DECLARE_REGISTER_INFOS_ARM64_STRUCT
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -72,6 +72,12 @@
 #include "RegisterInfos_arm64_sve.h"
 #undef DECLARE_REGISTER_INFOS_ARM64_STRUCT
 
+static lldb_private::RegisterInfo g_register_infos_pauth[] = {
+DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
+
+static lldb_private::RegisterInfo g_register_infos_mte[] = {
+DEFINE_EXTENSION_REG(mte_ctrl)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123202: [lldb] Fix detecting warning options for GCC

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe546bbfda0ab: [lldb] Fix detecting warning options for GCC 
(authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123202

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -168,22 +168,27 @@
 endif ()
 include_directories("${CMAKE_CURRENT_BINARY_DIR}/../clang/include")
 
+# GCC silently accepts any -Wno- option, but warns about those options
+# being unrecognized only if the compilation triggers other warnings to be
+# printed. Therefore, check for whether the compiler supports options in the
+# form -W, and if supported, add the corresponding -Wno- option.
+
 # Disable GCC warnings
-check_cxx_compiler_flag("-Wno-deprecated-declarations" 
CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS 
"-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-declarations" 
CXX_SUPPORTS_DEPRECATED_DECLARATIONS)
+append_if(CXX_SUPPORTS_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" 
CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-unknown-pragmas" CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS)
-append_if(CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wunknown-pragmas" CXX_SUPPORTS_UNKNOWN_PRAGMAS)
+append_if(CXX_SUPPORTS_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-strict-aliasing" CXX_SUPPORTS_NO_STRICT_ALIASING)
-append_if(CXX_SUPPORTS_NO_STRICT_ALIASING "-Wno-strict-aliasing" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
+append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
 # Disable Clang warnings
-check_cxx_compiler_flag("-Wno-deprecated-register" 
CXX_SUPPORTS_NO_DEPRECATED_REGISTER)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-register" 
CXX_SUPPORTS_DEPRECATED_REGISTER)
+append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-vla-extension" CXX_SUPPORTS_NO_VLA_EXTENSION)
-append_if(CXX_SUPPORTS_NO_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wvla-extension" CXX_SUPPORTS_VLA_EXTENSION)
+append_if(CXX_SUPPORTS_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
 
 # Disable MSVC warnings
 if( MSVC )


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -168,22 +168,27 @@
 endif ()
 include_directories("${CMAKE_CURRENT_BINARY_DIR}/../clang/include")
 
+# GCC silently accepts any -Wno- option, but warns about those options
+# being unrecognized only if the compilation triggers other warnings to be
+# printed. Therefore, check for whether the compiler supports options in the
+# form -W, and if supported, add the corresponding -Wno- option.
+
 # Disable GCC warnings
-check_cxx_compiler_flag("-Wno-deprecated-declarations" CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-declarations" CXX_SUPPORTS_DEPRECATED_DECLARATIONS)
+append_if(CXX_SUPPORTS_DEPRECATED_DECLARATIONS "-Wno-deprecated-declarations" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-unknown-pragmas" CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS)
-append_if(CXX_SUPPORTS_NO_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wunknown-pragmas" CXX_SUPPORTS_UNKNOWN_PRAGMAS)
+append_if(CXX_SUPPORTS_UNKNOWN_PRAGMAS "-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-strict-aliasing" CXX_SUPPORTS_NO_STRICT_ALIASING)
-append_if(CXX_SUPPORTS_NO_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
+append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
 # Disable Clang warnings
-check_cxx_compiler_flag("-Wno-deprecated-register" CXX_SUPPORTS_NO_DEPRECATED_REGISTER)
-append_if(CXX_SUPPORTS_NO_DEPRECATED_REGISTER "-Wno-deprecated-register" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wdeprecated-register" CXX_SUPPORTS_DEPRECATED_REGISTER)
+append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" CMAKE_CXX_FLAGS)
 
-check_cxx_compiler_flag("-Wno-vla-extension" CXX_SUPPORTS_NO_VLA_EXTENSION)
-append_if(CXX_SUPPORTS_NO_VLA_EXTENSION "-Wno-vla-extension" CMAKE_CXX_FLAGS)
+check_cxx_compiler_flag("-Wvla-extension" CXX_SUPPORTS_VLA_EXTENSION)
+append_if(CXX_SU

[Lldb-commits] [PATCH] D123203: [lldb] Silence GCC warnings about missing returns after fully covered switches. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGae2aa2d21b24: [lldb] Silence GCC warnings about missing 
returns after fully covered switches. (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123203

Files:
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Core/DebuggerEvents.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp


Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -102,6 +102,7 @@
 case BitOffset:
   return 10;
 }
+llvm_unreachable("Fully covered switch above!");
   };
 
   auto createError = [&](const char *expected_value_message) {
Index: lldb/source/Core/DebuggerEvents.cpp
===
--- lldb/source/Core/DebuggerEvents.cpp
+++ lldb/source/Core/DebuggerEvents.cpp
@@ -52,6 +52,7 @@
   case Type::Error:
 return "error";
   }
+  llvm_unreachable("Fully covered switch above!");
 }
 
 void DiagnosticEventData::Dump(Stream *s) const {
Index: lldb/source/Breakpoint/Breakpoint.cpp
===
--- lldb/source/Breakpoint/Breakpoint.cpp
+++ lldb/source/Breakpoint/Breakpoint.cpp
@@ -1026,6 +1026,7 @@
 case eBreakpointEventTypeThreadChanged: return "thread changed";
 case eBreakpointEventTypeAutoContinueChanged: return "autocontinue 
changed";
   };
+  llvm_unreachable("Fully covered switch above!");
 }
 
 Log *Breakpoint::BreakpointEventData::GetLogChannel() {


Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -102,6 +102,7 @@
 case BitOffset:
   return 10;
 }
+llvm_unreachable("Fully covered switch above!");
   };
 
   auto createError = [&](const char *expected_value_message) {
Index: lldb/source/Core/DebuggerEvents.cpp
===
--- lldb/source/Core/DebuggerEvents.cpp
+++ lldb/source/Core/DebuggerEvents.cpp
@@ -52,6 +52,7 @@
   case Type::Error:
 return "error";
   }
+  llvm_unreachable("Fully covered switch above!");
 }
 
 void DiagnosticEventData::Dump(Stream *s) const {
Index: lldb/source/Breakpoint/Breakpoint.cpp
===
--- lldb/source/Breakpoint/Breakpoint.cpp
+++ lldb/source/Breakpoint/Breakpoint.cpp
@@ -1026,6 +1026,7 @@
 case eBreakpointEventTypeThreadChanged: return "thread changed";
 case eBreakpointEventTypeAutoContinueChanged: return "autocontinue changed";
   };
+  llvm_unreachable("Fully covered switch above!");
 }
 
 Log *Breakpoint::BreakpointEventData::GetLogChannel() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123205: [lldb] Silence GCC/glibc warnings about ignoring the return value of write(). NFC.

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6e38824221db: [lldb] Silence GCC/glibc warnings about 
ignoring the return value of write(). (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123205

Files:
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -893,7 +893,8 @@
 return result;
   }
 
-  write(fds[WRITE], data, size);
+  int r = write(fds[WRITE], data, size);
+  (void)r;
   // Close the write end of the pipe, so that the command interpreter will exit
   // when it consumes all the data.
   llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -893,7 +893,8 @@
 return result;
   }
 
-  write(fds[WRITE], data, size);
+  int r = write(fds[WRITE], data, size);
+  (void)r;
   // Close the write end of the pipe, so that the command interpreter will exit
   // when it consumes all the data.
   llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123254: [lldb] Disable GCC's -Wstringop-truncation warning. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, JDevlieghere.
Herald added subscribers: pengfei, mgorny.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

This warning gives false positives about lldb's correct use of
strncpy to fill fixed length fields that don't need null termination,
in lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp, like this:

  In file included from /usr/include/string.h:495,
   from /usr/include/c++/9/cstring:42,
   from ../include/llvm/ADT/StringRef.h:19,
   from 
../tools/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:10:
  In function ‘char* strncpy(char*, const char*, size_t)’,
  inlined from ‘lldb::offset_t CreateAllImageInfosPayload(const ProcessSP&, 
lldb::offset_t, lldb_private::StreamString&, lldb::SaveCoreStyle)’ at 
../tools/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:6341:16:
  /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:34: warning: ‘char* 
__builtin_strncpy(char*, const char*, long unsigned int)’ specified bound 16 
equals destination size [-Wstringop-truncation]
106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos 
(__dest));
|  ^~~

The warning could be squelched locally with

  #pragma GCC diagnostic ignored "-Wstringop-truncation"

too, but Clang also interprets those GCC pragmas, and produces
a -Wunknown-warning-option warning instead. That could be remedied
by wrapping the pragma in an "#ifndef __clang__" - but that makes
things even more messy. Instead, just silence this warning entirely.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123254

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -183,6 +183,9 @@
 check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
 append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
+check_cxx_compiler_flag("-Wstringop-truncation" 
CXX_SUPPORTS_STRINGOP_TRUNCATION)
+append_if(CXX_SUPPORTS_STRINGOP_TRUNCATION "-Wno-stringop-truncation" 
CMAKE_CXX_FLAGS)
+
 # Disable Clang warnings
 check_cxx_compiler_flag("-Wdeprecated-register" 
CXX_SUPPORTS_DEPRECATED_REGISTER)
 append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -183,6 +183,9 @@
 check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
 append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
+check_cxx_compiler_flag("-Wstringop-truncation" CXX_SUPPORTS_STRINGOP_TRUNCATION)
+append_if(CXX_SUPPORTS_STRINGOP_TRUNCATION "-Wno-stringop-truncation" CMAKE_CXX_FLAGS)
+
 # Disable Clang warnings
 check_cxx_compiler_flag("-Wdeprecated-register" CXX_SUPPORTS_DEPRECATED_REGISTER)
 append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" CMAKE_CXX_FLAGS)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123254: [lldb] Disable GCC's -Wstringop-truncation warning. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5560b9e88423: [lldb] [CMake] Disable GCC's 
-Wstringop-truncation warning. NFC. (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123254

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -183,6 +183,9 @@
 check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
 append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
+check_cxx_compiler_flag("-Wstringop-truncation" 
CXX_SUPPORTS_STRINGOP_TRUNCATION)
+append_if(CXX_SUPPORTS_STRINGOP_TRUNCATION "-Wno-stringop-truncation" 
CMAKE_CXX_FLAGS)
+
 # Disable Clang warnings
 check_cxx_compiler_flag("-Wdeprecated-register" 
CXX_SUPPORTS_DEPRECATED_REGISTER)
 append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -183,6 +183,9 @@
 check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
 append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
+check_cxx_compiler_flag("-Wstringop-truncation" CXX_SUPPORTS_STRINGOP_TRUNCATION)
+append_if(CXX_SUPPORTS_STRINGOP_TRUNCATION "-Wno-stringop-truncation" CMAKE_CXX_FLAGS)
+
 # Disable Clang warnings
 check_cxx_compiler_flag("-Wdeprecated-register" CXX_SUPPORTS_DEPRECATED_REGISTER)
 append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" CMAKE_CXX_FLAGS)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123204: [lldb] Fix initialization of LazyBool/bool variables m_overwrite/m_overwrite_lazy. NFCI.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

Ping @jingham


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123204

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

[Lldb-commits] [PATCH] D123206: [lldb] Silence warnings about unused static variables in RegisterInfos_arm64.h

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG373d08adb445: [lldb] Silence warnings about unused static 
variables in RegisterInfos_arm64.h (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123206

Files:
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h


Index: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -784,10 +784,5 @@
 {DEFINE_DBG(wcr, 15)}
 };
 // clang-format on
-static lldb_private::RegisterInfo g_register_infos_pauth[] = {
-DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
-
-static lldb_private::RegisterInfo g_register_infos_mte[] = {
-DEFINE_EXTENSION_REG(mte_ctrl)};
 
 #endif // DECLARE_REGISTER_INFOS_ARM64_STRUCT
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -72,6 +72,12 @@
 #include "RegisterInfos_arm64_sve.h"
 #undef DECLARE_REGISTER_INFOS_ARM64_STRUCT
 
+static lldb_private::RegisterInfo g_register_infos_pauth[] = {
+DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
+
+static lldb_private::RegisterInfo g_register_infos_mte[] = {
+DEFINE_EXTENSION_REG(mte_ctrl)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,


Index: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -784,10 +784,5 @@
 {DEFINE_DBG(wcr, 15)}
 };
 // clang-format on
-static lldb_private::RegisterInfo g_register_infos_pauth[] = {
-DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
-
-static lldb_private::RegisterInfo g_register_infos_mte[] = {
-DEFINE_EXTENSION_REG(mte_ctrl)};
 
 #endif // DECLARE_REGISTER_INFOS_ARM64_STRUCT
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -72,6 +72,12 @@
 #include "RegisterInfos_arm64_sve.h"
 #undef DECLARE_REGISTER_INFOS_ARM64_STRUCT
 
+static lldb_private::RegisterInfo g_register_infos_pauth[] = {
+DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
+
+static lldb_private::RegisterInfo g_register_infos_mte[] = {
+DEFINE_EXTENSION_REG(mte_ctrl)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122523: [lldb] Fix building standalone LLDB on Windows.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

@MehdiChinoune can you provide your preferred form of the git author line, 
`Real Name `?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122523

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

[Lldb-commits] [PATCH] D122523: [lldb] Fix building standalone LLDB on Windows.

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3031fa88f01e: [lldb] Fix building standalone LLDB on 
Windows. (authored by MehdiChinoune, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122523

Files:
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  llvm/include/llvm/Config/config.h.cmake
  llvm/include/llvm/Config/llvm-config.h.cmake


Index: llvm/include/llvm/Config/llvm-config.h.cmake
===
--- llvm/include/llvm/Config/llvm-config.h.cmake
+++ llvm/include/llvm/Config/llvm-config.h.cmake
@@ -113,4 +113,7 @@
  * in non assert builds */
 #cmakedefine01 LLVM_UNREACHABLE_OPTIMIZE
 
+/* Define to 1 if you have the DIA SDK installed, and to 0 if you don't. */
+#cmakedefine01 LLVM_ENABLE_DIA_SDK
+
 #endif
Index: llvm/include/llvm/Config/config.h.cmake
===
--- llvm/include/llvm/Config/config.h.cmake
+++ llvm/include/llvm/Config/config.h.cmake
@@ -50,9 +50,6 @@
don't. */
 #cmakedefine01 HAVE_DECL_STRERROR_S
 
-/* Define to 1 if you have the DIA SDK installed, and to 0 if you don't. */
-#cmakedefine01 LLVM_ENABLE_DIA_SDK
-
 /* Define to 1 if you have the  header file. */
 #cmakedefine HAVE_DLFCN_H ${HAVE_DLFCN_H}
 
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -57,7 +57,7 @@
 #include "Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h"
 
 #if defined(_WIN32)
-#include "llvm/Config/config.h"
+#include "llvm/Config/llvm-config.h"
 #endif
 
 using namespace lldb;


Index: llvm/include/llvm/Config/llvm-config.h.cmake
===
--- llvm/include/llvm/Config/llvm-config.h.cmake
+++ llvm/include/llvm/Config/llvm-config.h.cmake
@@ -113,4 +113,7 @@
  * in non assert builds */
 #cmakedefine01 LLVM_UNREACHABLE_OPTIMIZE
 
+/* Define to 1 if you have the DIA SDK installed, and to 0 if you don't. */
+#cmakedefine01 LLVM_ENABLE_DIA_SDK
+
 #endif
Index: llvm/include/llvm/Config/config.h.cmake
===
--- llvm/include/llvm/Config/config.h.cmake
+++ llvm/include/llvm/Config/config.h.cmake
@@ -50,9 +50,6 @@
don't. */
 #cmakedefine01 HAVE_DECL_STRERROR_S
 
-/* Define to 1 if you have the DIA SDK installed, and to 0 if you don't. */
-#cmakedefine01 LLVM_ENABLE_DIA_SDK
-
 /* Define to 1 if you have the  header file. */
 #cmakedefine HAVE_DLFCN_H ${HAVE_DLFCN_H}
 
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -57,7 +57,7 @@
 #include "Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h"
 
 #if defined(_WIN32)
-#include "llvm/Config/config.h"
+#include "llvm/Config/llvm-config.h"
 #endif
 
 using namespace lldb;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123204: [lldb] Fix initialization of LazyBool/bool variables m_overwrite/m_overwrite_lazy. NFCI.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

Ping @jingham


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123204

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

[Lldb-commits] [PATCH] D123204: [lldb] Fix initialization of LazyBool/bool variables m_overwrite/m_overwrite_lazy. NFCI.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

Ping @jingham


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123204

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

[Lldb-commits] [PATCH] D123204: [lldb] Fix initialization of LazyBool/bool variables m_overwrite/m_overwrite_lazy. NFCI.

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2a84a8618439: [lldb] Fix initialization of LazyBool/bool 
variables… (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123204

Files:
  lldb/source/Commands/CommandObjectCommands.cpp


Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -1480,7 +1480,7 @@
 std::string m_class_name;
 std::string m_funct_name;
 std::string m_short_help;
-LazyBool m_overwrite_lazy;
+LazyBool m_overwrite_lazy = eLazyBoolCalculate;
 ScriptedCommandSynchronicity m_synchronicity =
 eScriptedCommandSynchronicitySynchronous;
   };
@@ -1647,7 +1647,7 @@
   std::string m_cmd_name;
   CommandObjectMultiword *m_container = nullptr;
   std::string m_short_help;
-  bool m_overwrite = eLazyBoolCalculate;
+  bool m_overwrite = false;
   ScriptedCommandSynchronicity m_synchronicity =
   eScriptedCommandSynchronicitySynchronous;
 };


Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -1480,7 +1480,7 @@
 std::string m_class_name;
 std::string m_funct_name;
 std::string m_short_help;
-LazyBool m_overwrite_lazy;
+LazyBool m_overwrite_lazy = eLazyBoolCalculate;
 ScriptedCommandSynchronicity m_synchronicity =
 eScriptedCommandSynchronicitySynchronous;
   };
@@ -1647,7 +1647,7 @@
   std::string m_cmd_name;
   CommandObjectMultiword *m_container = nullptr;
   std::string m_short_help;
-  bool m_overwrite = eLazyBoolCalculate;
+  bool m_overwrite = false;
   ScriptedCommandSynchronicity m_synchronicity =
   eScriptedCommandSynchronicitySynchronous;
 };
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126557: [lldb] Fix cross compiling on macOS

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: JDevlieghere.
Herald added a subscriber: mgorny.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

When cross compiling, a separate nested cmake is spawned, for building
host code generation tools such as lldb-tblgen.

When cross compiling on macOS, the nested native build would trigger
the lldb check for libc++, if testing is enabled (which it is by default).
(Even if `LLDB_INCLUDE_TESTS=OFF` is set on the main build, it has to
be passed separately in `CROSS_TOOLCHAIN_FLAGS_NATIVE` to reach the
nested build.)

Skip this check when building the host tools when cross compiling, as
the user won't try to run tests in that nested build.

Alternatively, we could consider disabling all the `*_INCLUDE_TESTS`
by default in the nested host tools build.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126557

Files:
  lldb/test/CMakeLists.txt


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -103,7 +103,7 @@
 add_lldb_test_dependency(tsan)
   endif()
 
-  if(APPLE)
+  if(APPLE AND NOT LLVM_TARGET_IS_CROSSCOMPILE_HOST)
 # FIXME: Standalone builds should import the cxx target as well.
 if(LLDB_BUILT_STANDALONE)
   # For now check that the include directory exists.


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -103,7 +103,7 @@
 add_lldb_test_dependency(tsan)
   endif()
 
-  if(APPLE)
+  if(APPLE AND NOT LLVM_TARGET_IS_CROSSCOMPILE_HOST)
 # FIXME: Standalone builds should import the cxx target as well.
 if(LLDB_BUILT_STANDALONE)
   # For now check that the include directory exists.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126557: [lldb] Fix cross compiling on macOS

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG919107870739: [lldb] Fix cross compiling on macOS (authored 
by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126557

Files:
  lldb/test/CMakeLists.txt


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -103,7 +103,7 @@
 add_lldb_test_dependency(tsan)
   endif()
 
-  if(APPLE)
+  if(APPLE AND NOT LLVM_TARGET_IS_CROSSCOMPILE_HOST)
 # FIXME: Standalone builds should import the cxx target as well.
 if(LLDB_BUILT_STANDALONE)
   # For now check that the include directory exists.


Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -103,7 +103,7 @@
 add_lldb_test_dependency(tsan)
   endif()
 
-  if(APPLE)
+  if(APPLE AND NOT LLVM_TARGET_IS_CROSSCOMPILE_HOST)
 # FIXME: Standalone builds should import the cxx target as well.
 if(LLDB_BUILT_STANDALONE)
   # For now check that the include directory exists.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126367: [lldb] Add gnu-debuglink support for Windows PE/COFF

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp:119
+  };
+  for (SectionType section_type : g_sections) {
+if (SectionSP section_sp =

I'm curious - this adds new logic (copied from SymbolVendorELF afaik?) for 
iterating over sections and finding the ones that contain the dwarf debug info. 
Prior to this change, finding debug info within the executable itself has 
worked just fine.

What codepath and where has handled that? Has it fallen back on SymbolVendorELF 
so far?

(If that used to work, why is a specific plugin for PECOFF needed at this point 
for debuglink?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126367

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

[Lldb-commits] [PATCH] D126657: [lldb] Fix loading DLL from some ramdisk on Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a reviewer: labath.
mstorsjo added a comment.

The implementation looks reasonable to me (I didn't investigate alternative 
ways of doing it but trust the reasoning that this is the most reasonable way 
of finding the pathname of an open handle with this filesystem driver).




Comment at: lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp:427
+
+  auto view_deleter = [](void *pMem) { ::UnmapViewOfFile(pMem); };
+  std::unique_ptr pMem(

Do you need a check against `NULL` here in `view_deleter`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126657

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

[Lldb-commits] [PATCH] D126657: [lldb] Fix loading DLL from some ramdisk on Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp:427
+
+  auto view_deleter = [](void *pMem) { ::UnmapViewOfFile(pMem); };
+  std::unique_ptr pMem(

alvinhochun wrote:
> mstorsjo wrote:
> > Do you need a check against `NULL` here in `view_deleter`?
> I think `unique_ptr` guarantees the deleter won't be called if the pointer is 
> null, so I didn't put a null check here.
Sounds plausible - I tried browsing the documentation for that but didn't find 
it spelled out explicitly (in the couple minutes I was browsing at least).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126657

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

[Lldb-commits] [PATCH] D126367: [lldb] Add gnu-debuglink support for Windows PE/COFF

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp:119
+  };
+  for (SectionType section_type : g_sections) {
+if (SectionSP section_sp =

alvinhochun wrote:
> mstorsjo wrote:
> > I'm curious - this adds new logic (copied from SymbolVendorELF afaik?) for 
> > iterating over sections and finding the ones that contain the dwarf debug 
> > info. Prior to this change, finding debug info within the executable itself 
> > has worked just fine.
> > 
> > What codepath and where has handled that? Has it fallen back on 
> > SymbolVendorELF so far?
> > 
> > (If that used to work, why is a specific plugin for PECOFF needed at this 
> > point for debuglink?)
> The SymbolVendorELF seems to only handle loading the external debug info 
> specified by the `.gnu_debuglink` section. The dwarf debug info already 
> embedded in the main executable should be handled somewhere else, which has 
> worked fine for both ELF and PE/COFF. Though I don't know where specifically 
> this was handled.
> 
> SymbolVendorELF tries to downcast the object file to `ObjectFileELF *`, so it 
> could not have worked for PE/COFF.
Ok, so what I misunderstood is that this function doesn't seem to handle dwarf 
debug info in the main executable after all - this only tries to dig up dwarf 
sections from `GetSymbolFileFileSpec()` or `GetDebugLink()`. So the existing 
code that locates dwarf sections in the executables themselves still runs as 
before.

So then this seems reasonable.

So essentially, if `GetDebugLink()` would be a virtual method, both 
SymbolVendorELF and SymbolVendorPECOFF could theoretically be merged into one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126367

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

[Lldb-commits] [PATCH] D127048: [lldb] Set COFF and PDB module env from default target triple

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D127048#3559866 , @DavidSpickett 
wrote:

>> This changes the PE/COFF and PDB plugins to set the module triple according 
>> to the default target triple used to build LLDB.
>
> I'm missing some context here but using triple used when *building* lldb 
> seems to conflict with the idea that the same lldb (client at least) can be 
> used to debug many architectures.

Yes, that's true. The idea here is that when doing local debugging in either 
the mingw or msvc ecosystems, the flavour of the lldb build itself could be a 
decent hint - but it's not indeed not a correct and faultproof fix.

> Or is this not a concern because remote debugging is not a thing for Windows? 
> For instance, is it possible that I'm on Linux with some AArch64 linux 
> targeted lldb trying to remote debug a windows system. If the default triple 
> is the best signal you've got then fair enough I guess just wanted to check 
> there wasn't some more dynamic way to get it.
>
> And I assume PE/COFF have no embedded flag in them to indicate this ABI?

No, there's no flag for that. Mingw and MSVC environment binaries are quite the 
same.

(Qt Creator tries to do some similar heuristics to guess which ABI a binary 
uses, by looking at e.g. linker version numbers in the binary, to deduce 
whether a binary has been linked with GNU ld.bfd or MS link.exe - but when it 
comes to LLD, that logic falls apart. A more complex heuristic that can be used 
is e.g. looking at the list of DLLs that a binary links against, but that's 
also much more complex and also not entirely faultproof.)

@alvinhochun has got another patch in progress - 
https://reviews.llvm.org/D127053 (which isn't yet submitted for review as it 
lacks tests), which tries to decude the same based on whether a binary contains 
dwarf debug info.

That's more fault proof, but is also not an entirely complete fix - while 
finding dwarf debug sections mostly can imply using the itanium C++ ABI, you 
can also use PDB files with itanium C++ ABI (i.e. in mingw setups). It's less 
common but still a viable scenario. And reversely, I think I've heard about 
special cases where people use dwarf debug info with MSVC-ecosystem builds 
too...

So in short, what would be needed is a good enough heuristic that works for the 
vast majority of cases, and an option that lets users specify it for the cases 
that can't easily be autodetected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127048

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

[Lldb-commits] [PATCH] D126367: [lldb] Add gnu-debuglink support for Windows PE/COFF

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

FWIW, most of the logic for how to deal with the gnu debuglink contents here is 
copied from the corresponding preexisting methods for ELF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126367

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

[Lldb-commits] [PATCH] D127048: [lldb] Set COFF module ABI from default triple and make it an option

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D127048#3565557 , @omjavaid wrote:

> I am not fully familiar with PE/COFF but lets try to get this merged. For 
> starters I have a couple of questions here:
>
> 1. This rev apparently assumes x86_64 only but what about Arm/AArch64?

Where do you get that impression? This should all be architecture independent 
(or done in the same way for all architectures).

> 2. If we have DLLs what abi setting they ll follow the one set by the user or 
> default which may be MSVC?

Here, all DLLs are expected to use the same ABI (either the implicit one based 
on the LLDB build). D127234  is a follow-up 
that allows you to set a specific ABI choice per DLL.

> 3. Is there anything in PE/COFF header information that can serve is a good 
> guess for the underlying ABI.

No, there's no such hint. It's possible to make heuristic guesses based on e.g. 
what DLLs they import, but it's all guesses, possibly brittle. Having an option 
allows you to get it right even for the cases where the default is wrong.

> 4. What more information can we get from debug info in this case? DWARF may 
> not be a good guess as we can generate it for MSVC abi as well.

Exactly, one could use presence of DWARF as a heuristic hint for this, but it's 
not always right (you can use PDB for mingw uses too, and DWARF for msvc abi 
too in uncommon cases).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127048

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

[Lldb-commits] [PATCH] D126367: [lldb] Add gnu-debuglink support for Windows PE/COFF

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

When testing this patch, I ran into crashes in the 
`SymbolFile/NativePDB/load-pdb.cpp` testcase. To fix it, I had to add a change 
like this:

  diff --git a/lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp 
b/lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp
  index 26df0035b37c..e08753b86d5b 100644
  --- a/lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp
  +++ b/lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp
  @@ -108,6 +108,8 @@ SymbolVendorPECOFF::CreateInstance(const lldb::ModuleSP 
&module_sp,
 // that.
 SectionList *module_section_list = module_sp->GetSectionList();
 SectionList *objfile_section_list = dsym_objfile_sp->GetSectionList();
  +  if (!objfile_section_list || !module_section_list)
  +return nullptr;
   
 static const SectionType g_sections[] = { 
 eSectionTypeDWARFDebugAbbrev,   eSectionTypeDWARFDebugAranges,

Also, when built with assertions enabled (which often is to prefer during 
development, to catch things before the buildbots do), reading the debuglink 
info failed an assert:

  lldb/source/Utility/DataExtractor.cpp:135: 
lldb_private::DataExtractor::DataExtractor(const void*, lldb::offset_t, 
lldb::ByteOrder, uint32_t, uint32_t): Assertion `addr_size >= 1 && addr_size <= 
8' failed.

This is easy to fix with a change like this:

  diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  index ba4612178be7..657fcdc5af6d 100644
  --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  @@ -63,7 +63,7 @@ static bool GetDebugLinkContents(const 
llvm::object::COFFObjectFile &coff_obj,
 }
 DataExtractor data(
 content->data(), content->size(),
  -  coff_obj.isLittleEndian() ? eByteOrderLittle : eByteOrderBig, 0);
  +  coff_obj.isLittleEndian() ? eByteOrderLittle : eByteOrderBig, 4);
 lldb::offset_t gnu_debuglink_offset = 0;
 gnu_debuglink_file = data.GetCStr(&gnu_debuglink_offset);
 // Align to the next 4-byte offset

If it's ok with you, I can squash these changes (plus a rewording regarding GNU 
ld and the codeview build ID) and push the patch.




Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:96
+
+  // The GNU linker does not write a PDB build id, so we should fall back to
+  // using the crc from .gnu_debuglink if it exists, just like how 
ObjectFileELF

Actually, the GNU linker does write such a build id if you pass it 
`-Wl,--build-id`, but it's not default like in LLD.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126367

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

[Lldb-commits] [PATCH] D127234: [lldb] Add setting to override PE/COFF ABI by module name

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a reviewer: omjavaid.
mstorsjo added a comment.

(I guess this one might need some updates after D127048 
 is finalized?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127234

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

[Lldb-commits] [PATCH] D126367: [lldb] Add gnu-debuglink support for Windows PE/COFF

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc8daf4a707ad: [lldb] Add gnu-debuglink support for Windows 
PE/COFF (authored by alvinhochun, committed by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D126367?vs=435214&id=435498#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126367

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Plugins/SymbolVendor/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/PECOFF/CMakeLists.txt
  lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp
  lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.h
  lldb/source/Symbol/LocateSymbolFile.cpp
  lldb/test/Shell/Minidump/Windows/Inputs/find-module.exe.yaml
  lldb/test/Shell/ObjectFile/PECOFF/dwarf-gnu-debuglink-i686.yaml
  lldb/test/Shell/ObjectFile/PECOFF/dwarf-gnu-debuglink-mismatched-crc.yaml
  lldb/test/Shell/ObjectFile/PECOFF/dwarf-gnu-debuglink-pdb-buildid.yaml
  lldb/test/Shell/ObjectFile/PECOFF/dwarf-gnu-debuglink.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/dwarf-gnu-debuglink.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/dwarf-gnu-debuglink.yaml
@@ -0,0 +1,50 @@
+# This test produces a stripped version of the object file and adds a
+# gnu-debuglink section to it linking to the unstripped version of the object
+# file. The debug info shall be loaded from the gnu-debuglink reference.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy --strip-all --add-gnu-debuglink=%t %t %t.stripped
+# RUN: lldb-test object-file %t.stripped | FileCheck %s
+
+# CHECK: Name: .debug_info
+# CHECK-NEXT: Type: dwarf-info
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 5152
+  ImageBase:   5368709120
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.data
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.debug_info
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  16384
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+symbols: []
+...
Index: lldb/test/Shell/ObjectFile/PECOFF/dwarf-gnu-debuglink-pdb-buildid.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/dwarf-gnu-debuglink-pdb-buildid.yaml
@@ -0,0 +1,63 @@
+# This test produces a stripped version of the object file and adds a
+# gnu-debuglink section to it linking to the unstripped version of the object
+# file. Then the unstripped version is stripped to keep only debug info to
+# cause its crc to change. In this case the object files still have the
+# synthetic PDB build id that LLD adds even for the mingw target. Because this
+# build id still matches, the debug info shall still be loaded from the
+# gnu-debuglink reference.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy --strip-all --add-gnu-debuglink=%t %t %t.stripped
+# RUN: llvm-strip --only-keep-debug %t
+# RUN: lldb-test object-file %t.stripped | FileCheck %s
+
+# CHECK: Name: .debug_info
+# CHECK-NEXT: Type: dwarf-info
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 5152
+  ImageBase:   5368709120
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  Debug:
+ 

[Lldb-commits] [PATCH] D127048: [lldb] Set COFF module ABI from default triple and make it an option

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG25c8a061c573: [lldb] Set COFF module ABI from default triple 
and make it an option (authored by alvinhochun, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127048

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/source/Core/PluginManager.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
  lldb/test/Shell/ObjectFile/PECOFF/basic-info-arm.yaml
  lldb/test/Shell/ObjectFile/PECOFF/basic-info-arm64.yaml
  lldb/test/Shell/ObjectFile/PECOFF/basic-info.yaml
  lldb/test/Shell/ObjectFile/PECOFF/default-triple-windows-gnu.yaml
  lldb/test/Shell/ObjectFile/PECOFF/default-triple-windows-msvc.yaml
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
  lldb/test/Shell/lit.cfg.py

Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -62,6 +62,12 @@
 if re.match(r'^arm(hf.*-linux)|(.*-linux-gnuabihf)', config.target_triple):
 config.available_features.add("armhf-linux")
 
+if re.match(r'.*-(windows-msvc)$', config.target_triple):
+config.available_features.add("windows-msvc")
+
+if re.match(r'.*-(windows-gnu|mingw32)$', config.target_triple):
+config.available_features.add("windows-gnu")
+
 def calculate_arch_features(arch_string):
 # This will add a feature such as x86, arm, mips, etc for each built
 # target
Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
@@ -0,0 +1,49 @@
+# RUN: yaml2obj %s -o %t
+
+## Default ABI is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+# CHECK-LABEL: image list --triple --basename
+# CHECK-NEXT: x86_64-pc-windows-[[ABI]] [[FILENAME]]
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 5152
+  ImageBase:   5368709120
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.data
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+symbols: []
+...
Index: lldb/test/Shell/ObjectFile/PECOFF/default-triple-windows-msvc.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/default-triple-windows-msvc.yaml
@@ -0,0 +1,41 @@
+# XFAIL: windows-gnu
+
+# RUN: yaml2obj %s -o %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Architecture: x86_64-pc-windows-msvc
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 5152
+  ImageBase:   5368709120
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+

[Lldb-commits] [PATCH] D126657: [lldb] Fix loading DLL from some ramdisk on Windows

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG87a2dba14ec8: [lldb] Fix loading DLL from some ramdisk on 
Windows (authored by alvinhochun, committed by mstorsjo).
Herald added a subscriber: Michael137.

Changed prior to commit:
  https://reviews.llvm.org/D126657?vs=432931&id=437144#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126657

Files:
  lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp

Index: lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
+++ lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/ProcessLaunchInfo.h"
 #include "lldb/Host/ThreadLauncher.h"
+#include "lldb/Host/windows/AutoHandle.h"
 #include "lldb/Host/windows/HostProcessWindows.h"
 #include "lldb/Host/windows/HostThreadWindows.h"
 #include "lldb/Host/windows/ProcessLauncherWindows.h"
@@ -29,6 +30,8 @@
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
 
+#include 
+
 #ifndef STATUS_WX86_BREAKPOINT
 #define STATUS_WX86_BREAKPOINT 0x401FL // For WOW64
 #endif
@@ -409,6 +412,61 @@
   return DBG_CONTINUE;
 }
 
+static llvm::Optional GetFileNameFromHandleFallback(HANDLE hFile) {
+  // Check that file is not empty as we cannot map a file with zero length.
+  DWORD dwFileSizeHi = 0;
+  DWORD dwFileSizeLo = ::GetFileSize(hFile, &dwFileSizeHi);
+  if (dwFileSizeLo == 0 && dwFileSizeHi == 0)
+return llvm::None;
+
+  AutoHandle filemap(
+  ::CreateFileMappingW(hFile, nullptr, PAGE_READONLY, 0, 1, NULL), nullptr);
+  if (!filemap.IsValid())
+return llvm::None;
+
+  auto view_deleter = [](void *pMem) { ::UnmapViewOfFile(pMem); };
+  std::unique_ptr pMem(
+  ::MapViewOfFile(filemap.get(), FILE_MAP_READ, 0, 0, 1), view_deleter);
+  if (!pMem)
+return llvm::None;
+
+  std::array mapped_filename;
+  if (!::GetMappedFileNameW(::GetCurrentProcess(), pMem.get(),
+mapped_filename.data(), mapped_filename.size()))
+return llvm::None;
+
+  // A series of null-terminated strings, plus an additional null character
+  std::array drive_strings;
+  drive_strings[0] = L'\0';
+  if (!::GetLogicalDriveStringsW(drive_strings.size(), drive_strings.data()))
+return llvm::None;
+
+  std::array drive = {L"_:"};
+  for (const wchar_t *it = drive_strings.data(); *it != L'\0';
+   it += wcslen(it) + 1) {
+// Copy the drive letter to the template string
+drive[0] = it[0];
+std::array device_name;
+if (::QueryDosDeviceW(drive.data(), device_name.data(),
+  device_name.size())) {
+  size_t device_name_len = wcslen(device_name.data());
+  if (device_name_len < mapped_filename.size()) {
+bool match = _wcsnicmp(mapped_filename.data(), device_name.data(),
+   device_name_len) == 0;
+if (match && mapped_filename[device_name_len] == L'\\') {
+  // Replace device path with its drive letter
+  std::wstring rebuilt_path(drive.data());
+  rebuilt_path.append(&mapped_filename[device_name_len]);
+  std::string path_utf8;
+  llvm::convertWideToUTF8(rebuilt_path, path_utf8);
+  return path_utf8;
+}
+  }
+}
+  }
+  return llvm::None;
+}
+
 DWORD
 DebuggerThread::HandleLoadDllEvent(const LOAD_DLL_DEBUG_INFO &info,
DWORD thread_id) {
@@ -420,6 +478,17 @@
 return DBG_CONTINUE;
   }
 
+  auto on_load_dll = [&](llvm::StringRef path) {
+FileSpec file_spec(path);
+ModuleSpec module_spec(file_spec);
+lldb::addr_t load_addr = reinterpret_cast(info.lpBaseOfDll);
+
+LLDB_LOG(log, "Inferior {0} - DLL '{1}' loaded at address {2:x}...",
+ m_process.GetProcessId(), path, info.lpBaseOfDll);
+
+m_debug_delegate->OnLoadDll(module_spec, load_addr);
+  };
+
   std::vector buffer(1);
   DWORD required_size =
   GetFinalPathNameByHandleW(info.hFile, &buffer[0], 0, VOLUME_NAME_DOS);
@@ -434,14 +503,10 @@
 if (path_str.startswith("?\\"))
   path += 4;
 
-FileSpec file_spec(path);
-ModuleSpec module_spec(file_spec);
-lldb::addr_t load_addr = reinterpret_cast(info.lpBaseOfDll);
-
-LLDB_LOG(log, "Inferior {0} - DLL '{1}' loaded at address {2:x}...",
- m_process.GetProcessId(), path, info.lpBaseOfDll);
-
-m_debug_delegate->OnLoadDll(module_spec, load_addr);
+on_load_dll(path);
+  } else if (llvm::Optional path =
+ GetFileNameFromHandleFallback(info.hFile)) {
+on_load_dll(*path);
   } else {
 LLDB_LOG(
 log,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/list

[Lldb-commits] [PATCH] D126657: [lldb] Fix loading DLL from some ramdisk on Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp:445
+  std::array drive = {L"_:"};
+  for (auto it = drive_strings.cbegin(); *it != L'\0'; it += wcslen(it) + 1) {
+// Copy the drive letter to the template string

FWIW, I had to change `auto it = drive_strings.cbegin()` here into `const 
wchar_t *it = drive_strings.data()` here, before pushing it, because with MSVC, 
`wcslen(it)` didn't automatically convert the iterator to a pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126657

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

[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D127436#3574289 , @alvinhochun 
wrote:

> Does this test look good:

With a bunch of minor fixups, this testcase does work - I uploaded a working 
copy of it at https://martin.st/temp/command-target-create-resolve-exe.test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

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

[Lldb-commits] [PATCH] D128226: [lldb] Remove an outdated comment. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, DavidSpickett, omjavaid, alvinhochun.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

This comment became outdated in 053eb35651906e693906fad6c695fce11415ade7
(but was moved along); that commit moved the code and the comment
to a separate function, with a separate local variable
`num_of_bytes_read`. On error, the possibly garbage value is never
copied back to the caller's reference, thus the comment is no longer
relevant (and slightly confusing as is).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128226

Files:
  lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp


Index: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -281,10 +281,6 @@
   SIZE_T num_of_bytes_read = 0;
   if (!::ReadProcessMemory(process.GetNativeProcess().GetSystemHandle(), addr,
buf, size, &num_of_bytes_read)) {
-// Reading from the process can fail for a number of reasons - set the
-// error code and make sure that the number of bytes read is set back to 0
-// because in some scenarios the value of bytes_read returned from the API
-// is garbage.
 error.SetError(GetLastError(), eErrorTypeWin32);
 LLDB_LOG(log, "reading failed with error: {0}", error);
   } else {


Index: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -281,10 +281,6 @@
   SIZE_T num_of_bytes_read = 0;
   if (!::ReadProcessMemory(process.GetNativeProcess().GetSystemHandle(), addr,
buf, size, &num_of_bytes_read)) {
-// Reading from the process can fail for a number of reasons - set the
-// error code and make sure that the number of bytes read is set back to 0
-// because in some scenarios the value of bytes_read returned from the API
-// is garbage.
 error.SetError(GetLastError(), eErrorTypeWin32);
 LLDB_LOG(log, "reading failed with error: {0}", error);
   } else {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, DavidSpickett, omjavaid, alvinhochun.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

25c8a061c5739677d2fc0af29a8cc9520207b923 
 / D127048 
 added an option
for setting the ABI to GNU.

When an object file is loaded, there's only minimal verification
done for the architecture spec set for it, if the object file only
provides one.

However, for i386 object files, the PECOFF object file plugin
provides two architectures, i386-pc-windows and i686-pc-windows.
This picks a totally different codepath in
TargetList::CreateTargetInternal, where it's treated as a fat
binary. This goes through more verifications to see if the
architectures provided by the object file matches what the
platform plugin supports.

The PlatformWindows() constructor explicitly adds the
"i386-pc-windows" and "i686-pc-windows" architectures (even when
running on other architectures), which allows this "fat binary
verification" to succeed for the i386 object files that provide
two architectures.

However, after this commit, if the object file is advertised with
the different environment (either when built in a mingw environment,
or if that setting is set), the fat binary validation won't accept
the file any longer.

Update ArchSpec::IsEqualTo with more logic for the Windows use
cases; mismatching vendors is not an issue (they don't have any
practical effect on Windows), and GNU and MSVC environments are
compatible to the point that PlatformWindows can handle object
files for both environments/ABIs.

As a separate path forward, one could also consider to stop returning
two architecture specs from ObjectFilePECOFF::GetModuleSpecifications
for i386 files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128268

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -0,0 +1,54 @@
+# RUN: yaml2obj %s -o %t
+
+## Default ABI is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+# CHECK-LABEL: image list --triple --basename
+# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4480
+  ImageBase:   268435456
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.data
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.debug_info
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  16384
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+symbols: []
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -979,22 +979,29 @@
 
   const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor();
   const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor();
-  if (lhs_triple_vendor != rhs_triple_vendor) {
-const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
-const bool lhs_vendor_specified = TripleVendorWasSpecified();
-// Both architectures had the vendor specified, so if they aren't equal
-// then we return false
-if (rhs_vendor_specified && lhs_vendor_specified)
-  return false;
-
-

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D128268#3598967 , @DavidSpickett 
wrote:

>> However, after this commit, if the object file is advertised with the 
>> different environment (either when built in a mingw environment, or if that 
>> setting is set), the fat binary validation won't accept the file any longer.
>
> Is "this commit" referring to 
> https://reviews.llvm.org/rG25c8a061c5739677d2fc0af29a8cc9520207b923 or to the 
> change we're reviewing here?

Sorry, I meant 25c8a061c5739677d2fc0af29a8cc9520207b923 
.

> I'm struggling to think through what this is actually fixing. Is the issue 
> that one half of a fat binary can have a different ABI? Wouldn't both sides 
> of the fat binary be loaded as the ABI chosen in the setting, or is that 
> exactly what you're fixing.

The issue is that in the case of a fat binary, we run a loop to check and pick 
an architecture out of it: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Target/TargetList.cpp#L173-L184
 This is checked against the architectures added here: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp#L127-L131

(In the case of a non-fat binary, i.e. all other architectures than i386/i686 
on PECOFF, we hit this case here, where we don't validate the arch spec at all: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Target/TargetList.cpp#L160-L164)

Now since the addition of the new setting in 
25c8a061c5739677d2fc0af29a8cc9520207b923 
, 
`ObjectFile::GetModuleSpecifications` can return `i386-pc-windows-gnu` and 
`i686-pc-windows-gnu`, while `PlatformWindows` provides a list containing 
`i386-pc-windows` (which is normalized to `i386-pc-windows-msvc`) and 
`i686-pc-windows` (`...-msvc`). Due to the differing environments, these are 
deemed incompatible, and LLDB flat out refuses to load the file, with the error 
message `error: no matching platforms found for this file`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

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

Added a comment to the testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -0,0 +1,61 @@
+## Check that i386 executables can be opened in both ABI modes.
+## I386 executables are special in the sense that the PECOFF object
+## file plugin returns two architectures for them, i386 and i686, which
+## causes more elaborate comparisons to be run against the Platform plugin's
+## architecture list. Check that this is accepted despite potential mismatches
+## in the environment part of the triple.
+
+# RUN: yaml2obj %s -o %t
+
+## Default ABI is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+# CHECK-LABEL: image list --triple --basename
+# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4480
+  ImageBase:   268435456
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.data
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.debug_info
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  16384
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+symbols: []
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -979,22 +979,29 @@
 
   const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor();
   const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor();
-  if (lhs_triple_vendor != rhs_triple_vendor) {
-const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
-const bool lhs_vendor_specified = TripleVendorWasSpecified();
-// Both architectures had the vendor specified, so if they aren't equal
-// then we return false
-if (rhs_vendor_specified && lhs_vendor_specified)
-  return false;
-
-// Only fail if both vendor types are not unknown
-if (lhs_triple_vendor != llvm::Triple::UnknownVendor &&
-rhs_triple_vendor != llvm::Triple::UnknownVendor)
-  return false;
-  }
 
   const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
   const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
+
+  if (lhs_triple_vendor != rhs_triple_vendor) {
+// On Windows, the vendor field doesn't have any practical effect, but
+// it is often set to either "pc" or "w64".
+if (exact_match || lhs_triple_os != llvm::Triple::Win32 ||
+rhs_triple_os != llvm::Triple::Win32) {
+  const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
+  const bool lhs_vendor_specified = TripleVendorWasSpecified();
+  // Both architectures had the vendor specified, so if they aren't equal
+  // then we return false
+  if (rhs_vendor_specified && lhs_vendor_specified)
+return false;
+
+  // Only fail if both vendor types are not unknown
+  if (lhs_triple_vendor != llvm::Triple::UnknownVendor &&
+  rhs_triple_vendor != llvm::Triple::UnknownVendor)
+return false;
+}
+  }
+
   const llvm::Triple::EnvironmentType lhs_triple_env =
   lhs_triple.getEnvironment();
   const llvm::Trip

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D128268#3601104 , @DavidSpickett 
wrote:

> LGTM. I like the ifs the way I suggested but up to you, either way it's not 
> the easiest logic to parse.

Oh, sorry, I forgot about that comment. Yes, that does indeed look even nicer, 
I'll try to add that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

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

Refactored the code to add the `both_windows` variable as suggested (which 
turned out much nicer, thanks!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -0,0 +1,61 @@
+## Check that i386 executables can be opened in both ABI modes.
+## I386 executables are special in the sense that the PECOFF object
+## file plugin returns two architectures for them, i386 and i686, which
+## causes more elaborate comparisons to be run against the Platform plugin's
+## architecture list. Check that this is accepted despite potential mismatches
+## in the environment part of the triple.
+
+# RUN: yaml2obj %s -o %t
+
+## Default ABI is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+# CHECK-LABEL: image list --triple --basename
+# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4480
+  ImageBase:   268435456
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.data
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.debug_info
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  16384
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+symbols: []
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -979,7 +979,16 @@
 
   const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor();
   const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor();
-  if (lhs_triple_vendor != rhs_triple_vendor) {
+
+  const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
+  const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
+
+  bool both_windows = lhs_triple.isOSWindows() && rhs_triple.isOSWindows();
+
+  // On Windows, the vendor field doesn't have any practical effect, but
+  // it is often set to either "pc" or "w64".
+  if ((lhs_triple_vendor != rhs_triple_vendor) &&
+  (exact_match || !both_windows)) {
 const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
 const bool lhs_vendor_specified = TripleVendorWasSpecified();
 // Both architectures had the vendor specified, so if they aren't equal
@@ -993,8 +1002,6 @@
   return false;
   }
 
-  const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
-  const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
   const llvm::Triple::EnvironmentType lhs_triple_env =
   lhs_triple.getEnvironment();
   const llvm::Triple::EnvironmentType rhs_triple_env =
@@ -1032,6 +1039,9 @@
   return true;
   }
 
+  if (!exact_match && both_windows)
+return true; // The Windows environments (MSVC vs GNU) are compatible
+
   return IsCompatibleEnvironment(lhs_triple_env, rhs_triple_env);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128226: [lldb] Remove an outdated comment. NFC.

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9846a1f2d472: [lldb] Remove an outdated comment. NFC. 
(authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128226

Files:
  lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp


Index: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -281,10 +281,6 @@
   SIZE_T num_of_bytes_read = 0;
   if (!::ReadProcessMemory(process.GetNativeProcess().GetSystemHandle(), addr,
buf, size, &num_of_bytes_read)) {
-// Reading from the process can fail for a number of reasons - set the
-// error code and make sure that the number of bytes read is set back to 0
-// because in some scenarios the value of bytes_read returned from the API
-// is garbage.
 error.SetError(GetLastError(), eErrorTypeWin32);
 LLDB_LOG(log, "reading failed with error: {0}", error);
   } else {


Index: lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -281,10 +281,6 @@
   SIZE_T num_of_bytes_read = 0;
   if (!::ReadProcessMemory(process.GetNativeProcess().GetSystemHandle(), addr,
buf, size, &num_of_bytes_read)) {
-// Reading from the process can fail for a number of reasons - set the
-// error code and make sure that the number of bytes read is set back to 0
-// because in some scenarios the value of bytes_read returned from the API
-// is garbage.
 error.SetError(GetLastError(), eErrorTypeWin32);
 LLDB_LOG(log, "reading failed with error: {0}", error);
   } else {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a64dd5b0614: [lldb] Fix reading i686-windows executables 
with GNU environment (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -0,0 +1,61 @@
+## Check that i386 executables can be opened in both ABI modes.
+## I386 executables are special in the sense that the PECOFF object
+## file plugin returns two architectures for them, i386 and i686, which
+## causes more elaborate comparisons to be run against the Platform plugin's
+## architecture list. Check that this is accepted despite potential mismatches
+## in the environment part of the triple.
+
+# RUN: yaml2obj %s -o %t
+
+## Default ABI is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+# CHECK-LABEL: image list --triple --basename
+# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4480
+  ImageBase:   268435456
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.data
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.debug_info
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  16384
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+symbols: []
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -979,7 +979,16 @@
 
   const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor();
   const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor();
-  if (lhs_triple_vendor != rhs_triple_vendor) {
+
+  const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
+  const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
+
+  bool both_windows = lhs_triple.isOSWindows() && rhs_triple.isOSWindows();
+
+  // On Windows, the vendor field doesn't have any practical effect, but
+  // it is often set to either "pc" or "w64".
+  if ((lhs_triple_vendor != rhs_triple_vendor) &&
+  (exact_match || !both_windows)) {
 const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
 const bool lhs_vendor_specified = TripleVendorWasSpecified();
 // Both architectures had the vendor specified, so if they aren't equal
@@ -993,8 +1002,6 @@
   return false;
   }
 
-  const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
-  const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
   const llvm::Triple::EnvironmentType lhs_triple_env =
   lhs_triple.getEnvironment();
   const llvm::Triple::EnvironmentType rhs_triple_env =
@@ -1032,6 +1039,9 @@
   return true;
   }
 
+  if (!exact_match && both_windows)
+return true; // The Windows environments (MSVC vs GNU) are compatible
+
   return IsCompatibleEnvironment(lhs_triple_env, rhs_triple_env);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2bae95605753: [lldb] Resolve exe location for `target 
create` (authored by alvinhochun, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

Files:
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Target/TargetList.cpp
  lldb/test/Shell/Commands/command-target-create-resolve-exe.test


Index: lldb/test/Shell/Commands/command-target-create-resolve-exe.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-target-create-resolve-exe.test
@@ -0,0 +1,28 @@
+# REQUIRES: system-windows
+
+## This checks that when starting lldb (or using `target create`) with a
+## program name which is on $PATH, or not specify the .exe suffix of a program
+## in the working directory on Windows, lldb can still detect the target
+## architecture correctly instead of producing an error.
+
+# RUN: mkdir -p "%t.dir"
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.dir/testmain.exe
+
+## Test with full path to exe
+# RUN: %lldb %t.dir/testmain.exe -b | FileCheck %s
+
+## Test with exe on path, with .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain.exe -b | FileCheck %s
+
+## Test with exe on path, without .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain -b | FileCheck %s
+
+## Test in cwd, with .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain.exe -b | FileCheck %s
+
+## Test in cwd, without .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain -b | FileCheck %s
+
+# CHECK-LABEL: target create
+# CHECK-NEXT: Current executable set to '{{.*[/\\]}}testmain.exe'
+# CHECK-NOT: Error
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -121,6 +121,14 @@
   if (!user_exe_path.empty()) {
 ModuleSpec module_spec(FileSpec(user_exe_path, FileSpec::Style::native));
 FileSystem::Instance().Resolve(module_spec.GetFileSpec());
+
+// Try to resolve the exe based on PATH and/or platform-specific suffixes,
+// but only if using the host platform.
+if (platform_sp->IsHost() &&
+!FileSystem::Instance().Exists(module_spec.GetFileSpec()))
+  FileSystem::Instance().ResolveExecutableLocation(
+  module_spec.GetFileSpec());
+
 // Resolve the executable in case we are given a path to a application
 // bundle like a .app bundle on MacOSX.
 Host::ResolveExecutableInBundle(module_spec.GetFileSpec());
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -299,12 +299,6 @@
 
   const char *file_path = command.GetArgumentAtIndex(0);
   LLDB_SCOPED_TIMERF("(lldb) target create '%s'", file_path);
-  FileSpec file_spec;
-
-  if (file_path) {
-file_spec.SetFile(file_path, FileSpec::Style::native);
-FileSystem::Instance().Resolve(file_spec);
-  }
 
   bool must_set_platform_path = false;
 
@@ -333,6 +327,18 @@
 
   PlatformSP platform_sp = target_sp->GetPlatform();
 
+  FileSpec file_spec;
+  if (file_path) {
+file_spec.SetFile(file_path, FileSpec::Style::native);
+FileSystem::Instance().Resolve(file_spec);
+
+// Try to resolve the exe based on PATH and/or platform-specific
+// suffixes, but only if using the host platform.
+if (platform_sp && platform_sp->IsHost() &&
+!FileSystem::Instance().Exists(file_spec))
+  FileSystem::Instance().ResolveExecutableLocation(file_spec);
+  }
+
   if (remote_file) {
 if (platform_sp) {
   // I have a remote file.. two possible cases


Index: lldb/test/Shell/Commands/command-target-create-resolve-exe.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-target-create-resolve-exe.test
@@ -0,0 +1,28 @@
+# REQUIRES: system-windows
+
+## This checks that when starting lldb (or using `target create`) with a
+## program name which is on $PATH, or not specify the .exe suffix of a program
+## in the working directory on Windows, lldb can still detect the target
+## architecture correctly instead of producing an error.
+
+# RUN: mkdir -p "%t.dir"
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.dir/testmain.exe
+
+## Test with full path to exe
+# RUN: %lldb %t.dir/testmain.exe -b | FileCheck %s
+
+## Test with exe on path, with .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain.exe -b | FileCheck %s
+
+## Test with exe on path, without .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain -b | FileCheck %s
+
+## Test in cwd, with .exe suffix
+# RUN: cd "%t.dir" && %lldb testmai

[Lldb-commits] [PATCH] D128201: [lldb][windows] Fix crash on getting nested exception

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4d123783957e: [lldb][windows] Fix crash on getting nested 
exception (authored by alvinhochun, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128201

Files:
  lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h


Index: lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
===
--- lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
+++ lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
@@ -25,11 +25,17 @@
 class ExceptionRecord {
 public:
   ExceptionRecord(const EXCEPTION_RECORD &record, lldb::tid_t thread_id) {
+// Notes about the `record.ExceptionRecord` field:
+// In the past, some code tried to parse the nested exception with it, but
+// in practice, that code just causes Access Violation. I suspect
+// `ExceptionRecord` here actually points to the address space of the
+// debuggee process. However, I did not manage to find any official or
+// unofficial reference that clarifies this point. If anyone would like to
+// reimplement this, please also keep in mind to check how this behaves 
when
+// debugging a WOW64 process. I suspect you may have to use the explicit
+// `EXCEPTION_RECORD32` and `EXCEPTION_RECORD64` structs.
 m_code = record.ExceptionCode;
 m_continuable = (record.ExceptionFlags == 0);
-if (record.ExceptionRecord)
-  m_next_exception.reset(
-  new ExceptionRecord(*record.ExceptionRecord, thread_id));
 m_exception_addr = reinterpret_cast(record.ExceptionAddress);
 m_thread_id = thread_id;
 m_arguments.assign(record.ExceptionInformation,
@@ -39,27 +45,16 @@
   // MINIDUMP_EXCEPTIONs are almost identical to EXCEPTION_RECORDs.
   ExceptionRecord(const MINIDUMP_EXCEPTION &record, lldb::tid_t thread_id)
   : m_code(record.ExceptionCode), m_continuable(record.ExceptionFlags == 
0),
-m_next_exception(nullptr),
 m_exception_addr(static_cast(record.ExceptionAddress)),
 m_thread_id(thread_id),
 m_arguments(record.ExceptionInformation,
-record.ExceptionInformation + record.NumberParameters) {
-// Set up link to nested exception.
-if (record.ExceptionRecord) {
-  m_next_exception.reset(new ExceptionRecord(
-  *reinterpret_cast(record.ExceptionRecord),
-  thread_id));
-}
-  }
+record.ExceptionInformation + record.NumberParameters) {}
 
   virtual ~ExceptionRecord() {}
 
   DWORD
   GetExceptionCode() const { return m_code; }
   bool IsContinuable() const { return m_continuable; }
-  const ExceptionRecord *GetNextException() const {
-return m_next_exception.get();
-  }
   lldb::addr_t GetExceptionAddress() const { return m_exception_addr; }
 
   lldb::tid_t GetThreadID() const { return m_thread_id; }
@@ -69,7 +64,6 @@
 private:
   DWORD m_code;
   bool m_continuable;
-  std::shared_ptr m_next_exception;
   lldb::addr_t m_exception_addr;
   lldb::tid_t m_thread_id;
   std::vector m_arguments;


Index: lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
===
--- lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
+++ lldb/source/Plugins/Process/Windows/Common/ExceptionRecord.h
@@ -25,11 +25,17 @@
 class ExceptionRecord {
 public:
   ExceptionRecord(const EXCEPTION_RECORD &record, lldb::tid_t thread_id) {
+// Notes about the `record.ExceptionRecord` field:
+// In the past, some code tried to parse the nested exception with it, but
+// in practice, that code just causes Access Violation. I suspect
+// `ExceptionRecord` here actually points to the address space of the
+// debuggee process. However, I did not manage to find any official or
+// unofficial reference that clarifies this point. If anyone would like to
+// reimplement this, please also keep in mind to check how this behaves when
+// debugging a WOW64 process. I suspect you may have to use the explicit
+// `EXCEPTION_RECORD32` and `EXCEPTION_RECORD64` structs.
 m_code = record.ExceptionCode;
 m_continuable = (record.ExceptionFlags == 0);
-if (record.ExceptionRecord)
-  m_next_exception.reset(
-  new ExceptionRecord(*record.ExceptionRecord, thread_id));
 m_exception_addr = reinterpret_cast(record.ExceptionAddress);
 m_thread_id = thread_id;
 m_arguments.assign(record.ExceptionInformation,
@@ -39,27 +45,16 @@
   // MINIDUMP_EXCEPTIONs are almost identical to EXCEPTION_RECORDs.
   ExceptionRecord(const MINIDUMP_EXCEPTION &record, lldb::tid_t thread_id)
   : m_code(record.ExceptionCode), m_continuable(record.ExceptionFlags == 0),
-m_next_exception(nullptr),
 m_exception_addr(static_cast(record.ExceptionAd

[Lldb-commits] [PATCH] D127234: [lldb] Add setting to override PE/COFF ABI by module name

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c867898c7be: [lldb] Add setting to override PE/COFF ABI by 
module name (authored by alvinhochun, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127234

Files:
  lldb/include/lldb/Interpreter/OptionValueDictionary.h
  lldb/source/Interpreter/OptionValueDictionary.cpp
  lldb/source/Interpreter/Property.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
@@ -1,4 +1,8 @@
 # RUN: yaml2obj %s -o %t
+# RUN: yaml2obj %s -o %t.debug
+# RUN: mkdir -p %t.dir
+# RUN: yaml2obj %s -o %t.dir/UPPER_CASE
+# RUN: yaml2obj %s -o %t.dir/UPPER_CASE.debug
 
 ## Default ABI is msvc:
 # RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
@@ -10,6 +14,57 @@
 # RUN:   -f %t -o "image list --triple --basename" -o exit | \
 # RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
 
+## Default ABI is msvc, module override is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu, module override is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is msvc, module override is gnu (with .debug suffix):
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=gnu" \
+# RUN:   -f %t.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp.debug %s
+
+## Default ABI is gnu, module override is msvc (with .debug suffix):
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=msvc" \
+# RUN:   -f %t.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp.debug %s
+
+## Check that case-sensitive match is chosen before lower-case:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case=msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi UPPER_CASE=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE %s
+
+## Check that lower-case match with .debug suffix is chosen before case-sensitive match without .debug suffix:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi UPPER_CASE=msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case.debug=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE.debug %s
+
+## Check that case-sensitive match without .debug suffix is chosen before lower-case match without .debug suffix:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case.debug=msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi UPPER_CASE.debug=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE.debug %s
+
+## Check that lower-case match without .debug suffix works:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case.debug=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE.debug %s
+
 # CHECK-LABEL: image list --triple --basename
 # CHECK-NEXT: x86_64-pc-windows-[[ABI]] [[FILENAME]]
 
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFPrope

[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D127436#3602224 , 
@stella.stamenova wrote:

> This broke the Windows LLDB bot:
>
> https://lab.llvm.org/buildbot/#/builders/83/builds/20295/steps/7/logs/stdio

Yep, noted. It worked for me in my MSVC build configuration - I'll investigate 
a bit more and either push another attempt at fixing it, or revert, in a little 
while.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

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

[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D127436#3602224 , 
@stella.stamenova wrote:

> This broke the Windows LLDB bot:
>
> https://lab.llvm.org/buildbot/#/builders/83/builds/20295/steps/7/logs/stdio

The second fix attempt, in 
https://github.com/llvm/llvm-project/commit/a1ee0b947d46c9be1cc2ea8db21603bac84efb18,
 seems to have fixed this test now - however the latest test run seems to have 
some other failures: https://lab.llvm.org/buildbot/#/builders/83/builds/20304 
(Not sure if these are spurious failures or if they are other regressions that 
happened meanwhile)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

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

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added subscribers: stella.stamenova, mstorsjo.
mstorsjo added a comment.

I think this might have broken a bunch of testcases on the 
lldb-x64-windows-ninja buildbot too, e.g. 
https://lab.llvm.org/buildbot/#/builders/83/builds/20304 (CC @stella.stamenova)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126513

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D128268#3603931 , @labath wrote:

>> As a separate path forward, one could also consider to stop returning
>> two architecture specs from ObjectFilePECOFF::GetModuleSpecifications
>> for i386 files.
>
> I think that would be much more preferable. I think the current 
> implementation GetModuleSpecifications is based on a misconception about what 
> the multiple results of that function mean (a fat binary -- as you've 
> discovered).
>
> We wouldn't want to return armv6-*-* (or armv7, v8, ...) for a file that 
> claims it can be run on armv5. Elf files also return just a single result for 
> i386 file.

Yep, that's probably a good path forward for the future - I was just weary of 
that possibly opening another huge can of worms. And regardless of that, I 
think the functional change of this patch, allowing considering various 
variations of triples that commonly occur on windows as compatible, is sensible 
on its own (such issues have caused unnecessary tweaking back and forth once or 
twice before too).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128410: [lldb] Add a testcase for nested exceptions on Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, alvinhochun, DavidSpickett.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

This adds a testcase for
4d123783957e547009e55346bf3a8ae43a88fa14 
 / D128201 
. Before that commit,
lldb would crash here, while now lldb manages to stop after the
exception.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128410

Files:
  lldb/test/Shell/Process/Windows/wndproc_exception.cpp


Index: lldb/test/Shell/Process/Windows/wndproc_exception.cpp
===
--- /dev/null
+++ lldb/test/Shell/Process/Windows/wndproc_exception.cpp
@@ -0,0 +1,54 @@
+// clang-format off
+
+// Check that lldb doesn't crash when there's a nested exception.
+
+// REQUIRES: system-windows
+// RUN: %clangxx_host -o %t.exe -luser32 -v -- %s
+// RUN: %lldb -f %t.exe -o "run"
+
+#include 
+
+static LRESULT CALLBACK WindowProc(HWND hwnd, UINT uMsg, WPARAM wParam,
+   LPARAM lParam) {
+  switch (uMsg) {
+  case WM_DESTROY:
+PostQuitMessage(0);
+return 0;
+
+  case WM_PAINT:
+*(volatile int *)nullptr = 1;
+return 0;
+  }
+  return DefWindowProcA(hwnd, uMsg, wParam, lParam);
+}
+
+int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR 
pCmdLine,
+   int nCmdShow) {
+  const char CLASS_NAME[] = "Crash Window Class";
+
+  WNDCLASSA wc = {};
+
+  wc.lpfnWndProc = WindowProc;
+  wc.hInstance = hInstance;
+  wc.lpszClassName = CLASS_NAME;
+
+  RegisterClassA(&wc);
+
+  HWND hwnd = CreateWindowExA(0, CLASS_NAME, "Crash", WS_OVERLAPPEDWINDOW,
+  CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT,
+  CW_USEDEFAULT,
+  NULL, NULL, hInstance, NULL);
+
+  if (hwnd == NULL)
+return 0;
+
+  ShowWindow(hwnd, nCmdShow);
+
+  MSG msg = {};
+  while (GetMessageA(&msg, NULL, 0, 0) > 0) {
+TranslateMessage(&msg);
+DispatchMessageA(&msg);
+  }
+
+  return 0;
+}


Index: lldb/test/Shell/Process/Windows/wndproc_exception.cpp
===
--- /dev/null
+++ lldb/test/Shell/Process/Windows/wndproc_exception.cpp
@@ -0,0 +1,54 @@
+// clang-format off
+
+// Check that lldb doesn't crash when there's a nested exception.
+
+// REQUIRES: system-windows
+// RUN: %clangxx_host -o %t.exe -luser32 -v -- %s
+// RUN: %lldb -f %t.exe -o "run"
+
+#include 
+
+static LRESULT CALLBACK WindowProc(HWND hwnd, UINT uMsg, WPARAM wParam,
+   LPARAM lParam) {
+  switch (uMsg) {
+  case WM_DESTROY:
+PostQuitMessage(0);
+return 0;
+
+  case WM_PAINT:
+*(volatile int *)nullptr = 1;
+return 0;
+  }
+  return DefWindowProcA(hwnd, uMsg, wParam, lParam);
+}
+
+int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR pCmdLine,
+   int nCmdShow) {
+  const char CLASS_NAME[] = "Crash Window Class";
+
+  WNDCLASSA wc = {};
+
+  wc.lpfnWndProc = WindowProc;
+  wc.hInstance = hInstance;
+  wc.lpszClassName = CLASS_NAME;
+
+  RegisterClassA(&wc);
+
+  HWND hwnd = CreateWindowExA(0, CLASS_NAME, "Crash", WS_OVERLAPPEDWINDOW,
+  CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT,
+  CW_USEDEFAULT,
+  NULL, NULL, hInstance, NULL);
+
+  if (hwnd == NULL)
+return 0;
+
+  ShowWindow(hwnd, nCmdShow);
+
+  MSG msg = {};
+  while (GetMessageA(&msg, NULL, 0, 0) > 0) {
+TranslateMessage(&msg);
+DispatchMessageA(&msg);
+  }
+
+  return 0;
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits