[Lldb-commits] [PATCH] D147669: PECOFF: enforce move semantics and consume errors properly

alvinhochun added a comment. Thanks for adding me in the loop. And thanks for looking into the fix, though this bit is a bit confusing to me: > Ensure that we explicitly indicate that we would like the move semantics > in the assignment. With MSVC 14.35.32215, the assignment would not > trigger

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

alvinhochun added a comment. I think the module spec of the modules should reflect the value of the `plugin.object-file.pe-coff.abi` setting. Can ProcessMinidump take the module spec of the executable module to use its target environment? Mixing MSVC and MinGW DLLs is a tricky case so I will no

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

alvinhochun abandoned this revision. alvinhochun added a comment. Since the original issue was fixed in a different patch I don't think I will follow up on this anyway. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134581/new/ https://reviews.llvm

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

alvinhochun abandoned this revision. alvinhochun added a comment. Handled in separate changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134133/new/ https://reviews.llvm.org/D134133 ___ lldb-commits

[Lldb-commits] [PATCH] D134751: [lldb] Fix running tests when LLVM target triple is different from host

alvinhochun added a comment. Thanks for the review. In D134751#3818743 , @JDevlieghere wrote: > Supporting builds with different host and target triples makes sense. > However, I have a few concerns about the current patch: > > - I wouldn't cal it the

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

alvinhochun added inline comments. Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:6-9 +# 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] D134636: [lldb][Windows] Always call SetExecutableModule on debugger connected

alvinhochun updated this revision to Diff 463459. alvinhochun edited the summary of this revision. alvinhochun added a comment. Actually update commit message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134636/new/ https://reviews.llvm.org/D13463

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

alvinhochun updated this revision to Diff 463458. alvinhochun added a comment. Updated the patch to be standalone. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134636/new/ https://reviews.llvm.org/D134636 Files: lldb/source/Plugins/Process/Wind

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

alvinhochun added a comment. Okay, I'll rebase the change to remove the dependency on D134581 . Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:21-22 # CHECK-NEXT: .main.exe -# CHECK-NEXT: .shlib.dll +# CHECK-NEXT: nt

[Lldb-commits] [PATCH] D134751: [lldb] Fix running tests when LLVM target triple is different from host

alvinhochun created this revision. alvinhochun added reviewers: labath, JDevlieghere, DavidSpickett, mstorsjo. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This passes the host triple to the lldbs

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

alvinhochun added a comment. In D134581#3815766 , @jasonmolenda wrote: > In D134581#3813757 , @alvinhochun > wrote: > >> Thanks for the comment. >> >> In D134581#3813484

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

alvinhochun added a comment. In D134581#3813757 , @alvinhochun wrote: > In D134581#3813484 , @jasonmolenda > wrote: > >> I probably misunderstand the situation DynamicLoaderWindows finds itself in, >> but in Dy

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

alvinhochun created this revision. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. In `ProcessWindows::OnDebuggerConnected` (triggered from `CREATE_PROCESS_DEBUG_EVENT`), we should always call `Targe

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

alvinhochun updated this revision to Diff 462860. alvinhochun added a comment. Fixed test again Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134581/new/ https://reviews.llvm.org/D134581 Files: lldb/source/Target/Target.cpp lldb/test/Shell/Tar

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

alvinhochun updated this revision to Diff 462858. alvinhochun added a comment. Updated test, thanks to @mstorsjo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134581/new/ https://reviews.llvm.org/D134581 Files: lldb/source/Target/Target.cpp ll

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

alvinhochun 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 "#befor

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

alvinhochun 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 "#befor

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

alvinhochun 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 "#befor

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

alvinhochun updated this revision to Diff 462745. alvinhochun added a comment. Updated condition for early return Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134581/new/ https://reviews.llvm.org/D134581 Files: lldb/source/Target/Target.cpp l

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

alvinhochun added a reviewer: DavidSpickett. alvinhochun added a comment. Thanks for the comment. In D134581#3813484 , @jasonmolenda wrote: > I probably misunderstand the situation DynamicLoaderWindows finds itself in, > but in DynamicLoaderDarwin we h

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

alvinhochun created this revision. alvinhochun added reviewers: labath, DavidSpickett, mstorsjo. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Both LLD and GNU ld write global/static variables to t

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

alvinhochun added a comment. @mstorsjo Can you please try the test I added and check that it passes with the patch and fails without it? (I don't have a setup to run tests on Windows.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134581/new/ http

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

alvinhochun created this revision. alvinhochun added reviewers: labath, mstorsjo, clayborg, jingham. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. LLDB preloads some dependent modules when creating

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

alvinhochun 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) +symbol.SetType(exported-

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

alvinhochun updated this revision to Diff 462513. alvinhochun retitled this revision from "[lldb][COFF] Skip forwarder export symbols in symtab" to "[lldb][COFF] Add note to forwarder export symbols in symtab". alvinhochun edited the summary of this revision. alvinhochun added a comment. Changed

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

alvinhochun updated this revision to Diff 462479. alvinhochun added a comment. Rebased on parent Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134517/new/ https://reviews.llvm.org/D134517 Files: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePE

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

alvinhochun updated this revision to Diff 462476. alvinhochun added a comment. Updated test based on parent and addressed review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134426/new/ https://reviews.llvm.org/D134426 Files: lldb/sou

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

alvinhochun updated this revision to Diff 462472. alvinhochun added a comment. Updated to new test from parent Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134265/new/ https://reviews.llvm.org/D134265 Files: lldb/source/Plugins/ObjectFile/PECOF

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

alvinhochun updated this revision to Diff 462471. alvinhochun added a comment. Updated the test to include more symbols used in later changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134196/new/ https://reviews.llvm.org/D134196 Files: lldb/

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

alvinhochun added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:817 +if (symbol_type != lldb::eSymbolTypeInvalid) + exported->SetType(symbol_type); +if (exported->GetMangled() == symbol.GetMangled()) { ---

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

alvinhochun added a comment. In D134518#3811218 , @labath wrote: > Ok, so is there any harm in keeping them this way? > > The symbols may not be very useful, but I would not say that they are > useless. If, for whatever reason, the user finds himself ins

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

alvinhochun added a comment. In D134518#3811155 , @mstorsjo wrote: > In D134518#3811153 , @labath wrote: > >> They may not be useful (at the moment), but if they're not actively causing >> harm (e.g. stopping som

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

alvinhochun added inline comments. Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1552 strm.IndentMore(); + strm.Indent("Name: "); + strm.PutCString(symbol->GetDisplayName().GetStringRef()); DavidSpickett wrote: > Could

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

alvinhochun created this revision. alvinhochun added reviewers: labath, DavidSpickett, mstorsjo. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Forwarder exports do not point to a real function or v

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

alvinhochun created this revision. alvinhochun added reviewers: labath, DavidSpickett, mstorsjo. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Depends on D134426

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

alvinhochun created this revision. alvinhochun added reviewers: labath, clayborg. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. When running `target module lookup` command, show the name of absolut

[Lldb-commits] [PATCH] D131705: Don't create sections for SHN_ABS symbols in ELF files.

alvinhochun added inline comments. Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1551-1556 + strm.Indent("Value: "); + strm.Printf("0x%16.16" PRIx64 "\n", symbol->GetRawValue()); + if (symbol->GetByteSizeIsValid()) { +strm.Ind

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

alvinhochun created this revision. alvinhochun added reviewers: labath, DavidSpickett, mstorsjo. Herald added a subscriber: mgrang. Herald added a project: All. alvinhochun published this revision for review. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. If a symbol is the

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

alvinhochun added a comment. I think marking the symbol as Data instead of Code does have an effect on commands that look for functions (e.g. "disassemble" will not disassemble a Data symbol, which makes sense). For the rest of the changes I think they are only visible in the symtab dump. (Sor

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

alvinhochun added a comment. I think marking the symbol as Data instead of Code does have an effect on commands that look for functions (e.g. "disassemble" will not disassemble a Code symbol, which makes sense). For the rest of the changes I think they are only visible in the symtab dump. Rep

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

alvinhochun marked 2 inline comments as done. alvinhochun added a comment. Thanks for reviewing. The COFF symbol table seems to be used by MinGW binaries in conjunction with DWARF debugging symbols. Some symbols (those added by the linker in particular) are not included in the DWARF symbol. I un

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

alvinhochun created this revision. alvinhochun added reviewers: labath, DavidSpickett, mstorsjo. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. - Skip dummy/invalid export symbols. - Make the export

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

alvinhochun updated this revision to Diff 461496. alvinhochun added a comment. Updated test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134196/new/ https://reviews.llvm.org/D134196 Files: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.

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

alvinhochun updated this revision to Diff 461486. alvinhochun added a comment. Use --implicit-check-not in test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134111/new/ https://reviews.llvm.org/D134111 Files: lldb/source/Commands/CommandObjectT

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

alvinhochun added a comment. Separated the first part with some new changes here: https://reviews.llvm.org/D134196 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134133/new/ https://reviews.llvm.org/D134133

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

alvinhochun created this revision. alvinhochun added reviewers: labath, DavidSpickett, mstorsjo. Herald added a project: All. alvinhochun requested review of this revision. Herald added projects: LLDB, LLVM. Herald added subscribers: llvm-commits, lldb-commits. This reimplements `ObjectFilePECOFF:

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

alvinhochun added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:819 + if (exported->GetType() != lldb::eSymbolTypeReExported && + exported->GetAddressRef() == symbols[i].GetAddressRef()) { +symbols[i].SetID(exported-

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

alvinhochun added a comment. Thanks for the review. Yes I shall split the changes into smaller pieces to aid review. Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1558 } + strm.IndentLess(); } mstorsjo wrote: > Looks lik

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

alvinhochun created this revision. alvinhochun added reviewers: labath, DavidSpickett, mstorsjo. Herald added a project: All. alvinhochun requested review of this revision. Herald added projects: LLDB, LLVM. Herald added subscribers: llvm-commits, lldb-commits. This reimplements `ObjectFilePECOFF:

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

alvinhochun created this revision. alvinhochun added a reviewer: labath. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This adds a line break between each result address in the output of the lldb c

[Lldb-commits] [PATCH] D131705: Don't create sections for SHN_ABS symbols in ELF files.

alvinhochun added inline comments. Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1550 +} else { + strm.IndentMore(); + strm.Indent("Value: "); This `IndentMore` added is missing a matching `IndentLess` call.

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

alvinhochun added a comment. I gave this a spin with `LLDB_USE_LLDB_SERVER=1`, but there looks to be some pre-existing issues with the lldb-server implementation so I can't really test much normally using builds of Krita: - Manually set breakpoints don't work (they stay unresolved?) - Debugger

[Lldb-commits] [PATCH] D128541: [WIP][lldb][windows] Handle OutputDebugString from debuggee

alvinhochun added a comment. In D128541#3621364 , @labath wrote: > Well.. most OSes don't have this kind of functionality, so we don't really > have an exact match for this. I suppose the closest thing would be the way we > handle darwin OS logs, so you

[Lldb-commits] [PATCH] D128541: [WIP][lldb][windows] Handle OutputDebugString from debuggee

alvinhochun created this revision. Herald added a project: All. alvinhochun added reviewers: mstorsjo, DavidSpickett, labath, omjavaid. alvinhochun added a comment. alvinhochun updated this revision to Diff 439961. alvinhochun updated this revision to Diff 441032. alvinhochun edited the summary of

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

alvinhochun 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" + alvinhochun wrote: > mstorsjo wrote: > > mstorsjo wrote: > > > labath wro

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

alvinhochun 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: > mstorsjo wrote: > > labath wrote: > > > Is there someth

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

alvinhochun added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

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

alvinhochun updated this revision to Diff 438737. alvinhochun added a comment. Added Windows-specific test 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

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

alvinhochun updated this revision to Diff 438734. alvinhochun added a comment. Added test and improved setting description as suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127234/new/ https://reviews.llvm.org/D127234 Files: lldb/inclu

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

alvinhochun added a comment. Yes, this looks right to me. 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.

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

alvinhochun updated this revision to Diff 438430. alvinhochun edited the summary of this revision. alvinhochun added a comment. Remove old code instead of `#if 0`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128201/new/ https://reviews.llvm.org/D

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

alvinhochun added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td:13 +EnumValues<"OptionEnumValues(g_abi_enums)">, +Desc<"A mapping of ABI override to use for specific modules. The module name is matched by its file name

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

alvinhochun added a comment. > Any reason to `#if 0` this instead of just removing it and maybe adding a one > line comment like "nested exceptions are not supported"? So that someone can > git blame that and find this commit with the removal. I want to leave a clear note explaining what not to

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

alvinhochun created this revision. Herald added a subscriber: mstorsjo. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. LLDB tries to follow `EXCEPTION_RECORD::ExceptionRecord` to follow the nested e

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

alvinhochun updated this revision to Diff 438133. alvinhochun added a comment. Herald added a subscriber: Michael137. Rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127234/new/ https://reviews.llvm.org/D127234 Files: lldb/include/lldb/Int

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

alvinhochun added a comment. Herald added a subscriber: Michael137. In D126367#3580683 , @labath wrote: > Is there any difference in functionality between SymbolVendorELF and the new > class introduced here? Could this have been achieved by teaching > S

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

alvinhochun added a comment. Does this test look good: diff diff --git a/lldb/test/Shell/Commands/command-target-create-resolve-exe.test b/lldb/test/Shell/Commands/command-target-create-resolve-exe.test new file mode 100644 index ..3dfa7f0d9853 --- /dev/null +++ b/lldb/te

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

alvinhochun added a comment. I suppose it is possible to have a Windows-only test for this. (Someone will need to verify them for me though as I don't have a build that runs tests on Windows.) This doesn't currently work with a path without the suffix though (like `C:\Windows\System32\notepad`

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

alvinhochun created this revision. alvinhochun added reviewers: JDevlieghere, labath, mstorsjo. Herald added subscribers: jsji, pengfei. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This fixes an

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

alvinhochun updated this revision to Diff 435605. alvinhochun added a comment. Rebased patch 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-commits] [PATCH] D127048: [lldb] Set COFF module ABI from default triple and make it an option

alvinhochun updated this revision to Diff 435604. alvinhochun added a comment. Rebased patch 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/C

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

alvinhochun added a comment. Ah I did not realize that GNU ld also has the option to add the same build id for mingw. I am fine with you committing these changes. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126367/new/ https://reviews.ll

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

alvinhochun updated this revision to Diff 435214. alvinhochun edited the summary of this revision. alvinhochun added a comment. This revision is now accepted and ready to land. Added the fix to the Minidump find-module test, also updated the commit message. Repository: rG LLVM Github Monorepo

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

alvinhochun added a comment. Thanks @mstorsjo for answering for me. >> 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 >

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

alvinhochun created this revision. Herald added a subscriber: mstorsjo. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The setting `plugin.object-file.pe-coff.module-abi` is a string-to-enum map tha

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

alvinhochun updated this revision to Diff 434875. alvinhochun retitled this revision from "[lldb] Set COFF and PDB module env from default target triple" to "[lldb] Set COFF module ABI from default triple and make it an option". alvinhochun edited the summary of this revision. alvinhochun added a

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

alvinhochun added a comment. The test https://github.com/llvm/llvm-project/blob/main/lldb/test/Shell/Minidump/Windows/find-module.test fails with this patch. That test opens a minidump and checks that the associated exe is loaded. The minidump contains a codeview PDB record for the exe which i

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

alvinhochun planned changes to this revision. alvinhochun added a comment. Sorry, I just realized the crc change broke a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126367/new/ https://reviews.llvm.org/D126367 _

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

alvinhochun updated this revision to Diff 434800. alvinhochun added a comment. Applied suggested change 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/Objec

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

alvinhochun added a comment. Thanks for the hint. I think I can add a setting, say `plugin.object-file.pe-coff.abi`. It needs some plumbing but should be doable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127048/new/ https://reviews.llvm.org/D1

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

alvinhochun updated this revision to Diff 434568. alvinhochun edited the summary of this revision. alvinhochun added a comment. Addressed comments, added CRC checking for debuglink (only applies if the object does not contain a PDB build id) and added tests for this. Repository: rG LLVM Githu

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

alvinhochun added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:883 + std::string gnu_debuglink_file = data.GetCStr(&gnu_debuglink_offset); + gnu_debuglink_offset = llvm::alignTo(gnu_debuglink_offset, 4); + data.GetU32(&gnu

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

alvinhochun planned changes to this revision. alvinhochun added a comment. In D126367#3560233 , @DavidSpickett wrote: > Can you include some links to documentation of the debuglink feature in the > commit message? I assume > https://sourceware.org/gdb/

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

alvinhochun added a comment. In D127048#3559918 , @mstorsjo wrote: > In D127048#3559866 , @DavidSpickett > wrote: > >>> This changes the PE/COFF and PDB plugins to set the module triple according >>> to the defa

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

alvinhochun updated this revision to Diff 434321. alvinhochun added a comment. Fixed tests again Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127048/new/ https://reviews.llvm.org/D127048 Files: lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.c

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

alvinhochun updated this revision to Diff 434319. alvinhochun added a comment. Changed test requirement to be more accurate Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127048/new/ https://reviews.llvm.org/D127048 Files: lldb/source/Plugins/Obj

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

alvinhochun updated this revision to Diff 434317. alvinhochun added a comment. Fixed tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127048/new/ https://reviews.llvm.org/D127048 Files: lldb/source/Plugins/ObjectFile/PDB/ObjectFilePDB.cpp l

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

alvinhochun created this revision. Herald added a subscriber: mstorsjo. Herald added a project: All. alvinhochun updated this revision to Diff 434316. alvinhochun added a comment. alvinhochun published this revision for review. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.

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

alvinhochun updated this revision to Diff 434309. alvinhochun added a comment. Debug-- 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-commits] [PATCH] D126367: [lldb] Add gnu-debuglink support for Windows PE/COFF

alvinhochun updated this revision to Diff 434308. alvinhochun added a comment. Fixed loading gnu-debuglink for x86 Windows and added a test for it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126367/new/ https://reviews.llvm.org/D126367 Files:

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

alvinhochun added inline comments. Comment at: lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp:119 + }; + for (SectionType section_type : g_sections) { +if (SectionSP section_sp = mstorsjo wrote: > alvinhochun wrote: > > mstorsjo wrote: > > >

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

alvinhochun added inline comments. Comment at: lldb/source/Plugins/SymbolVendor/PECOFF/SymbolVendorPECOFF.cpp:119 + }; + for (SectionType section_type : g_sections) { +if (SectionSP section_sp = mstorsjo wrote: > I'm curious - this adds new logic (copied fr

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

alvinhochun 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( mstorsjo wrote: > Do you need a check against `NULL` he

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

alvinhochun created this revision. Herald added a subscriber: mstorsjo. Herald added a project: All. alvinhochun requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. The WinAPI `GetFinalPathNameByHandle` is used to retrieve the DLL file name fro

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

alvinhochun updated this revision to Diff 432725. alvinhochun added a comment. Changed `GetPluginNameStatic` to "PE-COFF", and rebased onto `main`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126367/new/ https://reviews.llvm.org/D126367 Files: