[Lldb-commits] [PATCH] D134570: [lldb] Skip check for conflicting filter/synth when adding a new regex.

2022-09-23 Thread Jorge Gorbe Moya via Phabricator via lldb-commits
jgorbe updated this revision to Diff 462637. jgorbe added a comment. Added a couple of test cases to TestDataFormatterPythonSynth.py. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134570/new/ https://reviews.llvm.org/D134570 Files: lldb/source/Commands/CommandObjectType.cpp lldb/t

[Lldb-commits] [PATCH] D134574: [lldb][test] 4 - Add gmodules test category explicitly where previously done implicitly

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 462622. Michael137 added a comment. - Add decorator to another test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134574/new/ https://reviews.llvm.org/D134574 Files: lldb/test/API/lang/cpp/gmodules/templa

[Lldb-commits] [PATCH] D134524: [lldb][test] 1 - Don't do test-replication for gmodules debug_info variant

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 462621. Michael137 added a comment. - Fixup Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134524/new/ https://reviews.llvm.org/D134524 Files: lldb/docs/resources/test.rst lldb/packages/Python/lldbsuite/

[Lldb-commits] [PATCH] D134572: [lldb][test] 2 - Allow multiple precompiled headers in API tests via PCH_CXX_SOURCE

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 abandoned this revision. Michael137 added a comment. Actually realised this isn't necessary...abandoning Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134572/new/ https://reviews.llvm.org/D134572

[Lldb-commits] [PATCH] D134573: [lldb][test] 3 - Add gmodules XFAIL test for lang/cpp/gmodules/templates

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 462618. Michael137 added a comment. - Fix breakpoint and move docstring Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134573/new/ https://reviews.llvm.org/D134573 Files: lldb/test/API/lang/cpp/gmodules/te

[Lldb-commits] [PATCH] D134574: [lldb][test] 4 - Add gmodules test category explicitly where previously done implicitly

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 created this revision. Michael137 added a reviewer: aprantl. Herald added a project: All. Michael137 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Since we don't compile with `gmodules` implicitly via debug-info test replicatio

[Lldb-commits] [PATCH] D134573: [lldb][test] 3 - Add gmodules XFAIL test for lang/cpp/gmodules/templates

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 created this revision. Michael137 added a reviewer: aprantl. Herald added a project: All. Michael137 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Since we don't implicitly compile tests with `gmodules` now, add a `gmodules` te

[Lldb-commits] [PATCH] D134572: [lldb][test] 2 - Allow multiple precompiled headers in API tests via PCH_CXX_SOURCE

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 created this revision. Michael137 added a reviewer: aprantl. Herald added a project: All. Michael137 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Currently we only allow a single PCH file to be specified in an API test since C

[Lldb-commits] [PATCH] D134344: [WIP][lldb][test] 1 - Remove gmodules debug_info variant: add decorator for API tests that explicitly test gmodules

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added a comment. In D134344#3811143 , @labath wrote: > In D134344#3805953 , @Michael137 > wrote: > >> that would require an audit of each API test right? > > Not really. I think this could be a purely

[Lldb-commits] [PATCH] D134524: [WIP][lldb][test] 1 - Remove gmodules debug_info variant: add decorator for API tests that explicitly test gmodules

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 462609. Michael137 added a comment. - Reword commit - Add docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134524/new/ https://reviews.llvm.org/D134524 Files: lldb/docs/resources/test.rst lldb/package

[Lldb-commits] [PATCH] D134344: [WIP][lldb][test] 1 - Remove gmodules debug_info variant: add decorator for API tests that explicitly test gmodules

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 abandoned this revision. Michael137 added a comment. Abandoning in favour of https://reviews.llvm.org/D134524 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134344/new/ https://reviews.llvm.org/D134344 ___

[Lldb-commits] [PATCH] D134570: [lldb] Skip check for conflicting filter/synth when adding a new regex.

2022-09-23 Thread Jorge Gorbe Moya via Phabricator via lldb-commits
jgorbe created this revision. jgorbe added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. jgorbe requested review of this revision. When adding a new synthetic child provider, we check for an existing conflicting filter in the same category (and vice versa).

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

2022-09-23 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D134333#3812448 , @clayborg wrote: > I just did a search through our test sources and we use > GetError().GetCString() 34 times in our test suites python files. So the > SBError::GetCString() is being misused a lot like this

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

2022-09-23 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision. clayborg added a comment. Thanks for fixing the issue! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134516/new/ https://reviews.llvm.org/D134516 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

2022-09-23 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. I just did a search through our test sources and we use GetError().GetCString() 34 times in our test suites python files. So the SBError::GetCString() is being misused a lot like this already and is probably making some tests flaky. I would highly recommend we fix SBEr

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

2022-09-23 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In D134041#3811289 , @labath wrote: > In D134041#3806994 , @jasonmolenda > wrote: > >> In D134041#3805941 , @labath wrote: >> >>> In D134041#380

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

2022-09-23 Thread Martin Storsjö via Phabricator via lldb-commits
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) +symbol.SetType(exported->Ge

[Lldb-commits] [PATCH] D134509: [LLDB][NativePDB] Let native pdb use class layout in debug info.

2022-09-23 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu marked an inline comment as done. zequanwu added a comment. In D134509#3811203 , @labath wrote: > It's not clear to me how much of this patch is pure refactoring, and how much > of it is adding new features. It would have been better to split th

[Lldb-commits] [PATCH] D134509: [LLDB][NativePDB] Let native pdb use class layout in debug info.

2022-09-23 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 462555. zequanwu added a comment. Seperate refactor and the chang that fix class layout. This just hooks PdbAstBuilder to TypeSystem to use class layout from native pdb, but it's still broken because the layout bitsize is missing. Repository: rG LLVM Git

[Lldb-commits] [PATCH] D134515: [lldb] Fix completion of 'settings set' values

2022-09-23 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added inline comments. Comment at: lldb/test/API/functionalities/completion/TestCompletion.py:410 +"""Test that 'settins set stop-disassembly-display ' completes to [ +'never', 'always', 'no-debuginfo', 'no-source'].""" +self.complete_from_to('

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

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
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] D134515: [lldb] Fix completion of 'settings set' values

2022-09-23 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added inline comments. Comment at: lldb/test/API/functionalities/completion/TestCompletion.py:410 +"""Test that 'settins set stop-disassembly-display ' completes to [ +'never', 'always', 'no-debuginfo', 'no-source'].""" +self.complete_from_to('

[Lldb-commits] [PATCH] D134515: [lldb] Fix completion of 'settings set' values

2022-09-23 Thread Dave Lee via Phabricator via lldb-commits
kastiglione updated this revision to Diff 462532. kastiglione added a comment. Rename test and change the docstring to be meaningful. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134515/new/ https://reviews.llvm.org/D134515 Files: lldb/source/C

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

2022-09-23 Thread Martin Storsjö via Phabricator via lldb-commits
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::eSymbolTypeInval

[Lldb-commits] [lldb] 8600014 - [lldb] Use Optional::{has_value, value, value_or}

2022-09-23 Thread Kazu Hirata via lldb-commits
Author: Kazu Hirata Date: 2022-09-23T09:10:40-07:00 New Revision: 8600014b78d314e53fd941f93811492caeba2d0f URL: https://github.com/llvm/llvm-project/commit/8600014b78d314e53fd941f93811492caeba2d0f DIFF: https://github.com/llvm/llvm-project/commit/8600014b78d314e53fd941f93811492caeba2d0f.diff L

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

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
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] D134536: [LLDB] Add an llvm::Optional version of GetRegisterInfo

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. I appreciate this is a lot of churn for code that likely "just works" (or in reality, "isn't used very often") but with the recent riscv changes I'd like to modernise this area where possible. If this is too much at least I know to look for smaller opportunities i

[Lldb-commits] [PATCH] D134537: [LLDB] Move MIPS64/PPC64/RISCV and misc. to optional GetRegisterInfo

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 462494. DavidSpickett added a comment. Remove stray newline in riscv. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134537/new/ https://reviews.llvm.org/D134537 Files: lldb/source/Core/EmulateInstructi

[Lldb-commits] [PATCH] D134536: [LLDB] Add an llvm::Optional version of GetRegisterInfo

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: clayborg. DavidSpickett added a subscriber: clayborg. DavidSpickett added a comment. @clayborg As promised on https://reviews.llvm.org/D134041. The strategy is: - Make the original function concrete and have it call the optional version. - Derived classes implemen

[Lldb-commits] [PATCH] D134541: [LLDB] Remove the bool + RegisterInfo& version of GetRegisterInfo

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added subscribers: frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, atanasyan, edward-jones, zzheng, jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, kbarton, nemanja

[Lldb-commits] [PATCH] D134540: [LLDB][AArch64] Move instruction emulation to optional GetRegisterInfo

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added a subscriber: kristof.beyls. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Depends on D134539 Repository:

[Lldb-commits] [PATCH] D134539: [LLDB][MIPS] Move instruction emulation to optional GetRegisterInfo

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added subscribers: atanasyan, jrtc27, arichardson, sdardis. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Depends on D134538

[Lldb-commits] [PATCH] D134538: [LLDB][ARM] Move instruction emulation to optional GetRegisterInfo

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added a subscriber: kristof.beyls. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Depends on D134537 Repository:

[Lldb-commits] [PATCH] D134537: [LLDB] Move MIPS64/PPC64/RISCV and misc. to optional GetRegisterInfo

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added subscribers: sunshaoce, VincentWu, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, shchenz, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, atanasyan, edward-jones, zzheng, jrtc27, shiva0

[Lldb-commits] [PATCH] D134536: [LLDB] Add an llvm::Optional version of GetRegisterInfo

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added subscribers: frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, atanasyan, edward-jones, zzheng, jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, kbarton, nemanja

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

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
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

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
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

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
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

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
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

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
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

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
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] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test:1 +# XFAIL: system-windows +# REQUIRES: native && (target-x86 || target-x86_64) ---

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

2022-09-23 Thread Martin Storsjö via Phabricator via lldb-commits
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/sourc

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D134041#3806994 , @jasonmolenda wrote: > In D134041#3805941 , @labath wrote: > >> In D134041#3805034 , >> @DavidSpickett wrote: >> That s

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

2022-09-23 Thread Martin Storsjö via Phabricator via lldb-commits
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] [lldb] c831cea - [LLDB] Fix "memory region --all" when there is no ABI plugin

2022-09-23 Thread David Spickett via lldb-commits
Author: David Spickett Date: 2022-09-23T12:32:38Z New Revision: c831cea5efaaafaa97e3faf3119da3362868f41a URL: https://github.com/llvm/llvm-project/commit/c831cea5efaaafaa97e3faf3119da3362868f41a DIFF: https://github.com/llvm/llvm-project/commit/c831cea5efaaafaa97e3faf3119da3362868f41a.diff LOG

[Lldb-commits] [PATCH] D134030: [LLDB] Fix "memory region --all" when there is no ABI plugin

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc831cea5efaa: [LLDB] Fix "memory region --all" when there is no ABI plugin (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134030/

[Lldb-commits] [lldb] ee58200 - [LLDB] Properly return errors from "memory region --all"

2022-09-23 Thread David Spickett via lldb-commits
Author: David Spickett Date: 2022-09-23T12:32:12Z New Revision: ee582001bf19be8611257df7c5fc5a5e7e7da586 URL: https://github.com/llvm/llvm-project/commit/ee582001bf19be8611257df7c5fc5a5e7e7da586 DIFF: https://github.com/llvm/llvm-project/commit/ee582001bf19be8611257df7c5fc5a5e7e7da586.diff LOG

[Lldb-commits] [PATCH] D134029: [LLDB] Properly return errors from "memory region --all"

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGee582001bf19: [LLDB] Properly return errors from "memory region --all" (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134029/new/

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

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2443 StopInfoSP stop_info = thread_sp->GetStopInfo(); if (!stop_info) + continue; Please add a comment explaining why we walk all the threads. (though

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. lol, I knew about the last two parts (not that I agree with them, but I think we have about an equal amount of code which relies on it, and that which tries to work around it), but the first one is really weird. I think we have invented a middle ground between sign- and

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D134333#3807511 , @clayborg wrote: > In D134333#3805945 , @labath wrote: > >> Do we actually promise that strings returned by the SB API will live >> forever? That's not something *I* w

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread Andy Yankovsky via Phabricator via lldb-commits
werat added a comment. On a somewhat related note, is it expected that `GetValueAsUnsigned()` performs an overflow as `int32` if the type is smaller that `int64`? ❯ cat overflow.cc #include int main() { int8_t i8 = -1; int32_t i32 = -1; int64_t i64 = -1; return 0; }

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

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D134518#3811206 , @alvinhochun wrote: > In D134518#3811160 , @labath wrote: > >> Where is the "dll description string" that they point to? Could they be made >> to point to that? > > T

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. Seems fine (with the caveat that all I know about coroutines was learned in this review). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132815/new/ https://reviews.llvm.org/D132815 __

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

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:892 } + std::sort(export_list.begin(), export_list.end(), RVASymbolListCompareRVA); + return export_list; Can you have multiple symbols pointing to the same

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

2022-09-23 Thread Martin Storsjö via Phabricator via lldb-commits
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 https://lists.llvm.org/

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

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
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] D134509: [LLDB][NativePDB] Let native pdb use class layout in debug info.

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. It's not clear to me how much of this patch is pure refactoring, and how much of it is adding new features. It would have been better to split that out into two patches. I see some layout handling code in `UdtRecordCompleter` constructor, but that's just two lines of co

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

2022-09-23 Thread Martin Storsjö via Phabricator via lldb-commits
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) + ex

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

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Where is the "dll description string" that they point to? Could they be made to point to that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134518/new/ https://reviews.llvm.org/D134518

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

2022-09-23 Thread Martin Storsjö via Phabricator via lldb-commits
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 symtab

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

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. 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 symtab output), then I think we should keep them. You never know -- maybe someone will find

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd5d904285008: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types (authored by Michael137). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[Lldb-commits] [lldb] d5d9042 - [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread Michael Buch via lldb-commits
Author: Michael Buch Date: 2022-09-23T12:27:08+02:00 New Revision: d5d90428500870107909fb8f90023ff608cd1ec2 URL: https://github.com/llvm/llvm-project/commit/d5d90428500870107909fb8f90023ff608cd1ec2 DIFF: https://github.com/llvm/llvm-project/commit/d5d90428500870107909fb8f90023ff608cd1ec2.diff

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Ok, sounds good then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134493/new/ https://reviews.llvm.org/D134493 __

[Lldb-commits] [PATCH] D134344: [WIP][lldb][test] 1 - Remove gmodules debug_info variant: add decorator for API tests that explicitly test gmodules

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D134344#3805953 , @Michael137 wrote: > that would require an audit of each API test right? Not really. I think this could be a purely mechanical change which replaces `NO_DEBUG_INFO_TESTCASE = False` with something different.

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 462435. Michael137 added a comment. - Reword commit message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134493/new/ https://reviews.llvm.org/D134493 Files: lldb/source/Plugins/TypeSystem/Clang/TypeSyste

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 462434. Michael137 added a comment. - Fix test description Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134493/new/ https://reviews.llvm.org/D134493 Files: lldb/source/Plugins/TypeSystem/Clang/TypeSystem

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added a comment. In D134493#3811097 , @labath wrote: > In D134493#3810290 , @Michael137 > wrote: > >> Wasn't sure how to properly test this since the original reproducer >> technically relies on imple

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

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1552 strm.IndentMore(); + strm.Indent("Name: "); + strm.PutCStrin

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D134493#3810290 , @Michael137 wrote: > Wasn't sure how to properly test this since the original reproducer > technically relies on implementation-defined behaviour (i.e., initialising a > bitfield with an out-of-range value).

[Lldb-commits] [PATCH] D134524: [WIP][lldb][test] 1 - Remove gmodules debug_info variant: add decorator for API tests that explicitly test gmodules

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 462432. Michael137 added a comment. - Fixup prototype Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134524/new/ https://reviews.llvm.org/D134524 Files: lldb/packages/Python/lldbsuite/test/lldbtest.py ll

[Lldb-commits] [PATCH] D134344: [WIP][lldb][test] 1 - Remove gmodules debug_info variant: add decorator for API tests that explicitly test gmodules

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added a comment. > (B) Keep the `gmodules` category in the debug_info categories but add an > indicator (e.g., by making the `debug_info_categories` a dictionary) that > will skip replication if set. That would solve (1). And (2) would work as it > does today without changes. Upload

[Lldb-commits] [PATCH] D134524: [WIP][lldb][test] 1 - Remove gmodules debug_info variant: add decorator for API tests that explicitly test gmodules

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 created this revision. Michael137 added reviewers: aprantl, labath. Herald added a project: All. Michael137 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Currently, by default LLDB runs an API test with 3 variants, one of which

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

2022-09-23 Thread Alvin Wong via Phabricator via lldb-commits
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] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added a comment. In D134493#3810991 , @DavidSpickett wrote: > It wouldn't need to be same across platforms either really. Can always > `@skipifnotarchwhatever` and as long as it's tested somewhere that's fine. Ok > clang could change its min

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 462427. Michael137 added a comment. - Add API test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134493/new/ https://reviews.llvm.org/D134493 Files: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cp

[Lldb-commits] [PATCH] D134515: [lldb] Fix completion of 'settings set' values

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/test/API/functionalities/completion/TestCompletion.py:409 +def test_settings_set_stop_disassembly_display_value(self): +"""Test that 'settins set stop-disassembly-display ' completes to [ +'never', 'always'

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

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: DavidSpickett. DavidSpickett added inline comments. Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1552 strm.IndentMore(); + strm.Indent("Name: "); + strm.PutCString(symbol->GetDisplayName().GetStringRef()

[Lldb-commits] [PATCH] D134515: [lldb] Fix completion of 'settings set' values

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: DavidSpickett. DavidSpickett added inline comments. Comment at: lldb/test/API/functionalities/completion/TestCompletion.py:409 +def test_settings_set_stop_disassembly_display_value(self): +"""Test that 'settins set stop-disassembly-disp

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. It wouldn't need to be same across platforms either really. Can always `@skipifnotarchwhatever` and as long as it's tested somewhere that's fine. Ok clang could change its mind so we mitigate that by making sure the test would fail if it does, then update it as ne

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

2022-09-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > Wasn't sure how to properly test this since the original reproducer > technically relies on implementation-defined behaviour (i.e., initialising a > bitfield with an out-of-range value). Suggestions are welcome Is this undefined behaviour defined at least for cl