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

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:

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

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/

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

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] [PATCH] D157589: [lldb] Fix building with latest libc++

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/

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

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

[Lldb-commits] [PATCH] D151344: Reland "[CMake] Bumps minimum version to 3.20.0.

mstorsjo added inline comments. Comment at: libunwind/src/CMakeLists.txt:28-35 # See add_asm_sources() in compiler-rt for explanation of this workaround. # CMake doesn't work correctly with assembly on AIX. Workaround by compiling # as C files as well. if((APPLE AND

[Lldb-commits] [PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

mstorsjo added a comment. In D144509#4349051 , @hans wrote: > In D144509#4347562 , @glandium > wrote: > >> FYI, 65429b9af6a2c99d340ab2dcddd41dab201f399c >>

[Lldb-commits] [PATCH] D144509: [CMake] Bumps minimum version to 3.20.0.

mstorsjo added a comment. In D144509#4347562 , @glandium wrote: > FYI, 65429b9af6a2c99d340ab2dcddd41dab201f399c > is > causing problems on Windows compiler-rt for some reason I

[Lldb-commits] [PATCH] D144078: [lldb] Fix a log format warning on Windows, don't assume uint64_t is a long type

This revision was automatically updated to reflect the committed changes. Closed by commit rGa8483b9b3038: [lldb] Fix a log format warning on Windows, dont assume uint64_t is a long type (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D144078: [lldb] Fix a log format warning on Windows, don't assume uint64_t is a long type

mstorsjo created this revision. mstorsjo added reviewers: Emmmer, DavidSpickett, JDevlieghere, labath. Herald added subscribers: luke, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27,

[Lldb-commits] [PATCH] D138259: Add the ability to see when a type in incomplete.

mstorsjo added a comment. This seems to have caused build errors with GCC: ../tools/lldb/source/Symbol/CompilerType.cpp: In member function ‘bool lldb_private::CompilerType::IsForcefullyCompleted() const’: ../tools/lldb/source/Symbol/CompilerType.cpp:99:25: error: base operand of ‘->’ has

[Lldb-commits] [PATCH] D137873: [LLDB][Minidump] Set abi environment for windows.

mstorsjo added a subscriber: alvinhochun. mstorsjo added a comment. In D137873#3930683 , @labath wrote: > If that doesn't work then (besides knowing why), I'd like us try some kind of > a single-setting solution, as I don't think it makes sense to have

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mstorsjo added a comment. In D136572#3890835 , @mgorny wrote: > In D136572#3889317 , @mstorsjo > wrote: > >> Also for the record, I'd love to get rid of this symlink based setup, if I >> could make cmake

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mstorsjo added a comment. Also for the record, I'd love to get rid of this symlink based setup, if I could make cmake produce project files where caching works in the same way. The main requirement for that would be to have the whole cmake build started from the toplevel llvm-project directory

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mstorsjo accepted this revision. mstorsjo added a comment. In D136572#3888630 , @mgorny wrote: > @mstorsjo, could you check this version? LGTM, this seems to work for my usecase (with lld,lldb,clang enabled this way). Thanks for the quick revert and

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mstorsjo added a comment. In D136572#3888177 , @mstorsjo wrote: > This broke builds where clang/lld/lldb are symlinked into `llvm/tools` > instead of specified via `LLVM_ENABLE_PROJECTS` - since > `${CMAKE_CURRENT_SOURCE_DIR}/../cmake` doesn't find

[Lldb-commits] [PATCH] D136572: Harmonize cmake_policy() across standalone builds of all projects

mstorsjo added a comment. Herald added subscribers: JDevlieghere, jdoerfert. This broke builds where clang/lld/lldb are symlinked into `llvm/tools` instead of specified via `LLVM_ENABLE_PROJECTS` - since `${CMAKE_CURRENT_SOURCE_DIR}/../cmake` doesn't find anything in that context. (The reason

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

mstorsjo added a comment. Was the consensus that we'd drop this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134581/new/ https://reviews.llvm.org/D134581 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D134636: [lldb][Windows] Always call SetExecutableModule on debugger connected

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfe17e026959c: [lldb][Windows] Always call SetExecutableModule on debugger connected (authored by alvinhochun, committed by mstorsjo). Repository:

[Lldb-commits] [PATCH] D134585: [lldb][COFF] Map symbols without base+complex type as 'Data' type

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8a67a05e9334: [lldb][COFF] Map symbols without base+complex type as Data type (authored by alvinhochun, committed by mstorsjo). Repository: rG

[Lldb-commits] [PATCH] D134518: [lldb][COFF] Add note to forwarder export symbols in symtab

This revision was automatically updated to reflect the committed changes. Closed by commit rGacf7d081198f: [lldb][COFF] Add note to forwarder export symbols in symtab (authored by alvinhochun, committed by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D134517: [lldb][COFF] Load absolute symbols from COFF symbol table

This revision was automatically updated to reflect the committed changes. Closed by commit rG7ebff6ab26aa: [lldb][COFF] Load absolute symbols from COFF symbol table (authored by alvinhochun, committed by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D134426: [lldb][COFF] Match symbols from COFF symbol table to export symbols

This revision was automatically updated to reflect the committed changes. Closed by commit rG20c2f94c3cc1: [lldb][COFF] Match symbols from COFF symbol table to export symbols (authored by alvinhochun, committed by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D134196: [lldb][COFF] Rewrite ParseSymtab to list both export and symbol tables

This revision was automatically updated to reflect the committed changes. Closed by commit rGbf0cda9ed278: [lldb][COFF] Rewrite ParseSymtab to list both export and symbol tables (authored by alvinhochun, committed by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D134265: [lldb][COFF] Improve info of symbols from export table

This revision was automatically updated to reflect the committed changes. Closed by commit rG0870afc68e1a: [lldb][COFF] Improve info of symbols from export table (authored by alvinhochun, committed by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D134516: [lldb] Improve display of absolute symbol lookup

This revision was automatically updated to reflect the committed changes. Closed by commit rGf4991bfa891e: [lldb] Improve display of absolute symbol lookup (authored by alvinhochun, committed by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D134111: [lldb] Add newline in output of `target modules lookup`

This revision was automatically updated to reflect the committed changes. Closed by commit rGa426753ef029: [lldb] Add newline in output of `target modules lookup` (authored by alvinhochun, committed by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

mstorsjo added a comment. In D134037#3815240 , @DavidSpickett wrote: >> I might add the AArch64 and Arm equivalent if I have some spare time. > > I looked into this and there's a detail that makes this work on x86 only. So > I'll leave it as is, it's

[Lldb-commits] [PATCH] D134636: [lldb][Windows] Always call SetExecutableModule on debugger connected

mstorsjo added a reviewer: jasonmolenda. mstorsjo added a subscriber: jasonmolenda. mstorsjo added a comment. The test runs fine on Windows, but I'm not familiar with these aspects of lldb to (yet) have a good opinion on it; adding @jasonmolenda to the review who commented on the previous one

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

mstorsjo added inline comments. Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7 +# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll \ +# RUN: %if windows-msvc %{-Wl,-implib:%t.shlib.lib} \ +# RUN: %else

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

mstorsjo added inline comments. Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7 +# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll +# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe +# RUN: %lldb -b -o "#before"

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

mstorsjo added inline comments. Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7 +# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll +# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe +# RUN: %lldb -b -o "#before"

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

mstorsjo added inline comments. Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7 +# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll +# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe +# RUN: %lldb -b -o "#before"

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

mstorsjo added inline comments. Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7 +# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll +# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe +# RUN: %lldb -b -o "#before"

[Lldb-commits] [PATCH] D134585: [lldb][COFF] Map symbols without base+complex type as 'Data' type

mstorsjo added a comment. In D134585#3814463 , @DavidSpickett wrote: >> MSVC/link.exe doesn't write symbols into linked PE images at all. > > So by the time we get to a debugger, it's not an issue anyway. Yep, most of these patches about symbol table

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

mstorsjo added inline comments. Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7 +# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll +# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe +# RUN: %lldb -b -o "#before"

[Lldb-commits] [PATCH] D134585: [lldb][COFF] Map symbols without base+complex type as 'Data' type

mstorsjo added a comment. In D134585#3814458 , @DavidSpickett wrote: > You're the expert on what the linker does and the code looks fine. > > Is it possible that msvc uses these `IMAGE_SYM_TYPE_NULL` in a different way > that could cause a problem?

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

This revision was automatically updated to reflect the committed changes. Closed by commit rG8a3597d73c8f: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads (authored by mstorsjo). Changed prior to commit: https://reviews.llvm.org/D134037?vs=462454=462836#toc

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

mstorsjo added inline comments. Comment at: lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test:1 +# XFAIL: system-windows +# REQUIRES: native && (target-x86 || target-x86_64) DavidSpickett wrote: > mstorsjo wrote: > > DavidSpickett wrote: > > > Isn't this

[Lldb-commits] [PATCH] D134585: [lldb][COFF] Map symbols without base+complex type as 'Data' type

mstorsjo added a comment. This LGTM, but I'll leave it to some LLDB maintainer to formally approve. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134585/new/ https://reviews.llvm.org/D134585 ___

[Lldb-commits] [PATCH] D134426: [lldb][COFF] Match symbols from COFF symbol table to export symbols

mstorsjo added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:828 + // either names will work. Only synchronize the symbol type. + if (symbol.GetType() == lldb::eSymbolTypeInvalid) +

[Lldb-commits] [PATCH] D134426: [lldb][COFF] Match symbols from COFF symbol table to export symbols

mstorsjo added a comment. No further objections/comments from me on this! Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:828 + // either names will work. Only synchronize the symbol type. + if (symbol.GetType() ==

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

mstorsjo updated this revision to Diff 462454. mstorsjo added a comment. Add a code comment about the code that is changed in the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134037/new/ https://reviews.llvm.org/D134037 Files:

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

mstorsjo added inline comments. Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2443 StopInfoSP stop_info = thread_sp->GetStopInfo(); if (!stop_info) + continue; DavidSpickett wrote: > Please add a comment explaining why we walk all the

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

mstorsjo added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134037/new/ https://reviews.llvm.org/D134037 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D134426: [lldb][COFF] Match symbols from COFF symbol table to export symbols

mstorsjo added a comment. Herald added a subscriber: JDevlieghere. This looks mostly reasonable to me, a couple discussion points only. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:817 +if (symbol_type != lldb::eSymbolTypeInvalid) +

[Lldb-commits] [PATCH] D134518: [lldb][COFF] Skip forwarder export symbols in symtab

mstorsjo added a comment. In D134518#3811153 , @labath wrote: > They may not be useful (at the moment), but if they're not actively causing > harm (e.g. stopping some other feature from functioning, or if we're badly > misrepresenting them in the

[Lldb-commits] [PATCH] D134196: [lldb][COFF] Rewrite ParseSymtab to list both export and symbol tables

mstorsjo added a comment. This looks good to me, nothing to add from my PoV. Nice to have this condensed to a small enough rewrite, with few enough functional changes to wrap one's head around. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D134265: [lldb][COFF] Improve info of symbols from export table

mstorsjo added a comment. Looks good to me in general. Do any of the changes make a functional difference, or is it just improved (and less misleading) display to the user of the debugger? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134265/new/

[Lldb-commits] [PATCH] D134133: WIP: [lldb][COFF] Enhance symtab loading of symbol and export tables

mstorsjo added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:787 + Symbol *symbols = symtab.Extend(num_syms); + uint32_t i = 0; + for (const auto _ref : m_binary->symbols()) { alvinhochun wrote: > mstorsjo wrote: > >

[Lldb-commits] [PATCH] D134133: WIP: [lldb][COFF] Enhance symtab loading of symbol and export tables

mstorsjo added a comment. So previously, LLDB essentially used the COFF symbol table for executables, but only the list of exported symbols for DLLs, ignoring (or, reading and then overwriting) the symbol table for any DLL with exports? Then this certainly does look like a good direction.

[Lldb-commits] [PATCH] D130796: [LLDB][NativePDB] Switch to use DWARFLocationList.

mstorsjo added a comment. FYI, this change caused a strange regression for reading PDB files in i386 mode, when LLDB is built in mingw mode (not when it is built in MSVC mode), see https://github.com/llvm/llvm-project/issues/57799 for details. Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

mstorsjo updated this revision to Diff 460880. mstorsjo added a comment. Added a testcase. I'm not sure if this is the most accurate/correct place for the test, but the name is at least verbose enough to describe what it exercises. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

mstorsjo added a comment. In D134037#3796251 , @jingham wrote: > We never made any guarantee about the order threads would be listed in the > array returned by GetThreadList. I'm not sure it would be worth trying to do > that, because often you don't

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

mstorsjo created this revision. mstorsjo added reviewers: labath, DavidSpickett. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: LLDB. If a process has multiple threads, the thread with the stop info might not be the first one in the thread list.

[Lldb-commits] [PATCH] D133045: Partial fix for handling backticks in commands and aliases

mstorsjo added a comment. This seems to break the build with GCC (tested with the system compiler GCC 9 on Ubuntu 20.04): ld.lld: error: undefined symbol: lldb_private::CommandInterpreter::g_argument >>> referenced by CommandAlias.cpp >>>

[Lldb-commits] [PATCH] D133002: [LLDB] Make API tests to run using MSYS tools

mstorsjo accepted this revision. mstorsjo added a comment. This revision is now accepted and ready to land. LGTM, this looks reasonable to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133002/new/ https://reviews.llvm.org/D133002

[Lldb-commits] [PATCH] D132841: [lldb] Use the NativeSock type instead of plain 'int'

This revision was automatically updated to reflect the committed changes. Closed by commit rG118038e878cf: [lldb] Use the NativeSock type instead of plain int (authored by mstorsjo). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132841/new/

[Lldb-commits] [PATCH] D132841: [lldb] Use the NativeSock type instead of plain 'int'

mstorsjo created this revision. mstorsjo added reviewers: labath, fixathon, jasonmolenda, clayborg, JDevlieghere, DavidSpickett. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: LLDB. This fixes a warning when building for Windows:

[Lldb-commits] [PATCH] D131983: [LLDB][NFC] Fix optons parsing and misc. reliability in CommandObjectThread

mstorsjo added a comment. This commit caused warnings when building with GCC: ../tools/lldb/source/Commands/CommandObjectThread.cpp: In member function ‘virtual lldb_private::Status CommandObjectThreadBacktrace::CommandOptions::SetOptionValue(uint32_t, llvm::StringRef,

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

mstorsjo added a comment. In D131159#3742370 , @labath wrote: > In D131159#3742364 , @mgorny wrote: > >> In D131159#3742235 , @mstorsjo >> wrote: >> >>> (They're only

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

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'

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

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:

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

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

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

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?)

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

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

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

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

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

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:

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

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

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

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] [PATCH] D129377: [lldb/Fuzzer] Add fuzzer for expression evaluator

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

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

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

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

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:

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

mstorsjo added a comment. In D130309#3673045 , @clayborg wrote: > Another fix for linux: > > commit 0bbce7a4c2d2bff622bdadd4323f93f5d90e6d24 > (HEAD > -> main, origin/main,

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

mstorsjo added a comment. In D130309#3672934 , @clayborg wrote: > Submitted a fix for the buildbots: > > commit f959d815f4637890ebbacca379f1c38ab47e4e14 > (HEAD > -> main) > Author:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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"));`

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

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

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

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

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

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

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

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

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

mstorsjo added a comment. In D128268#3608197 , @labath wrote: > In D128268#3604555 , @mstorsjo > wrote: > >> I found that this duality was introduced in >> 5e6f45201f0b62c1e7a24fc396f3ea6e10dc880d / D7120 >>

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

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`

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

mstorsjo added a subscriber: zturner. mstorsjo added a comment. In D128268#3604081 , @labath wrote: >> If we'd just set this to the baseline, i386, would that have any effect for >> how lldb e.g. is able to disassemble/interpret instructions that don't

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

mstorsjo added a comment. In D128410#3604083 , @mstorsjo wrote: > I'm not all that familiar with exactly what happens in these APIs here that > makes this a nested exception and what other non-GUI APIs would produce the > same effect. Maybe any API

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

mstorsjo added inline comments. Comment at: lldb/test/Shell/Process/Windows/wndproc_exception.cpp:7 +// RUN: %clangxx_host -o %t.exe -luser32 -v -- %s +// RUN: %lldb -f %t.exe -o "run" + mstorsjo wrote: > labath wrote: > > Is there something reasonable we could

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

mstorsjo added a subscriber: stella.stamenova. mstorsjo added a comment. In D128410#3604075 , @labath wrote: > In D128410#3604066 , @mstorsjo > wrote: > >> For this testcase to work, it needs to be able to open

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

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

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

mstorsjo added a comment. Herald added a subscriber: JDevlieghere. For this testcase to work, it needs to be able to open a window - so it can't run entirely in headless mode. But I guess that a bunch of other tests in LLDB also already do that, since running the LLDB tests on Windows pops up a

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

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

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

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

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

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

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

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

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

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 -

  1   2   3   4   5   >