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

clayborg added a comment. Our other option is to add more headerdoc to the SBError.h file and deal with this API issue by stating that the SBError object must be kept alive long enough to copy the error string returned by "const char *SBError::GetCString()". I am ok with either, I just think

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

clayborg added a comment. In D134333#3805945 , @labath wrote: > Do we actually promise that strings returned by the SB API will live forever? > That's not something *I* would want to promote. I always though that we're > ConstStringifying the return

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

Michael137 added a comment. In D134344#3806509 , @aprantl wrote: > In D134344#3805953 , @Michael137 > wrote: > >>> teach the debug info replication to ignore tests with the gmodules category >>> (just like it

[Lldb-commits] [PATCH] D130534: loading a binary at a slide multiple times leaves old entries in the SectionLoadList

jasonmolenda added a comment. And to be clear, the problem is that we start out with an expression like `first[0]` which we find the Section + address for `first`, then resolve to a load address (addr_t) via the Target SectionLoadList, and then try to read memory from that address - at which

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

jasonmolenda added a comment. In D134041#3805941 , @labath wrote: > In D134041#3805034 , @DavidSpickett > wrote: > >>> That said I would *love* is someone changed the RegisterInfo structs into >>> something

[Lldb-commits] [PATCH] D133525: fix extra bytes when compressing for 32bit objcopy

MaskRay added a comment. D134385 should fix the problem:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133525/new/ https://reviews.llvm.org/D133525 ___ lldb-commits

[Lldb-commits] [PATCH] D130534: loading a binary at a slide multiple times leaves old entries in the SectionLoadList

jasonmolenda updated this revision to Diff 462000. jasonmolenda added a comment. Getting back to this old patch. When I lost track of it, @DavidSpickett was saying that the test case was still not clear -- I was loading a section at an address, checking that I could read values out of arrays,

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

aeubanks created this revision. Herald added a reviewer: shafik. Herald added a project: All. aeubanks updated this revision to Diff 461963. aeubanks added a comment. aeubanks added a reviewer: dblaikie. aeubanks published this revision for review. Herald added a project: LLDB. Herald added a

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

aprantl added a comment. In D134344#3805953 , @Michael137 wrote: >> teach the debug info replication to ignore tests with the gmodules category >> (just like it does for @no_debug_info_test_case tests). This step wouldn't >> be necessary if we made

[Lldb-commits] [PATCH] D134252: Track .dwo/.dwp loading errors and notify user when viewing variables.

yinghuitan accepted this revision. yinghuitan added a comment. This revision is now accepted and ready to land. Technical wise this patch looks good. One concern is that these error messages are user facing but we are making it favoring debugging, like skeleton DIE, showing the DIE offset

[Lldb-commits] [PATCH] D133670: [LLDB][RISCV] Add RVM and RVA instruction support for EmulateInstructionRISCV

aprantl added a comment. Awesome, bot is green again! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133670/new/ https://reviews.llvm.org/D133670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

aprantl added a comment. Thanks! As the one who introduced this feature back in the day (and against Pavel's resistance) I've come around to seeing more value in targeted tests than a spray-paint approach of running the entire testsuite. It doesn't catch many interesting issues, since most

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

Michael137 added a comment. Thanks for reviewing > - keep `gmodules` as a category, but not a *debug info* category. Among other > things this enables running all gmodules tests with the `--category gmodules` > flag. This should be easy to arrange and was the other alternative I considered.

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

labath added a comment. Do we actually promise that strings returned by the SB API will live forever? That's not something *I* would want to promote. I always though that we're ConstStringifying the return values only when we don't know any better... Repository: rG LLVM Github Monorepo

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

labath added a comment. In D134041#3805034 , @DavidSpickett wrote: >> That said I would *love* is someone changed the RegisterInfo structs into >> something saner, but I think that will need to be more elaborate than simply >> stuffing a std::vector

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

labath added a comment. celebration_balloons I like this a lot. Incidentally, I believe the main reason these tests don't work on non-darwin platforms is because their system

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

labath added a comment. I am afraid that this patch misses one method of initiating a debug session -- connecting to a running debug server (`process connect`, `SBTarget::ConnectRemote`). The breakpoint hit counts don't get reset in case of a reconnect. This isn't a particularly common use

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

Michael137 updated this revision to Diff 461864. Michael137 added a comment. - Fix typo in comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134344/new/ https://reviews.llvm.org/D134344 Files:

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

Michael137 added a comment. An alternative would be to keep the `debug_info` category and simply flag it as not to be run by default unless specified explicitly in the test. That way we wouldn't need the `MAKE_GMODULES` in the Makefile and the special `MAKE_DSYM` rule in

[Lldb-commits] [PATCH] D134346: [WIP][lldb][test] 3 - Add gmodules decorator to API tests

Michael137 created this revision. Herald added a project: All. Michael137 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D134346 Files:

[Lldb-commits] [PATCH] D134345: [WIP][lldb][test] 2 - Remove gmodules debug_info variant from lldbsuite

Michael137 created this revision. Herald added a project: All. Michael137 requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D134345 Files:

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

Michael137 created this revision. Michael137 added reviewers: labath, aprantl. 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

[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.

[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.

[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] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

DavidSpickett added inline comments. Comment at: lldb/include/lldb/Core/EmulateInstruction.h:196 + struct RegisterPlusOffsetStruct { RegisterInfo reg; // base register int64_t signed_offset; // signed offset added to base register

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

DavidSpickett added a comment. > I think the main reason that the RegisterInfo struct is such as it is, is > because it wants to be trivially constructible (or whatever the c++ term for > that is). I'm pretty sure that adding a std::string member will make it stop > being that, and I don't

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

DavidSpickett added a comment. > Sorry. I hoped that variant would make your life easier. Np, one can dream (until the next out of reach c++ feature comes along). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134041/new/

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

clayborg added a comment. F24632929: Screen Shot 2022-09-21 at 12.31.23 AM.png Here is a screenshot showing this happening when a .o file is missing on Darwin for DWARF in .o file debugging. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION