[Lldb-commits] [PATCH] D76697: [lldb] Replace debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType with lldbassert

davide added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76697/new/ https://reviews.llvm.org/D76697 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

davide closed this revision. davide added a comment. Somebody will pick this up, probably in a different form. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82813/new/ https://reviews.llvm.org/D82813 ___ lldb-commits mailing list lldb-commits

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

davide added inline comments. Comment at: lldb/source/Symbol/Type.cpp:378 m_byte_size_has_value = true; +return m_byte_size; } shafik wrote: > labath wrote: > > davide wrote: > > > shafik wrote: > > > > Wouldn't it be better to turn `m_byt

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

davide added a subscriber: teemperor. davide added a comment. @labath something we noticed when finding this (and related bugs) is that `frame var` carries a decent diagnostic (int *) l_125 = and the expression parser returns just returns something not particularly useful: (lldb) p l_125

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

davide added inline comments. Comment at: lldb/source/Symbol/Type.cpp:378 m_byte_size_has_value = true; +return m_byte_size; } shafik wrote: > Wouldn't it be better to turn `m_byte_size` into an `Optional`? As this fix > shows this having

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. I came to the same conclusion when analyzing https://bugs.llvm.org/show_bug.cgi?id=47257 (but you beat me to the punch). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[Lldb-commits] [PATCH] D85396: Fix a small memory leak in VectorType.cpp and BlockPointer.cpp

davide added a comment. In D85396#2198668 , @tschuett wrote: > auto_ptr is deprecated since C++11 and will be removed in C++17. @jasonmolenda you might want to use `std::unique_ptr` instead -- probably LLVM won't migrate to 17 for a while, but still. R

[Lldb-commits] [PATCH] D85396: Fix a small memory leak in VectorType.cpp and BlockPointer.cpp

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. This is correct to the best of my understanding. Thank you Jason. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85396/new/ https://reviews.llvm.

[Lldb-commits] [PATCH] D85388: [lldb] Fix bug in skipIfRosetta decorator

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. yes, this makes sense. We could refine the check in future. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85388/new/ https://reviews.llvm.org/D85388 ___

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

davide added inline comments. Comment at: lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:2 +EXE := a.out +CFLAGS := -O1 + davide wrote: > This is fundamentally a no-go. Depending on the optimization pipeline passes > this in a regist

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

davide requested changes to this revision. davide added inline comments. This revision now requires changes to proceed. Comment at: lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:2 +EXE := a.out +CFLAGS := -O1 + This is fundamentally

[Lldb-commits] [PATCH] D85049: Unify the code that updates the ArchSpec after finding a fat binary with how it is done for a lean binary

davide added inline comments. Comment at: lldb/source/Target/TargetList.cpp:110-111 +// architecture so that the platform matching can be more accurate. +if (!platform_arch.TripleOSWasSpecified() || +!platform_arch.TripleVendorWasSpecified()) { + prefer_platf

[Lldb-commits] [PATCH] D85365: [lldb] Modify the `skipIfRemote` decorator so we can skip all PExpect tests.

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. I assume we're still allowing to put the decorator on a test-by-test basis, and that seems the case from what I see. If so, LG. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://r

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

davide closed this revision. davide added a comment. https://github.com/llvm/llvm-project/commit/57605758b5de3726eec1d6e587de1003af1ab5b7 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84263/new/ https://reviews.llvm.org/D84263 ___ lldb-commit

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Reviewed by Jason privately. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84263/new/ https://reviews.llvm.org/D84263 ___ lldb-commits mai

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

davide updated this revision to Diff 282073. davide added a comment. Added the check that Jason requested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84263/new/ https://reviews.llvm.org/D84263 Files: lldb/tools/debugserver/source/DNB.cpp lldb/tools/debugserver/source/debugserver

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. why? Do you have a testcase? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84272/new/ https://reviews.llvm.org/D84272 ___ lldb

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

davide added a comment. yeah, I think it's reasonable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84263/new/ https://reviews.llvm.org/D84263 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

davide created this revision. davide added a reviewer: jasonmolenda. When we detect a process that the native debugserver cannot handle, handoff the connection fd to the translated debugserver. https://reviews.llvm.org/D84263 Files: lldb/tools/debugserver/source/DNB.cpp lldb/tools/debugser

[Lldb-commits] [PATCH] D83796: [ObjC] Wrap namespace-global structure in an anonymous namespace to avoid ODR violations

This revision was automatically updated to reflect the committed changes. Closed by commit rG3f2d880a9329: [ObjC] Wrap namespace-global structs in an anonymous namespace to avoid ODR… (authored by davide). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[Lldb-commits] [PATCH] D83796: [ObjC] Wrap namespace-global structure in an anonymous namespace to avoid ODR violations

davide created this revision. davide added a reviewer: teemperor. rdar://problem/65537147 https://reviews.llvm.org/D83796 Files: lldb/source/Plugins/Language/ObjC/NSArray.cpp lldb/source/Plugins/Language/ObjC/NSDictionary.cpp Index: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp ==

[Lldb-commits] [PATCH] D83600: Add a decorator to skip tests when running under Rosetta

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Good, one minor suggestion. Comment at: lldb/packages/Python/lldbsuite/test/decorators.py:560 +return False +return platform.uname()[5] == "arm" and self.

[Lldb-commits] [PATCH] D83582: Fix nesting of #ifdef

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83582/new/ https://reviews.llvm.org/D83582 ___ lldb-commits mailing list lldb-commits

[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

davide added a comment. This broke macOS: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22425/ I reverted it in: commit 27d52cd86a2cf82214b71519dffd450c54cf87ae (HEAD -> master, origin/master, origin/HEAD) Author: Davide Italiano Date: Wed Jul 8 13:00:29 2020 -0700 Re

[Lldb-commits] [PATCH] D82804: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

This revision was automatically updated to reflect the committed changes. davide marked an inline comment as done. Closed by commit rG5832473dcf4e: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed. (authored by davide). Herald added a project: LLDB. Repository: rG LLVM Gi

[Lldb-commits] [PATCH] D82804: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

This revision was automatically updated to reflect the committed changes. davide marked an inline comment as done. Closed by commit rG5832473dcf4e: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed. (authored by davide). Herald added a project: LLDB. Repository: rG LLVM Gi

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

davide added a comment. In D83327#2136842 , @jingham wrote: > In D83327#2136814 , @davide wrote: > > > Aside from cosmetics, I'm not entirely sure this is the correct fix. Why > > are we calling this code _at all_

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

davide added a comment. Aside from cosmetics, I'm not entirely sure this is the correct fix. Why are we calling this code _at all_ if the type is incomplete? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83327/new/ https://reviews.llvm.org/D83327

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

davide abandoned this revision. davide added a comment. Cleaning up my queue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43048/new/ https://reviews.llvm.org/D43048 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.l

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

davide marked an inline comment as done. davide added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3424 +if (Host::IsProcessTranslated(process_info)) { + FileSpec rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver

[Lldb-commits] [PATCH] D82616: Improve the detection of iOS/tvOS/watchOS simulator binaries in debugserver and lldb

davide accepted this revision. davide added a comment. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82616/new/ https://reviews.llvm.org/D82616 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

davide marked an inline comment as done. davide added inline comments. Comment at: lldb/include/lldb/Host/Host.h:231 + /// Check whether a process is translated (Rosetta). + /// \arg process_info The info structure for the process queried. aprantl wrote: > Is

[Lldb-commits] [PATCH] D82804: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

davide marked an inline comment as done. davide added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:366 -if args.server: +if args.server and not args.out_of_tree_debugserver: os.environ['LLDB_DEBUGSERVER_PATH'] = args.server ---

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

davide marked an inline comment as done. davide added inline comments. Comment at: lldb/include/lldb/Host/Host.h:231 + /// Run a shell command. + /// \arg process_info The info structure for the process queried. Stale comment, I'll update CHANGES SINCE LAST

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

davide created this revision. davide added reviewers: aprantl, jasonmolenda. Nothing crazy here, just an organizational cleanup. https://reviews.llvm.org/D82813 Files: lldb/include/lldb/Host/Host.h lldb/source/Host/common/Host.cpp lldb/source/Host/macosx/objcxx/Host.mm lldb/source/Plugi

[Lldb-commits] [PATCH] D82491: [Apple Silicon] Initial support for Rosetta

davide marked an inline comment as done. davide added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3444 +size_t bufsize = sizeof(processInfo); +if (sysctl(mib, (unsigned)(sizeof(mib)/sizeof(int)), &processInfo, + &

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

davide updated this revision to Diff 274253. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82813/new/ https://reviews.llvm.org/D82813 Files: lldb/include/lldb/Host/Host.h lldb/source/Host/common/Host.cpp lldb/source/Host/macosx/objcxx/Host.mm lldb/source/Plugins/Process/gdb-remot

[Lldb-commits] [PATCH] D82804: Do not set LLDB_DEBUGSERVER_PATH if --out-of-tree-debugserver is passed.

davide created this revision. davide added reviewers: vsk, aprantl, jasonmolenda. Now that there are two implementations of debugserver, one for native and the other for Rosetta [on Apple Silicon], this is needed. https://reviews.llvm.org/D82804 Files: lldb/packages/Python/lldbsuite/test/dot

[Lldb-commits] [PATCH] D82493: [Apple Silicon] Debugging of process under Rosetta is supported

davide accepted this revision. davide added a comment. commit b4fdddf971b191aa9a6643ab637b87bc1d686254 (HEAD -> master, origin/master, origin/HEAD) Author: Davide Italiano Date: Wed Jun 24 12:25:01 2020 -0700 [Apple Silicon] Debugging of process under Rosetta is supported.

[Lldb-commits] [PATCH] D82491: [Apple Silicon] Initial support for Rosetta

davide closed this revision. davide added a comment. commit fd19ddb8f2a2b082f492fc59f7f360adf3495701 (HEAD -> master, origin/master, origin/HEAD) Author: Davide Italiano Date: Wed Jun 24 12:18:29 2020 -0700 [Apple Silicon] Initial support for Rosetta Translated proc

[Lldb-commits] [PATCH] D82491: [Apple Silicon] Initial support for Rosetta

davide added a comment. In D82491#2112269 , @jasonmolenda wrote: > LGTM. With p_flag, we only need to evaluate (processInfo.kp_proc.p_flag & > P_TRANSLATED) as a boolean, but that's a style pref as much as anything. Fair, let me change that. CHANGES

[Lldb-commits] [PATCH] D82493: [Apple Silicon] Debugging of process under Rosetta is supported

davide created this revision. davide added reviewers: jasonmolenda, friss, aprantl. Remove this early exit. It's vestigial from the ppc -> Intel transition, but it doesn't apply anymore: https://reviews.llvm.org/D82493 Files: lldb/source/Host/macosx/objcxx/Host.mm Index: lldb/source/Host/m

[Lldb-commits] [PATCH] D82491: [Apple Silicon] Initial support for Rosetta

davide created this revision. davide added reviewers: jasonmolenda, aprantl, friss. jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. davide closed this revision. LGTM. With p_flag, we only need to evaluate (processInfo.kp_proc.p_f

[Lldb-commits] [PATCH] D82394: [debugserver] Initial support for Apple Silicon.

davide added a comment. commit 2276bb48be0175f96b1494ca67b7921f0d4e87d8 (HEAD -> master, origin/master, origin/HEAD) Author: Davide Italiano Date: Tue Jun 23 10:18:54 2020 -0700 [debugserver] Initial support for Apple Silicon. Set the correct os type in the arch tri

[Lldb-commits] [PATCH] D82394: [debugserver] Initial support for Apple Silicon.

davide added a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82394/new/ https://reviews.llvm.org/D82394 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82394: [debugserver] Initial support for Apple Silicon.

davide added a comment. Danke shoen CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82394/new/ https://reviews.llvm.org/D82394 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-comm

[Lldb-commits] [PATCH] D81119: [lldb] Fix SLEB128 decoding

davide added a comment. Greg & Pavel might have opinions on this patch. I'm not qualified to review it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81119/new/ https://reviews.llvm.org/D81119 ___ lldb

[Lldb-commits] [PATCH] D80955: Fix UB in EmulateInstructionARM64.cpp

davide added a comment. This is great Adrian. I feel like there are several other instances of additions that should use `llvm::checkAdd` here instead of `+`. Is this the only UB triggered in the testsuite? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80955/new/ https://reviews.llvm.

[Lldb-commits] [PATCH] D80955: Fix UB in EmulateInstructionARM64.cpp

davide added inline comments. Comment at: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp:25 +#include + shafik wrote: > Why? also should be `cstdlib` in C++. It's just something shuffled around, probably a side effect of clang-format. CHAN

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

davide accepted this revision. davide added inline comments. This revision is now accepted and ready to land. Comment at: lldb/include/lldb/DataFormatters/Mock.h:1-8 +//===-- Mock.h --*- C++ -*-===// +// +// Part of the LLVM Projec

[Lldb-commits] [PATCH] D80150: [lldb/DataFormatter] Check for overflow when finding NSDate epoch

davide added a comment. Looking, as I touched `NSDate` last year. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80150/new/ https://reviews.llvm.org/D80150 ___ lldb-commits mailing list lldb-commits@lis

[Lldb-commits] [PATCH] D79823: [lldb][Core] Remove dead codepath in Mangled

davide added a comment. I'm curious what happens if you end up here with something like `_T`. Did you check? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79823/new/ https://reviews.llvm.org/D79823 __

[Lldb-commits] [PATCH] D79823: [lldb][Core] Remove dead codepath in Mangled

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Presumably. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79823/new/ https://reviews.llvm.org/D79823

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

davide added a comment. Saleem is the right person to review this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78972/new/ https://reviews.llvm.org/D78972 ___ lldb-commits mailing list lldb-com

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

davide added a comment. I'm really worried about this change -- and we should think about it very carefully because it might cause the LLVM verifier to freak out -- or even worse, causing random crashes when executing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

davide added a comment. Weak functions can actually be overridden -- I'm really wondering whether this causes issues here. Do you have an example [even if it's in swift] of a symbol that's not exported correctly? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[Lldb-commits] [PATCH] D78588: [lldb/Core] Check that ArchSpec is valid.

davide added a comment. What's exactly the testcase doing? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78588/new/ https://reviews.llvm.org/D78588 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

davide added a comment. This looks fine to me but please give Pavel another chance to look before committing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78396/new/ https://reviews.llvm.org/D78396 _

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

davide added a comment. This is almost ready. After we're done with this round of cosmetics, I'll take another look a the algorithm and sign off. Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:18-19 + return true; +else if (m_ptr_size == 8 && m_ht_64) +

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

davide added a comment. Thanks for taking care of this. It's a lot of work. First round of comments. Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:12 + +CFBasicHash::~CFBasicHash() { m_address = LLDB_INVALID_ADDRESS; } + Why do you need this? Ca

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

davide marked an inline comment as done. davide added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1029 + return false; + +case DW_TAG_compile_unit: aprantl wrote: > To save everyone the trouble: > ``` > 0x000

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

davide closed this revision. davide added a comment. commit d51b38f1b3a34c2a8e1869af6434ebd743ce7a5e (HEAD -> master, origin/master, origin/HEAD) Author: Davide Italiano Date: Wed Apr 8 11:06:00 2020 -0700 [DWARF] Not al

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

davide updated this revision to Diff 256063. davide added a comment. Herald added a reviewer: jdoerfert. Added test, updated comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77698/new/ https://reviews.llvm.org/D77698 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEnt

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

davide created this revision. davide added reviewers: jingham, labath. Herald added subscribers: arphaman, aprantl. davide added a comment. davide added a reviewer: friss. See also https://bugs.llvm.org/show_bug.cgi?id=45471 SymbolFileDWARF::ParseVariableDIE consider all constant variables as "s

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

davide added a comment. See also https://bugs.llvm.org/show_bug.cgi?id=45471 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77698/new/ https://reviews.llvm.org/D77698 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.ll

[Lldb-commits] [PATCH] D77173: [PATCH] [debugserver/ARM64] Make sure watchpoints hit are attributed correctly.

davide created this revision. davide added a reviewer: jasonmolenda. davide added a project: LLDB. Herald added subscribers: danielkiss, kristof.beyls. >From e330facaea0c3780734a6a061134551662fb9d74 Mon Sep 17 00:00:00 2001 From: Davide Italiano Date: Tue, 31 Mar 2020 13:55:36 -0700 Subject: [PAT

[Lldb-commits] [PATCH] D76730: [RNBRemote/arm64] Remove a couple of unused functions

davide created this revision. davide added reviewers: jasonmolenda, LLDB. Herald added a subscriber: kristof.beyls. Jason, before submitting this I wanted to run it past you. https://reviews.llvm.org/D76730 Files: lldb/tools/debugserver/source/RNBRemote.cpp Index: lldb/tools/debugserver/sour

[Lldb-commits] [PATCH] D76687: [lldb][NFC] Always update m_cache_{hits/misses} in FormatCache

davide added a comment. Testcase? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76687/new/ https://reviews.llvm.org/D76687 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.o

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

davide added a comment. In D76697#1939731 , @JDevlieghere wrote: > According to the comment there is at least one valid reason for this code > path to trigger. An `lldb_assert` will encourage users to file bugs and I > think we have enough real issues t

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

davide added a comment. I don't recall the details. Soft assertions. LLDB provides lldb_assert() as a soft alternative to cover the middle ground of situations that indicate a recoverable bug in LLDB. In a Debug configuration lldb_assert() behaves like assert(). In a Release configuration it

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

davide added a comment. Also, this was asserting already -- so it's not introducing any new `assert()`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76697/new/ https://reviews.llvm.org/D76697 ___ lldb-commits mailing list lldb-commits@list

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

davide added a comment. Please don't do this. I've seen that assertion triggering. If anything, you might want to make a `lldbassert`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76697/new/ https://reviews.llvm.org/D76697 ___ lldb-commit

[Lldb-commits] [PATCH] D75496: [lldb][NFC] Remove some commented out code in TypeSystemClang

davide added a comment. @teemperor quite honestly you touched this code enough lately that you can commit such trivial changes without asking for review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75496/new/ https://reviews.llvm.org/D75496

[Lldb-commits] [PATCH] D75496: [lldb][NFC] Remove some commented out code in TypeSystemClang

davide added a comment. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75496/new/ https://reviews.llvm.org/D75496 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D73921: Assert that a subprogram should have a name when parsing DWARF

davide added a comment. In D73921#1868286 , @JDevlieghere wrote: > In D73921#1868228 , @jingham wrote: > > > In D73921#1855961 , @davide wrote: > > > > > DWARFASTParserClang

[Lldb-commits] [PATCH] D73938: [Host.mm] Check for the right macro instead of inlining it

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM =) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73938/new/ https://reviews.llvm.org/D73938 ___ lldb-commits mailing list lldb-comm

[Lldb-commits] [PATCH] D73952: [lldb] Ignore type sugar in TypeSystemClang::GetPointerType

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

[Lldb-commits] [PATCH] D71316: [FormatManager] Upstream and test swift bits for GetCandidateLanguages().

davide closed this revision. davide added a comment. Not relevant anymore. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71316/new/ https://reviews.llvm.org/D71316 ___ lldb-commits mailing list lldb-co

[Lldb-commits] [PATCH] D73938: [Host.mm] Check for the right macro instead of inlining it.

davide created this revision. davide added reviewers: jingham, friss, vsk. Herald added a project: LLDB. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D73938 Files: lldb/source/Host/macosx/objcxx/Host.mm Index: lldb/source/Host/macosx/objcxx/Host.mm =

[Lldb-commits] [PATCH] D73921: Assert that a subprogram should have a name when parsing DWARF

davide added a comment. DWARFASTParserClang looks to me the wrong layer to fix this. Why can't this be caught in the generic DWARF Parser? I also believe that it's better if dwarfdump -verify crashes on this, rather than lldb. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73921/new/ h

[Lldb-commits] [PATCH] D73921: Assert that a subprogram should have a name when parsing DWARF

davide added a comment. `DWARFASTParserClang` looks to me the right layer to fix this. Why can't this be caught in the generic DWARF Parser? I also believe that it's better if `dwarfdump -verify` crashes on this, rather than lldb. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73921/new

[Lldb-commits] [PATCH] D73913: [lldb/DataExtractor] Fix UB shift in GetMaxS64Bitfield

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM, thanks for taking care of this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73913/new/ https://reviews.llvm.org/D73913 ___ lldb-

[Lldb-commits] [PATCH] D73913: [lldb/DataExtractor] Fix UB shift in GetMaxS64Bitfield

davide added a comment. In D73913#1855486 , @shafik wrote: > I wish I had caught that one when I did D70992 > , might be worth checking out more of the > code for similar issues. I guess this is why we have tooling =)

[Lldb-commits] [PATCH] D73389: [lldb/Breakpoint] Include whether or not a breakpoint is a HW BP

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73389/new/ https://reviews.llvm.org/D73389 ___ lldb-commit

[Lldb-commits] [PATCH] D72879: Add testing for DW_OP_piece and fix a bug with small Scalar values.

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Looks great! Thanks Adrian. My understanding is that `DW_OP_piece` is still incomplete, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D728

[Lldb-commits] [PATCH] D72684: [lldb][NFC] Rename ClangASTContext to TypeSystemClang

davide added a comment. This is very good, go for it. Should we do the same for Swift? cc: @aprantl For the future, please CC: me directly on these kind of changes if you want my review, as I might miss them otherwise. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72684/new/ https://r

[Lldb-commits] [PATCH] D72495: [lldb] Make CompleteTagDeclsScope completion order deterministic

davide accepted this revision. davide added a comment. LGTM Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72495/new/ https://reviews.llvm.org/D72495 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https:/

[Lldb-commits] [PATCH] D72413: Add missing nullptr checks.

davide added a comment. In D72413#1813630 , @teemperor wrote: > The C++ expression parser will probably behave incredibly incorrectly without > a persistent state but before this patch it just crashed, so I think this is > good to go. Should we conside

[Lldb-commits] [PATCH] D72413: Add missing nullptr checks.

davide added a comment. @teemperor what do you think? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72413/new/ https://reviews.llvm.org/D72413 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/

[Lldb-commits] [PATCH] D72413: Add missing nullptr checks.

davide added a comment. In D72413#1812464 , @aprantl wrote: > Should we merge this like that, or is there a better way of doing this? We should merge it like this, IMHO. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72413/new/ https://reviews.

[Lldb-commits] [PATCH] D72413: Add missing nullptr checks.

davide added a comment. Raphael and Jim should look at the expression evaluator bits. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72413/new/ https://reviews.llvm.org/D72413 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https:/

[Lldb-commits] [PATCH] D68679: [CMake] Add a cache for iOS.

davide abandoned this revision. davide added a comment. We don't need this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68679/new/ https://reviews.llvm.org/D68679 ___ lldb-commits mailing list lldb-c

[Lldb-commits] [PATCH] D71379: lldbutil: Forward ASan launch info to test inferiors

davide accepted this revision. davide added a comment. This looks good to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71379/new/ https://reviews.llvm.org/D71379 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.l

[Lldb-commits] [PATCH] D71311: LanguageRuntime: Simplify NSException::GetSummary() output

davide accepted this revision. davide 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/D71311/new/ https://reviews.llvm.org/D71311 __

[Lldb-commits] [PATCH] D71311: LanguageRuntime: Simplify NSException::GetSummary() output

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. Did you test on swift? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71311/new/ https://reviews.llvm.org/D71311 ___

[Lldb-commits] [PATCH] D71316: [FormatManager] Upstream and test swift bits for GetCandidateLanguages().

davide added a comment. This is not a bad idea after all. Let me see if I can cook something reasonable without getting burned. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71316/new/ https://reviews.llvm.org/D71316 ___

[Lldb-commits] [PATCH] D71316: [FormatManager] Upstream and test swift bits for GetCandidateLanguages().

davide added a comment. Ideally -- if we're able to make this a callback in the language plugins, we don't need this patch at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71316/new/ https://reviews.llvm.org/D71316

[Lldb-commits] [PATCH] D71316: [FormatManager] Upstream and test swift bits for GetCandidateLanguages().

davide created this revision. davide added reviewers: teemperor, aprantl, labath, friss, jingham. Herald added a project: LLDB. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D71316 Files: lldb/source/DataFormatters/FormatManager.cpp lldb/unittests/DataFormatter/FormatManager

[Lldb-commits] [PATCH] D71311: LanguageRuntime: Simplify NSException::GetSummary() output

davide added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/NSException.cpp:107-109 + if (NSStringSummaryProvider(*reason_sp, reason_str_summary, options) && + !reason_str_summary.Empty()) { +stream.Printf("%s", reason_str_summary.GetData()); ---

[Lldb-commits] [PATCH] D71310: RFC: Remove "Validators"

davide added a comment. In D71310#1778534 , @jingham wrote: > Adrian and I talked about this some more. Apparently the idea was that you > have some type Foo and you want to look for some error state in instances of > that type (Foo::a + Foo::b < 10).

  1   2   3   4   5   6   >