[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

aprantl created this revision. aprantl added reviewers: jasonmolenda, jingham, friss. aprantl requested review of this revision. In particular this affects how `target create --arch` is handled — it allowed us to override the deployment target (a useful feature for the expression evaluator), but

[Lldb-commits] [PATCH] D84716: [lldb/ArchSpec] Always match simulator environment in IsEqualTo

aprantl added inline comments. Herald added a subscriber: JDevlieghere. Comment at: lldb/unittests/Utility/ArchSpecTest.cpp:315 +ASSERT_FALSE(B.IsCompatibleMatch(A)); +ASSERT_FALSE(B.IsCompatibleMatch(A)); + } We want to be *really* sure of this? :-) R

[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

aprantl added a comment. It would be nice to (in a follow-up-patch) clearly state what this function's goal is and then re-implement it with slightly fewer fallbacks. The code as it is is very convoluted. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85049/new/ https://reviews.llvm.or

[Lldb-commits] [PATCH] D85243: [WIP] Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

aprantl created this revision. aprantl added reviewers: friss, jasonmolenda, teemperor, vsk. Herald added a subscriber: mgorny. aprantl requested review of this revision. The implementation of these classes was copied & pasted from the iPhone simulator plugin with only a handful of configuration

[Lldb-commits] [PATCH] D84576: Fix debugserver's qProcessInfo reporting of maccatalyst binaries

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG7e9bab6ad51a: Fix debugserver's qProcessInfo reporting of maccatalyst binaries (authored by aprantl). Herald added a proje

[Lldb-commits] [PATCH] D85358: Correctly detect legacy iOS simulator Mach-O objectfiles

aprantl created this revision. aprantl added reviewers: friss, jasonmolenda. aprantl requested review of this revision. The code in ObjectFileMachO didn't disambiguate between ios and ios-simulator object files for Mach-O objects using the legacy ambiguous LC_VERSION_MIN load commands. This used

[Lldb-commits] [PATCH] D85358: Correctly detect legacy iOS simulator Mach-O objectfiles

aprantl updated this revision to Diff 283401. aprantl added a comment. Excellent point, Fred! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85358/new/ https://reviews.llvm.org/D85358 Files: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp lldb/source/Utility/ArchSpec.cpp

[Lldb-commits] [PATCH] D85358: Correctly detect legacy iOS simulator Mach-O objectfiles

aprantl updated this revision to Diff 283404. aprantl added a comment. Fix typo in test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85358/new/ https://reviews.llvm.org/D85358 Files: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp lldb/source/Utility/ArchSpec.cpp lldb/

[Lldb-commits] [PATCH] D85358: Correctly detect legacy iOS simulator Mach-O objectfiles

aprantl updated this revision to Diff 283405. aprantl added a comment. Fix a second mistake in the updated test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85358/new/ https://reviews.llvm.org/D85358 Files: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp lldb/source/Uti

[Lldb-commits] [PATCH] D85243: [WIP] Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

aprantl added inline comments. Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp:558 + "arm64e-apple-ios-simulator", "arm64-apple-ios-simulator", + "x86_64-apple-ios-simulator", "x86_64h-apple-ios-simulator", +#else

[Lldb-commits] [PATCH] D85243: Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

aprantl updated this revision to Diff 283413. aprantl retitled this revision from "[WIP] Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)" to "Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)". aprantl added a comment

[Lldb-commits] [PATCH] D85243: Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

aprantl updated this revision to Diff 283415. aprantl added a comment. Actually update the right patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85243/new/ https://reviews.llvm.org/D85243 Files: lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt lldb/source/Plugins/Platform/Ma

[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

aprantl added inline comments. Comment at: lldb/source/Target/TargetList.cpp:162 // Only one arch and none was specified. -prefer_platform_arch = true; -platform_arch = matching_module_spec.GetArchitecture(); +update_platform_arch(

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

aprantl added inline comments. Comment at: lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:1 +EXE := a.out +CFLAGS := -O1 I think this is redundant. The default is a.out Comment at: lldb/test/API/functionalities/dat

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

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

[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

aprantl updated this revision to Diff 283669. aprantl added a comment. Address review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85049/new/ https://reviews.llvm.org/D85049 Files: lldb/source/Target/TargetList.cpp lldb/test/API/macosx/universal/Makefile lldb/test/API/

[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

aprantl 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_plat

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

aprantl accepted this revision. aprantl added a comment. The test LGTM now! Please be sure to address Fred's comment before committing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85376/new/ https://reviews.llvm.org/D85376 ___ lldb-commits

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

aprantl added a comment. Do we have an end-to-end (with source code instead of assembler) test for ObjC bitfields, too? If not, it might still be a good a idea to add one even if it doesn't add coverage for this particular change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85376/new

[Lldb-commits] [PATCH] D85358: Correctly detect legacy iOS simulator Mach-O objectfiles

This revision was automatically updated to reflect the committed changes. Closed by commit rG05df9cc70367: Correctly detect legacy iOS simulator Mach-O objectfiles (authored by aprantl). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[Lldb-commits] [PATCH] D85243: Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

aprantl added inline comments. Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp:85 + if (!m_sdk.empty()) +strm.Printf(" SDK Path: \"%s\"\n", m_sdk.str().c_str()); + else JDevlieghere wrote: > If you use `<<` you could drop the `.s

[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

aprantl added inline comments. Comment at: lldb/source/Target/TargetList.cpp:144 prefer_platform_arch = true; platform_arch = matching_module_spec.GetArchitecture(); } jasonmolenda wrote: > Should this conditional

[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

This revision was automatically updated to reflect the committed changes. Closed by commit rG0fa520af6734: Unify the code that updates the ArchSpec after finding a fat binary (authored by aprantl). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D85049?vs=283669&

[Lldb-commits] [PATCH] D85243: Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

aprantl added inline comments. Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp:558 + "arm64e-apple-ios-simulator", "arm64-apple-ios-simulator", + "x86_64-apple-ios-simulator", "x86_64h-apple-ios-simulator", +#else

[Lldb-commits] [PATCH] D85243: Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

aprantl added inline comments. Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h:96 protected: + const char *m_class_name; + const char *m_description; JDevlieghere wrote: > aprantl wrote: > > JDevlieghere wrote: > > > "While you are her

[Lldb-commits] [PATCH] D85243: Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

aprantl updated this revision to Diff 283729. aprantl added a comment. Address feedback from @JDevlieghere CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85243/new/ https://reviews.llvm.org/D85243 Files: lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt lldb/source/Plugins/Platform

[Lldb-commits] [PATCH] D85243: Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

aprantl updated this revision to Diff 283730. aprantl marked 2 inline comments as done. aprantl added a comment. clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85243/new/ https://reviews.llvm.org/D85243 Files: lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt lldb/sour

[Lldb-commits] [PATCH] D85243: Factor out common code from the iPhone/AppleTV/WatchOS simulator platform plugins. (NFC)

This revision was automatically updated to reflect the committed changes. Closed by commit rG243903f3262d: Factor out common code from the iPhone/AppleTV/WatchOS simulator platform… (authored by aprantl). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[Lldb-commits] [PATCH] D85542: lldbutil: add a retry mechanism for the ios simulator

aprantl created this revision. aprantl added reviewers: JDevlieghere, jingham, jasonmolenda, friss. aprantl requested review of this revision. We've been seeing this failure on green dragon when the system is under high load. Unfortunately this is outside of LLDB's control. https://reviews.llvm

[Lldb-commits] [PATCH] D85542: lldbutil: add a retry mechanism for the ios simulator

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/lldbutil.py:824 +while (retry and error.Fail() and + isinstance(error.GetCString(), six.string_types) and + "Unable to boot the Simulator" in error.GetCString()): --

[Lldb-commits] [PATCH] D85542: lldbutil: add a retry mechanism for the ios simulator

aprantl updated this revision to Diff 283995. aprantl added a comment. Address review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85542/new/ https://reviews.llvm.org/D85542 Files: lldb/packages/Python/lldbsuite/test/lldbutil.py Index: lldb/packages/Python/lldbsuite/test/

[Lldb-commits] [PATCH] D85542: lldbutil: add a retry mechanism for the ios simulator

This revision was automatically updated to reflect the committed changes. Closed by commit rG968cba8e89f7: lldbutil: add a retry mechanism for the ios simulator (authored by aprantl). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D85542?vs=283995&id=284020#toc

[Lldb-commits] [PATCH] D85758: [lldb] Make several attempts to read the PID in TestAppleSimulatorOSType

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py:66 +for _ in range(0, self.PID_RETRIES): stderr = sim_launcher.stderr.readl

[Lldb-commits] [PATCH] D85859: [lldb] Fix relative imports and set the appropriate include dirs

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Looks plausible! Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85859/new/ https://reviews.llvm.org/D85859 ___

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

aprantl added a comment. I only added a few non-substantial inline comments. Comment at: lldb/include/lldb/Target/Trace.h:39 +public: + ~Trace() override = default; + I'm just curious: What's the purpose of this? Is the destructor = 0 in the base class and we

[Lldb-commits] [PATCH] D85988: Fix the ability to list iOS simulator processes.

aprantl added a comment. In D85988#616 , @clayborg wrote: > Adrian: is there something I need to do to enable simulator tests? I added a > test to TestSimulatorPlatform.py but if I run: > > $ ../debug/bin/llvm-lit -sv lldb/test/API/macosx/simulator

[Lldb-commits] [PATCH] D86244: [lldb] Update TestSimulatorPlatform.py now that the Makefile no longer decomposes the TRIPLE

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. I just learned that we no longer compute SDKROOT in Makefile.rules, so this seems resonable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86244/new/ https://reviews.llvm.org/D8624

[Lldb-commits] [PATCH] D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

aprantl added a comment. The code looks fine, I think the test needs an extra REQUIRES. Comment at: lldb/source/Core/ValueObjectVariable.cpp:137 + if (m_data.GetDataStart() && m_data.GetByteSize()) +m_value.SetBytes(m_data.GetDataStart(), m_data.GetByteSize());

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

aprantl added a comment. This looks nice! I'm somewhat suspicious that the new test doesn't specifically test the union case of the old test, but I'm assuming that would still work and your simpler tests covers the same code? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. LGTM with that caveat Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86348/new/ https://reviews.llvm.org/D86348 __

[Lldb-commits] [PATCH] D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

aprantl added a comment. @shafik would you mind preparing a separate commit that add the REQUIRES everywhere it is needed, or if there are many, create an x86 subdir with an implicit requirement (like we do in the llvm/test/debuginfo tests)? If it weren't for Rosetta, these would break on Apple

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

aprantl added a comment. Sounds good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86348/new/ https://reviews.llvm.org/D86348 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm

[Lldb-commits] [PATCH] D86493: [lldb][NFC] Remove unused/misnamed SetObjectModificationTime

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Sometimes these odd leaf APIs are used by custom downstream variants of LLDB, but without any comment pointing into that direction and no unit test, I think this makes sense to remove. Also,

[Lldb-commits] [PATCH] D86493: [lldb][NFC] Remove unused/misnamed SetObjectModificationTime

aprantl added a comment. The largest downstream LLDB fork that I know about is swift-lldb, which is also open source and thus easy to check. There also exists a Rust version, and many custom forks for various specialized hardware. Putting patches up for review here gives the maintainers a chanc

[Lldb-commits] [PATCH] D86493: [lldb][NFC] Remove unused/misnamed SetObjectModificationTime

aprantl added a comment. I should also mention that we are actively looking into ways to upstream the swift-lldb code into llvm.org. Now that both branches are licensed under Apache the only challenge left is to make the Swift support a real language *plugin* that can be conditionally compiled.

[Lldb-commits] [PATCH] D86615: [lldb/DWARF] Fix handling of variables with both location and const_value attributes

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. What a function :-) Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3134 if ((tag == DW_TAG_variable) || (tag == DW_TAG_constant) || (tag ==

[Lldb-commits] [PATCH] D86662: Simplify Symbol Status Message to Only Debug Info Size

aprantl added a comment. Could you please also include a testcase? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86662/new/ https://reviews.llvm.org/D86662 ___ lldb-commits mailing list lldb-commits@list

[Lldb-commits] [PATCH] D86662: Simplify Symbol Status Message to Only Debug Info Size

aprantl added a comment. Sorry, I missed this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86662/new/ https://reviews.llvm.org/D86662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://l

[Lldb-commits] [PATCH] D86615: [lldb/DWARF] Fix handling of variables with both location and const_value attributes

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3257 +} else if (const char *str = const_value_form.AsCString()) { + uint32_t string_length = strlen(str) + 1; + location = DWARFExpression( -

[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

aprantl added inline comments. Comment at: lldb/source/Commands/CommandObjectTrace.h:1 +//===-- CommandObjectTrace.h --===// +// Can you add `-*- C++ -*-` markers the the `.h` files you are adding? Otherwise tools (li

[Lldb-commits] [PATCH] D87441: Speedup collecting DWARF attribute values

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFAttribute.h:83 + +AttributeValue(const DWARFUnit *_cu, dw_offset_t _die_offset, + dw_attr_t _attr, dw_form_t _form) We usually don't prefix constructor argu

[Lldb-commits] [PATCH] D87626: Fix .debug_aranges parsing.

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

[Lldb-commits] [PATCH] D88516: [lldb] Add a "design" section to the documentation.

aprantl added a comment. What's the difference between design/ and architecture/? These sounds like synonyms to me. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88516/new/ https://reviews.llvm.org/D88516 ___ lldb-c

[Lldb-commits] [PATCH] D88792: [lldb/test] Catch invalid calls to expect()

aprantl added a comment. Oh wow — that paid off! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88792/new/ https://reviews.llvm.org/D88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https:/

[Lldb-commits] [PATCH] D88792: [lldb/test] Catch invalid calls to expect()

aprantl added inline comments. Comment at: lldb/test/API/types/TestRecursiveTypes.py:54 +self.expect("print tpi") +self.expect("print *tpi") I'm not not sure I understand this change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[Lldb-commits] [PATCH] D88866: [lldb] Add another fallback to GetXcodeSDK

aprantl added a comment. So this is adding a fallback that ignores DEVELOPER_DIR if it is set, but doesn't exist? Is that really desirable, or am I misunderstanding the patch? I would expect LLDB to fail hard if the DEVELOPER_DIR in the environment is wrong and not silently ignore it...

[Lldb-commits] [PATCH] D88866: [lldb] Add another fallback to GetXcodeSDK

aprantl added inline comments. Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:405 + auto find_sdk = [&](std::string sdk_name) -> std::string { +std::string developer_dir = GetEnvDeveloperDir(); +if (developer_dir.empty()) perhaps add ```

[Lldb-commits] [PATCH] D88866: [lldb] Add another fallback to GetXcodeSDK

aprantl added inline comments. Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:423 +if (path.RemoveLastPathComponent()) { + developer_dir = path.GetPath(); + std::string sdk = xcrun(developer_dir, sdk_name); Can you use a fr

[Lldb-commits] [PATCH] D88866: [lldb] Add another fallback to GetXcodeSDK

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Apart from a cosmetic detail (inline) this LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88866/new/ https://reviews.llvm.org/D88866 __

[Lldb-commits] [PATCH] D88866: [lldb] Add another fallback to GetXcodeSDK

aprantl added inline comments. Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:378 + auto xcrun = [](std::string sdk, + std::string developer_dir = "") -> std::string { +std::string xcrun_cmd = "xcrun --show-sdk-path --sdk " + sdk; -

[Lldb-commits] [PATCH] D89351: Move initialization of Variable::m_loc_is_const_data into constructor (NFC)

aprantl created this revision. aprantl added a reviewer: JDevlieghere. aprantl requested review of this revision. This makes it symmetric with all other flags and makes it easier to not forget to initialize it. https://reviews.llvm.org/D89351 Files: lldb/include/lldb/Symbol/Variable.h lldb

[Lldb-commits] [PATCH] D89351: Move initialization of Variable::m_loc_is_const_data into constructor (NFC)

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:823 scope, comp_unit.get(), ranges, &decl, location, is_external, false, - false); - var_sp->SetLocationIsConstantValueData(false); + false, false); ---

[Lldb-commits] [PATCH] D89428: Add support for more OS types to AddClangModuleCompilationOptionsForSDKType()

aprantl created this revision. aprantl added reviewers: JDevlieghere, teemperor. aprantl requested review of this revision. [This is upstreaming an Apple Silicon support patch rdar://problem/66927829] This patch also avoids hardcoding the clang options, which makes it less likely for them to bec

[Lldb-commits] [PATCH] D89428: Add support for more OS types to AddClangModuleCompilationOptionsForSDKType()

aprantl updated this revision to Diff 298453. aprantl added a comment. Address review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89428/new/ https://reviews.llvm.org/D89428 Files: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp Index: lldb/source/Plugins/Platform/

[Lldb-commits] [PATCH] D89428: Add support for more OS types to AddClangModuleCompilationOptionsForSDKType()

aprantl added inline comments. Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1525 + // If the SDK type is for the host OS, use its version number. + auto get_host_os = []() { return HostInfo::GetTargetTriple().getOS(); }; switch (sdk_type) {

[Lldb-commits] [PATCH] D89428: Add support for more OS types to AddClangModuleCompilationOptionsForSDKType()

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGcf245086518e: Add support for more OS types to AddClangModuleCompilationOptionsForSDKType() (authored by aprantl). Herald

[Lldb-commits] [PATCH] D89351: Move initialization of Variable::m_loc_is_const_data into constructor (NFC)

aprantl closed this revision. aprantl added a comment. landed in 9b1c06c0e84a9cc763e12b289becb5fc3c9d01ea CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89351/new/ https://reviews.llvm.org/D89351

[Lldb-commits] [PATCH] D68156: [debugserver] Add --version/-V command line option to debugserver.

aprantl added a comment. Good catch! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68156/new/ https://reviews.llvm.org/D68156 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[Lldb-commits] [PATCH] D68169: Fix a regression in FindTypes

aprantl created this revision. aprantl added reviewers: friss, JDevlieghere, jasonmolenda. Herald added a project: LLDB. In r368345 I accidentally introduced a regression that would over-report the number of matches found by FIndTypes if the DeclContext Filter was hit. This patch fixes this issu

[Lldb-commits] [PATCH] D68171: Remove unused "append" parameter from FindTypes API

aprantl created this revision. aprantl added reviewers: labath, friss, JDevlieghere, jasonmolenda. I noticed that SymbolFileDWARFDebugMap::FindTypes was implementing it incorrectly (passing append=false in a for-loop to recursive calls to FindTypes would yield only the very last set of results),

[Lldb-commits] [PATCH] D68169: Fix a regression in FindTypes

aprantl planned changes to this revision. aprantl added a comment. Pavel suggested to just remove the return value altogether, which would be even better. I'll give that a try. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68169/new/ https://revie

[Lldb-commits] [PATCH] D68169: Remove size_t return parameter from FindTypes

aprantl updated this revision to Diff 222473. aprantl retitled this revision from "Fix a regression in FindTypes" to "Remove size_t return parameter from FindTypes". aprantl edited the summary of this revision. aprantl added a reviewer: labath. Herald added subscribers: dexonsmith, mehdi_amini. C

[Lldb-commits] [PATCH] D68169: Remove size_t return parameter from FindTypes

aprantl updated this revision to Diff 222476. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68169/new/ https://reviews.llvm.org/D68169 Files: lldb/include/lldb/Core/Module.h lldb/include/lldb/Core/ModuleList.h lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Symbol/TypeLis

[Lldb-commits] [PATCH] D68171: Remove unused "append" parameter from FindTypes API

aprantl added a comment. I made a blind attempt in r373342. Hopefully that should fix it. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68171/new/ https://reviews.llvm.org/D68171 ___ lldb-commits mailing list lldb-com

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

aprantl added a comment. Cool. I mean both are supposed to be JSON, but are we expecting any fallout from a debugserver using the old library vs and lldb using the new one? I suppose not.. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68248/new/ https://revie

[Lldb-commits] [PATCH] D68282: [JSON] Use LLVM's library for decoding JSON in StructuredData

aprantl added inline comments. Comment at: lldb/source/Utility/StructuredData.cpp:48 } - - JSONParser json_parser(buffer_or_error.get()->getBuffer()); - return_sp = ParseJSONValue(json_parser); - return return_sp; + return ParseJSON(buffer_or_error.get()->getBuffer().str()

[Lldb-commits] [PATCH] D68326: [lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST

aprantl added inline comments. Comment at: clang/include/clang/AST/ExternalASTMerger.h:87 const OriginMap &OM; +/// True iff the source only exists temporary, i.e. it will be removed from +/// the ExternalASTMerger during the life time of the ExternalASTMerger. -

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

aprantl added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:895 + /// Dump clang AST types from the symbol table + /// nit: `.` at the end Comment at: include/lldb/Symbol/ClangASTContext.h:896 + /// Dump clang AST type

[Lldb-commits] [PATCH] D68366: Parametrize scripted ThreadPlans using SBStructuredData

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. From a purely mechanical point of view, this lgtm. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68366/new/ https://reviews.llvm.org/D68366

[Lldb-commits] [PATCH] D68558: [Testsuite] Get rid of most of the recursive shared library Makefiles

aprantl added a comment. This is some really tricky Make (wow), but I think the benefit of it being so much shorter and localized outweighs the steeper learning curve. Thanks! Comment at: lldb/packages/Python/lldbsuite/test/commands/expression/top-level/Makefile:9 + $(M

[Lldb-commits] [PATCH] D68595: Remove constructor and unused function (NFC)

aprantl created this revision. aprantl added a reviewer: JDevlieghere. https://reviews.llvm.org/D68595 Files: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

[Lldb-commits] [PATCH] D68597: Replace static const StringRef with StringRef (NFC)

aprantl created this revision. aprantl added a reviewer: JDevlieghere. I just don't think that we are saving anything by making these StringRefs global variables. The strings they reference are constants anyway. https://reviews.llvm.org/D68597 Files: lldb/source/Interpreter/OptionValueBoolea

[Lldb-commits] [PATCH] D68597: Replace static const StringRef with StringRef (NFC)

aprantl updated this revision to Diff 223664. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68597/new/ https://reviews.llvm.org/D68597 Files: lldb/source/Interpreter/OptionValueBoolean.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp lldb/source

[Lldb-commits] [PATCH] D68536: [MachO] Fix symbol merging during symtab parsing.

aprantl added inline comments. Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:3736 if (is_debug) { switch (nlist.n_type) { because of the early exit above — isn't this either always true or always false?

[Lldb-commits] [PATCH] D68536: [MachO] Fix symbol merging during symtab parsing.

aprantl added inline comments. Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:3736 if (is_debug) { switch (nlist.n_type) { aprantl wrote: > because of the early exit above — isn't this either always true or always > false

[Lldb-commits] [PATCH] D68605: ObjectFileMachO: Replace std::map with llvm::DenseMap (NFC)

aprantl created this revision. aprantl added reviewers: JDevlieghere, labath. This makes parsing the symbol table of clang ~10ms faster. (Hashtable versus tree). https://reviews.llvm.org/D68605 Files: lldb/include/lldb/Utility/ConstString.h lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileM

[Lldb-commits] [PATCH] D68558: [Testsuite] Get rid of most of the recursive shared library Makefiles

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/make/Makefile.rules:771 else - $(RM) "$(EXE)" + $(RM) -r $(wildcard $(BUILDDIR)/*) endif I would just error, period. IMHO `make clean` shouldn'd delete anything that `ma

[Lldb-commits] [PATCH] D68558: [Testsuite] Get rid of most of the recursive shared library Makefiles

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. > We don't delete $(BUILDDIR), we delete what's inside of it. Sorry, I missed that detail! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68558

[Lldb-commits] [PATCH] D68609: Replace regex match with rfind (NFCish)

aprantl created this revision. aprantl added reviewers: JDevlieghere, friss. This change is mostly performance-neutral since our regex engine is fast, but it's IMHO slightly more readable. Also, matching matching parenthesis is not a great match for regular expressions. https://reviews.llvm.or

[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

aprantl added a comment. Disclaimer: I'm not familiar with this code. Would it be safe/better to store a weak pointer instead of a raw pointer? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68641/new/ https://reviews.llvm.org/D68641

[Lldb-commits] [PATCH] D68605: ObjectFileMachO: Replace std::map with llvm::DenseMap (NFC)

aprantl abandoned this revision. aprantl added a comment. Pavel correctly pointed out that this isn't necessarily correct, because the addresses could conflict with the Tombstone/Empty keys. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68605/new/ https://reviews.llvm.org/D68605 ___

[Lldb-commits] [PATCH] D68595: Remove constructor and unused function (NFC)

This revision was automatically updated to reflect the committed changes. Closed by commit rGbde5a6a45a3c: Remove constructor and unused method (NFC). (authored by aprantl). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D68597: Replace static const StringRef with StringRef (NFC)

This revision was automatically updated to reflect the committed changes. Closed by commit rG917b8df0e531: Replace static const StringRef with StringRef (NFC) (authored by aprantl). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D68597?vs=223664&id=223886#toc R

[Lldb-commits] [PATCH] D68609: Replace regex match with rfind (NFCish)

This revision was automatically updated to reflect the committed changes. Closed by commit rGea6377505435: Replace regex match with rfind (NFCish) (authored by aprantl). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D68609?vs=223695&id=223887#toc Repository:

[Lldb-commits] [PATCH] D68605: ObjectFileMachO: Replace std::map with llvm::DenseMap (NFC)

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG8db229e28771: ObjectFileMachO: Replace std::map with llvm::DenseMap (NFC) (authored by aprantl). Herald added a project: L

[Lldb-commits] [PATCH] D68641: [LLDB] Fix for synthetic children memory leak

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Cool. May be nice to put a comment on the pointer why this is safe. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68641/new/ https://reviews.llvm.org/D6864

[Lldb-commits] [PATCH] D68549: make ConstString allocate memory in non-tiny chunks

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This makes sense to me. We may need to make this configurable for low-memory hosts at some point. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68549/new/

[Lldb-commits] [PATCH] D68549: make ConstString allocate memory in non-tiny chunks

aprantl added a comment. I assume you experimented with many different values, and these are the best tradeoff? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68549/new/ https://reviews.llvm.org/D68549 ___ lldb-com

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

aprantl created this revision. aprantl added reviewers: jasonmolenda, jingham, friss. Herald added a subscriber: JDevlieghere. Herald added a reviewer: JDevlieghere. When ingesting aranges from a dSYM it makes sense to always trust the contents of the accelerator table since it always comes from

[Lldb-commits] [PATCH] D68655: Trust the arange accelerator tables in dSYMs

aprantl added a comment. In D68655#1700314 , @jasonmolenda wrote: > Nice fix. The real cost of computing the aranges manually is expanding all > the line tables and calling GetContiguousFileAddressRanges on them. I wonder > if avoiding the full lineta

<    6   7   8   9   10   11   12   13   14   15   >